Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.If you start on a url that has a query string, after you have authenticated using bakery and logged in, the query string will be lost. I think we need to store this in the cookie data, so that we can pass these additional query parameters to drupal_goto().
Here is an initial patch. This is rolled against a site and not the bakery module, as I don;t have the repo checked out atm, sorry :)
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | bakery-decode_destination-1954462-12.patch | 510 bytes | msupko |
| #10 | 1954462-10_6.x-2.x.patch | 2.07 KB | coltrane |
| #9 | 1954462-9_6.x-2.x.patch | 2.1 KB | damiankloip |
| #9 | interdiff.txt | 508 bytes | damiankloip |
| #7 | 1954462-7.patch | 2.85 KB | coltrane |
Comments
Comment #1
damiankloip CreditAttribution: damiankloip commentedHere is a patch against the 7.x-2.x branch.
Comment #2
coltrane#1 didn't apply cleanly to 7.x-2.x for me, re-rolled and added a comment
With this I could login from D7 sub-site at a URL like user/login?destination=filter/tips&s=1 and would end up at /filter/tips?s=1 correctly.
Comment #3
coltraneThere are tests up at https://github.com/bjeavons/Bakery-Chef for this.
cucumber features/queryarguments.featureThe D7 login scenario passes.Comment #4
damiankloip CreditAttribution: damiankloip commentedHere is a patch for the 6.x-2.x branch.
Comment #5
damiankloip CreditAttribution: damiankloip commentedI created a pull request on github for the D5 patch: https://github.com/bjeavons/bakery5/pull/2
Comment #6
coltraneLogin from any D7 subsite with a query parameter like ?sdf=sdf is ending up back with path node%3Fsdf%3Dsdf and 404 message. I think this is because query arguments get added to the form action destination and then lost as separate query arguments.
Edit, query parameters are passed through when the login happens on a path, but when logging in on the front page (/) they are lost.
Comment #7
coltraneFix for D7. D6 patch needs work because drupal_get_query_parameters() doesn't exist in D6.
Comment #8
damiankloip CreditAttribution: damiankloip commentedThat's a good point :)
Comment #9
damiankloip CreditAttribution: damiankloip commentedRerolled the 6.x-2.x patch, I think we only need to unset ['q']?
Comment #10
coltraneThe query argument to drupal_goto() was wrong for 6. Updated patch.
With these two patches the following scenarios pass:
* As a Bakery user, I want my query arguments to carry through login on D6 from path (/user/login)
* As a Bakery user, I want my query arguments to carry through login on D6 (no path)
* As a Bakery user, I want my query arguments to carry through login on D7 on a path
* As a Bakery user, I want my query arguments to carry through login on D7 (no path)
However, on a URL like "http://subsite.example.com/?destination=filter/tips&foo=bar" I'll end up on filter/tips but without "foo=bar".
Are there any other tests you can think of?
It strikes me that all of the query arguments could be passed along with destination, instead of as a separate cookie data element. That would probably run require URL decoding/encoding tho. What do you think Damian?
Comment #11
coltraneI re-reviewed this today and think it looks good for login. Committed http://drupalcode.org/project/bakery.git/commit/f9030d5 and http://drupalcode.org/project/bakery.git/commit/db96ba4
Thanks @damiankloip!
Leaving open to handle the remaining case of query arguments when destination is set as well as handling registration.
Comment #11.0
coltraneUpdated issue summary.
Comment #12
msupko CreditAttribution: msupko commentedI'm having a few issues with http://drupalcode.org/project/bakery.git/commit/f9030d5 and URL encoding. For example, suppose a visitor tries to go to this URL:
http://www.example.com/search/stuff?page=2
In our case, r4032login module redirects them to:
http://www.example.com/user?destination=search/stuff%3Fpage%3D2
Without bakery module enabled, this works as expected—user logs in, goes directly to the right page. With bakery module enabled, the user gets redirected to a 404 at:
http://www.example.com/search/stuff%3Fpage%3D2
It seems the code in this commit is checking $cookie['data']['destination'] for a '?', which it will never find since it's encoded as %3F instead, at least in our case. I'm attaching a patch to decode the destination before redirecting the user—this resolves the issue for us.