Using Debian Linux, php 4.4.4, with lighttpd I found that the #action form value in a user login block was being set incorrectly resulting in the user being sent to an invalid page resulting a page not found error of:
node?destination=node
In user_login_block the page the user is sent to on login is set with:
'#action' => url($_GET['q'], drupal_get_destination()),
But on examining drupal_get_destination() if a destination isn't already set then it uses the following to create part of the destination:
// Use $_GET here to retrieve the original path in source form.
$path = isset($_GET['q']) ? $_GET['q'] : '';
So the url #action is set to contains the value of $_GET['q'] twice.
I believe that user_login_block should use the following, which works for me:
'#action' => url(null, drupal_get_destination()),
Lee
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | user_login_block_sets-123217-21.patch | 953 bytes | alansaviolobo |
| #17 | drupal-login-block-action-123217-14-do-not-test.patch | 994 bytes | darren oh |
| #16 | drupal-login-block-action-123217-14.patch | 1.24 KB | darren oh |
| #13 | user.module-123217-13.patch | 793 bytes | darren oh |
| #11 | user.module-123217-11.patch | 801 bytes | darren oh |
Comments
Comment #1
cburschkaIf you already have a proposed fix, you should use "cvs diff -up modules/user/user.module" from the main drupal directory (in a proper CVS checkout of course) and upload the resulting patch here.
I've simply applied your change in this patch rolled against 5.x-dev.
Comment #2
moshe weitzman commentedi see no use for that line at all. so here is a patch to remove it.
with this patch applied, try this:
log out,
go to the /admin page, you get access denied.
login using the block with an admin account
you get to /admin.
i think the code here is very old and simply doesn't belong.
Comment #3
darren ohRemoving the line doesn't work if the user is on the front page of the site. I need to work on this any way, since the current behavior interferese with code I wrote to redirect users to their account edit page if it is incomplete, so I'm assigning it to myself.
Comment #4
darren ohActually, I'd rather not delay this bug fix. The patch in #1 works and is ready to be committed.
Comment #5
darren ohJust realized that this patch would overwrite the current destination if one were set. The existing code is by design. See CVS commit 21851.
Comment #6
moshe weitzman commenteddarren - please clarify why my patch is not desireable. you said "Removing the line doesn't work if the user is on the front page of the site. " but i don't understand - user will be authenticated and again see home page.
Comment #7
darren ohI was taken to the user page rather than the home page.
Comment #8
gábor hojtsyDarren, that a change was added a year and a half ago, did not mean this is by design, the environment could have changed (ie. how the destination is computed). Jakub Suchy is also working on an information page for new user logins, which is in relation to this issue: http://drupal.org/node/161496 (this is a simpler issue, but the bigger idea is rooted here, and there is no actual issue yet for the information page feature).
Seems like the patch here needs a bit more testing.
Comment #9
meba commentedAd Gabor: http://drupal.org/node/162037
Comment #10
darren ohGabor, I set it to "by design" because I believe the original reasons for adding that line still apply. The line only causes problems for servers which don't handle redirection properly. At the same time, this issue is not really about removing the line but about hiding the defect in those servers, which could be accomplished by using
NULLif the result ofdrupal_get_destination()equals$_GET['q'].Comment #11
darren ohUpdated for Drupal 7.
Comment #13
darren ohComment #15
neRok commentedI was just going over some old core bugs to see if any could be closed. The code in 7.22 is
$form['#action'] = url(current_path(), array('query' => drupal_get_destination(), 'external' => FALSE));current_path just returns $_GET['q'], so I think the problem still exists. And Drupal8 code seems the same.
Comment #16
darren ohEmbarrassing that this problem still exists. Updated patch attached.
Comment #17
darren ohPorted patch to Drupal 7.
Comment #18
alansaviolobo commentedpatch still applies
Comment #21
alansaviolobo commentedComment #29
darren ohComment #32
quietone commentedThe action in the user login block was changed to use lazy_builder in #2867703: Change #action in UserLoginBlock to a #lazy_builder to avoid the route cache context , see https://git.drupalcode.org/project/drupal/-/blob/9.3.x/core/modules/user....
Comment #33
pameeela commentedAnyone know whether this is still reproducible in light of all the changes since the last patch?
Comment #36
quietone commentedThere has been no activity here on the patch in 7 years.
Since we need more information to move forward with this issue, and none has been supplied in 8 months I am closing this as outdated. If you are experiencing this problem provide steps to reproduce the issue (starting from "Install Drupal core") and set the status to 'Active'.
Thanks.
Comment #37
darren ohStill a valid Drupal 7 bug.
Comment #38
poker10 commentedI am not able to replicate this bug on clean Drupal 7.88 install. Can you please provide the exactly steps leading to this behavior?
My login block has #action set to:
/node?destination=node, but the login and the next redirect is working correctly. No 404 page.Comment #39
poker10 commentedI agree with @quietone to close this until there are some steps to reproduce the issue.