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.drupal_goto() passes $_GET['destination'] though urldecode() before using the variable. We should urlencode() parameters when generating URLs, but PHP takes care of decoding them again, so calling urldecode() will give unexpected results under certain (rare) circumstances.
The urlencode() call was added in #89059: clicking login from the url under a comment produces 404 error (clean urls) back in D4.7. Since then #578520: Make $query in url() only accept an array changed how destination is encoded, and that may have fixed the original problem. At least it seems that the redirect in comment module (the one that #89059 was fixing) now works even without the urlencode() call.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | drupal-goto-urlencode-3.patch | 11.62 KB | c960657 |
| #2 | drupal-goto-urlencode-2.patch | 1.13 KB | c960657 |
| drupal-goto-urlencode-1.patch | 1.13 KB | c960657 | |
Comments
Comment #2
c960657 CreditAttribution: c960657 commentedComment #3
sunCan this be asserted via tests somehow?
Comment #4
c960657 CreditAttribution: c960657 commentedThe tests for drupal_goto() now includes a test for ?destination=xxx. Without the change to drupal_goto(), the new test would fail.
I also updated and extended the existing tests a bit. I moved the existing tests for drupal_get_destination() from DrupalHTTPRequestTestCase to DrupalGotoTest (the tests didn't have anything to do with drupal_http_request()) and moved the corresponding menu callback from system_test.module to common_test.module (because the test is defined in common.test).
Comment #5
Damien Tournoud CreditAttribution: Damien Tournoud commentedThe change is needed, and the new tests look good to me. RTBC supposing the test suite passes.
Comment #6
Dries CreditAttribution: Dries commentedThis looks good. Thanks for the great tests. Committed to CVS HEAD.