Page being redirected in a way that will never complete
bonobo - April 12, 2008 - 00:32
| Project: | Checkout (Content locking) |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | needs review |
Description
To replicate:
A 5.7 install, on a content type (in this case, a blog post) with no form of access control installed.
Create the blog post. Hit submit. Click the edit tab on the newly created node. Edit the node. Hit submit. Get whitescreen, and eventually the error message that "Firefox has determined that the page is being redirected in a way that will never complete.
Then, on every page load, you see the repeated message in the attached screenshot.
| Attachment | Size |
|---|---|
| checkout.png | 105.7 KB |

#1
the same with Drupal 6.3
#2
Doesn't happen for me on a fresh installation (tested with 6.x-2.0). Either there is another module interfering (a list of enabled modules would help), or your description is not accurate enough to reproduce this issue.
#3
Back when I initially reported the bug, I had it on a completely vanilla install -- core plus checkout, nothing else. I can't speak for the 6.x version, though, as I haven't tested that yet.
#4
maybe:
referer_uri(), GET['q'] and clean urls ?
i use clean urls and have problem.
EDIT: i checked witch normal urls and all work ok.... where is problem with clean urls???
#5
Could you please try the attached patch and see if it helps (it applies to D5 and D6 versions)?
#6
Yeah,... cool, work but i have still this:
"This document is locked for editing by Tomek since sob., 12/07/2008 - 00:33.
Click here to check back in now."
on non clean urls this not hapen when i click elsewhere :)
thx. for fast reply.
good module :)
#7
hm, too fast i reply ...
i found:
node/211/edit = ~tomasz/ni/content/burnproof-i-overburn
maybe pathauto?
#8
Can you tell me a little bit more about the path on the right side: which elements of the path are belonging to the path alias? I can imagine "content/burnproof-i-overburn" would be the alias, but where is "~tomasz/ni" coming from?
#9
i test on localhost with rewite base. never mind, problem is one:
clean urls. when i turn off are work ok.
your solution with :
if ($_GET['q'] == $referer) {
return;
}
this is not enough :(
i search in code and when i found some idea then tell you ... thanks for help and reply.
#10
I think I've got it fixed. Could you please test again, Tomasz?
#11
Hi,
Yeah! Now this work like on non clean urls - just good. Thanks for your work. Good job.
I think that you can apply this patch to module and release.
Once thanks.
#12
Thanks for testing, Tomasz.
Committed to D5 and D6 branches.
#13
Hi,
it still happens when using prefixes for languages. E.g., when I edit a language neutral page in my default language, the path is sth. like "node/138". When I switch to english (with the page still the same, because it is language neutral), the path is "en/node/138". When I edit the node and click save, I run into the endless redirection loop again because
current path: node/138/editreferer: en/node/138/edit
Maybe we can ignore everything before "node/"? Please see also http://drupal.org/node/285702, maybe both bugs can be hit with changing one regexp?
#14
The problem is that
$_GET['q']gets the language prefix removed in language_initialize which is called for every page, but referer still contains it, so your patch doesn't work when using language prefixes.The patch cleans $referer from a language prefix if it exists. Note: the patch doesn't fix http://drupal.org/node/285702. Additional patch there.
#15
Hi!
I can confirm having the same issue, and that the patch in comment #14 fixes the problem. Thanks!
#16
If someone who tested it would set the status to "Patch reviewed and tested" it might help making smk-ka consider it :-)
#17
If someone who tested it would set the status to "Patch reviewed and tested" it might help making smk-ka consider it :-)
I could do that, but my knowledge of php resumes to:
patch -i ../checkout_clean_referer_from_lang.patchso, the testing is OK from my side, but reviewing should be done by someone would actually understands php :)
#18
I see ;-)
#19
Having same issue when: Editing the page, then clicking "Preview", then clicking "Submit."
Have tested by disabling checkout module, and this removes the error.
#20
Tried the patch from #14, this didn't fix the issue.
#21
Strange... but good for me, using 6.x-2.2 with patch from #14 works for me on my devel and production site (without the patch i had the redirect problem because of language prefix).
My php knowledge is sorryly very low so i better don't change the status of this bug.
#22
Same issue with lang prefixes in url and last dev release.
#23
The dev version doesn't contain the patch, could you test it?
I've no idea why smk-ka doesn't take a look at the patch :-(
#24
@@ -197,6 +197,19 @@$referer = drupal_get_normal_path($referer);
}
...
+ $referer = preg_replace("?^". $language->prefix ."/?",'',$referer);
We want to munge $referer before invoking drupal_get_normal_path().
+ // $_GET['q'] doesn't have a language prefix, referer_uri does (if defined).+ // Remove it from $referer to enable comparison with $_GET['q'].
Please append
()when referring to function names in comments. Also, we can shorten this to or similar.+ // Check language_negotiation mode because prefix can be defined without+ // using it in URLs, and someone could define a fixed path starting
+ // with the same string as a language prefix.
Can we shorten this comment and make it up to the point? Something like or similar.
+ switch ($mode) {+ case LANGUAGE_NEGOTIATION_PATH_DEFAULT:
+ case LANGUAGE_NEGOTIATION_PATH:
Wrong indentation here.
+ $referer = preg_replace("?^". $language->prefix ."/?",'',$referer);If you use
?as delimiter, you need to preg_quote() $referer accordingly. Please use@or similar instead. In any case,preg_quote($referer, '@')is required. Please also fix the spacing (after commas) in this line.#25
Suscribing
#26
Not tested yet, but the proposed solution might not work in case a language has been switched while being on a locked node, since only the current language is considered. My guess is that we need to match all available languages instead to be on the safe side.
#27
Now, I am everything else than sure about this new patch, which incorporates almost a 1:1 copy of language_initialize(). However, I think we need to be able to determine the correct language of the referring URL, because otherwise drupal_get_normal_path() won't be able to resolve the internal path, and that would be bad. It would be extremely helpful if somebody with a real-world multilanguage setup could test this patch and report back. Of special interest here, as written in my last comment, is switching the language while on a node edit page, which should still leave the node locked (ie. display no messages).
#28
Subscribing
#29
Subscribing.
#30
I can confirm it works for me, but I've only two languages and the default one is without a prefix, so I don't hit your improvement.
But at least it fixes the original bug for a two-language site, so it's better than nothing :-)
#31
This is against the 6.x version.
I pursued a little bit different take on the problem; since, even with the latest patch, I had issues with other kinds of prefixes:
- moved the check in code to hook_nodeapi() ($op == 'prepare')
- the non-persistent lock seemed a poster-child for sessions, so, I used sessions to eliminate all of the regexing on paths
- it then made sense to make clearing of non-persistent locks a cleanup task, so I killed the hook_init() in favor of hook_exit()
The problem with this patch is the loss of locking on revisions. No idea if this is an acceptable loss; but, I thought I would provide the patch anyways.
#32
Subscribing