On my site, we do not use the login block. So I have a login link at the top corner. We wanted to have the user directed back to where they were after logging in so I found the suggestion to add a destination= parameter to the url. This works great except in the case where the URL they were at has a query parameter.
One common example of this occurs when you log out and then log in again immediately:
- when you log out you got to http://www.mysite.com/?time=12345678
- when click login, you go to http://www.mysite.com/user/login?destination=%3Ftime%3D12345678 (looks good so far)
- submitting the login form works great and if you trace the code you'll see that we get to the end of user_login_submit()
- here's where the first possible problem occurs: user_login_submit() grabs the whole destination and return it as the path and then tacks on a query parameter of its own
- we eventually end up in drupal_goto() with $path='?time=12345678' and $query='time=99999999' (the new time for the login)
- now drupal_goto() looks at $_REQUEST itself and pulls out the destination again. things start to get confusing
- it does this: extract(parse_url($_REQUEST['destination']));
- this should totally ignore the values passed into the function, meaning that the time=99999999 is replaced by whatever query string was in the destination string. This seems like possible problem number two, since you obviously want that new time value included.
- except it doesn't quite work that way in this case because the destination URL has no path, so $path doesn't get overwritten. So now we have $path='?time=12345678' and $query='time=12345678'. This is possible bug three.
- now in url(), the path gets encoded. url() will handle external URLs with query strings already attached, but not internal ones. Possible bug number four? So the final URL we get redirected to is http://www.mysite.com/%3Ftime%3D12345678?time=12345678 which returns a page not found error.
So as you can see, there are four things that could be considered bugs which all contribute to this problem. And that leaves me unsure what fix to make. If anyone can give some guidance, I don't mind submitting a patch, but I'll need some pointers.
I feel like user_login_submit() should be smarter about what it's returning. But then if drupal_goto() is just going to ignore it anyway, there's not really any point fixing it there. In which case, the best fix would be to set $query, $path, and $fragment to null before doing the extract(). But that still leaves us with new time=99999999 value not being used (just all the time now instead of some of the time).
Help! :)
Comments
Comment #1
shane birley commentedThis is exactly the problem I am having and I have been to the following to try and figure it out:
http://drupal.org/node/77379
http://drupal.org/node/70521
Comment #2
shane birley commentedI have applied the patch from http://drupal.org/node/70521 but it has not corrected the behaviour.
Comment #3
shane birley commented