Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.This problem is exhibited in both the cvs and 4.6 versions.
I have two Drupal installations inside subdirectories for testing this module:
http://localhost/drupal-4.6.5
http://localhost/head
To test the new patch for 403 redirection, I logged out, then tried to access ?q=admin. I was given a login form at the access denied screen, as expected. After logging in, I *was* properly redirected to the administration panel (for some weird reason), however my querystring shows up as:
?q=head/&q=admin
I believe this is caused by this code in the logintoboggan_destination function:
$uriarray = explode('/', $uri);
array_shift($uriarray);
if (!variable_get('clean_url', 0)) {
$uriarray[0] = str_replace('?q=', '', $uriarray[0]);
}
This seems to be trying to make a 'clean URL' path from a non-clean URL path, which would be fine except that it assumes the first argument will be the ?q= part, and in my case it is actually the subdirectory that Drupal is installed in.
There is probably an easy way to fix this, I'll do some thinking on it, but if someone else gets to it first, by all means. ;)
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | logintoboggan.module.2006-02-22.patch | 1.31 KB | rmiotke |
| #2 | logintoboggan.module.2006-02-21.patch | 927 bytes | rmiotke |
Comments
Comment #1
webchickhunmonk has comitted a fix to both 4.6 and HEAD. Thanks! :D
Comment #2
rmiotke commentedhad something related to this chunk of code so posting here. not sure if it should be a new post though.
problem: using clean urls (at least), if destination should be drupal root (ie, no anonymous access allowed at all and user requests drupal root and front is node listing), destination converts to the fallback error case of drupal/user.
not sure why the logintoboggan get_destination function constructs "full" path, when the full path might not be correct? (ie, assumes http and not https; assumes env variable SERVER_NAME will be the same as actual host being used [www.acme.com vs. acme.com].)
is there a reason not to use base_path instead of base_url? patch attached for review.
NB: to actually get the problem described above fully fixed requires a patch (i think) to drupal_goto in common.inc, where destination is not checked via shorthand true/false (since drupal root is null) but via isset. additional patch submitted thereto.
Comment #3
hunmonk commentedafaik the request_uri() does not give you the full path, so i don't think this will work. i'm pretty sure the current solution will work fine. feel free to reopen if you find a better solution.
Comment #4
rmiotke commentedi've reopened here, but not sure if you meant i should open as new thread?
i'm pretty sure request_uri() will always give full path from web server document root, whichever global variable (ie, $_SERVER['REQUEST_URI'] or $_SERVER['PHP_SELF']) request_uri() actually ends up using to get the value.
and base_path() will always return the path from web server document root to drupal root.
in other words, if drupal's at http://acme.com/drupal/, both request_uri() and base_path() will return /drupal/.
perhaps i'm misunderstanding but i think my point stands. the code for figuring destination can trip on its assumptions that destination protocol and $base_url protocol will match, and that the reported SERVER_NAME and $base_url host name will match.
additionally, $internal_path will evaluate to false if the intended destination is drupal root, since trying to substr a string by its exact length (or greater length) evaluates to false. in fact, as far as i can tell, since request_uri() *always* returns something (even if it's a single quotation mark, or a single forward slash), the only time you ever hit the "failover" condition (reverting destination to user) is when the intended destination is drupal root. (or some other snafu around string length, like a discrepancy between www.acme.com and acme.com.)
submitting correction to previous patch here. but, again, i might just be missing something. additionally, it makes more sense to me to use drupal root as failover condition rather than user path (since request_uri() always returns something, wherever the user was trying to go, unless of there's something else terribly wrong), but i didn't touch this. (this would also make the patch much simpler, without needing to first preserve a prepended slash [so as not to false out on the substr call] and then substr it out if $internal_path is longer than 1 character.)
FYI: for testing with a destination of drupal root, this patch will only work if a (pending, not yet reviewed even, so be advised) patch to common.inc is also applied. that patch can be found at:
http://drupal.org/node/50772
-r
Comment #5
hunmonk commentedyep, i see what you're saying now. fixed in 4.7 and HEAD, and i also made the default redirect to be drupal's homepage, which makes more sense to me and solves the other issue you were having. note that now you can also set a redirect upon form submission via form api's #redirect param, which is how i handled it.
unfortunately, base_path is brand new, so we'll need to stick w/ the same hack in 4.6 unless somebody has a better idea.
Comment #6
(not verified) commented