Closed (fixed)
Project:
Domain
Version:
5.x-1.8
Component:
Code
Priority:
Critical
Category:
Bug report
Assigned:
Reporter:
Created:
10 Sep 2008 at 16:06 UTC
Updated:
1 Oct 2008 at 17:52 UTC
Jump to comment: Most recent file
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'];
}
}
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | domain.module.patch | 649 bytes | zttt |
| #3 | domain.module.patch | 644 bytes | zttt |
Comments
Comment #1
agentrickardhttp://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?
Comment #2
agentrickardComment #3
zttt commentedI 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.
Comment #4
zttt commentedSorry, 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.
Comment #5
agentrickardWhat 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().
Comment #6
zttt commentedOK. 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.
Comment #7
agentrickardThis 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?
Comment #8
agentrickardCommitted 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.
Comment #9
agentrickardCommitted to 5.x
Comment #10
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.