Domain Access does not handle properly external domains. For example, a payment module (Webmoney and Roboxchange are tested) for Ubercart forms an external link and the domain of that link is substituted by the current domain.
The error is there:
domain.module

function domain_form_alter($form_id, &$form) {
  // If SEO is turned on, then form actions need to be absolute paths
  // to the currently active domain.  See http://drupal.org/node/196217.
  $seo = variable_get('domain_seo', 0);
  if ($seo) {
    global $_domain;
    $action = parse_url($form['#action']);
    if ($action['query']) {
      $action['path'] .= '?';
    }
    $form['#action'] =  $_domain['scheme'] .'://'. $_domain['subdomain'] . $action['path'] . $action['query'];
  }

This code must be substituted by more correct:

function domain_form_alter($form_id, &$form) {
  // If SEO is turned on, then form actions need to be absolute paths
  // to the currently active domain.  See http://drupal.org/node/196217.
  $seo = variable_get('domain_seo', 0);
  if ($seo) {
    global $_domain;
    $action = parse_url($form['#action']);
    if ($action['query']) {
      $action['path'] .= '?';
    }
    if(empty($action["scheme"]) || empty($action["host"])){
        $form['#action'] =  $_domain['scheme'] .'://'. $_domain['subdomain'] . $action['path'] . $action['query'];
    }
  }
CommentFileSizeAuthor
#6 domain.module.patch649 byteszttt
#3 domain.module.patch644 byteszttt

Comments

agentrickard’s picture

Status: Active » Needs work

http://drupal.org/patch/create

And is empty($action['host']) sufficient? Should we not check to be sure that this is a value external to Drupal and the DA module?

agentrickard’s picture

Title: External domains falure » External domains form action failure
zttt’s picture

StatusFileSize
new644 bytes

I think it is sufficient since if a module proposes a domain-address it should know what it does and the domain-access must not prevent that.
Patch is here. I synchronized it with 5x-1.8 version.

zttt’s picture

Sorry, I could not understand the question properly at last answer.
If there is an incorrect domain address (like no scheme), domain access is better handling this case.
This is my understanding. For my case it would be sufficient only empty($action['host']), but I just enforced that rule.

agentrickard’s picture

Status: Needs work » Needs review

What I am thinking is that if another module provided an #action with scheme and host, we should assume that it can handle the URL correctly. And I think checking for the host is probably sufficient.

What I am wondering is if we would ever need to check if the host is actually external to Drupal and Domain Access. We would do that be checking the host value against the values returned by domain_domains().

zttt’s picture

Version: 5.x-1.7 » 5.x-1.8
StatusFileSize
new649 bytes

OK. I agree that checking for host is sufficient.
Concerning domain_domains(), there could be some incompatible modules which propose incorrect hosts being initialized from $base_url, but as far as I understand (1) this is not in concept of Drupal and (2) $base_url equivalent to $_domain in DA.
On the other hand there could be some specific tasks say interdomain forms. They could be realized by means of module_invoke_all('domainignore') but it is much difficult then simple forming of the specific #action.
So, I think host is sufficient in any case.

This is the corrected patch.

agentrickard’s picture

Priority: Normal » Critical

This is the kind of patch that I would really like to get some confirmation of before committing. It feels right, but I worry about edge cases.

(Oh, and the domain_domains() call in the current patch is not used, so can be removed -- no need to re-roll, though.)

Anyone else care to test?

agentrickard’s picture

Status: Needs review » Patch (to be ported)

Committed to HEAD.

One note -- if another module tries to set the action path using hook_form_alter(), you may still have issues depending on the order in which the modules execute. That will need to be handled on a case-by-case basis.

agentrickard’s picture

Status: Patch (to be ported) » Fixed

Committed to 5.x

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.