Have you ever wondered why drupal's destination parameter appears to be scrambled? At least jpetso has... ;) #192603: Use drupal_urlencode() instead of plain PHP urlencode()

The problem is that the destination parameter gets urlencoded two times, first off drupal_get_destination() does, then url() does and so drupal_goto() as well as l(). So if drupal_get_destination() is used together with drupal_goto() or together with l(), which is common usage, the urls get urlencoded twice.

Attached is a patch, which fixes the problem by changing drupal_get_destination() to not urlencode the destination. It's not documented whether drupal_get_destination() does urlencoding, but it's documented that it can be used together with drupal_goto(). So changing the urlencoding to fix the documented use case should be fine.

Note: This problem appears to apply to drupal 5, 6 and 7.
Patch for d7 attached.

CommentFileSizeAuthor
drupal_7_destination_double_url_encoded.patch791 bytesfago

Comments

fago’s picture

fago
Primary language German
Location Vienna
commented
Status: Active » Needs review

fixing status

c960657’s picture

c960657 commented

What do you mean by “scrambled”? Could you give steps to reproduce the problem?

fago’s picture

fago
Primary language German
Location Vienna
commented

Take a look at the links generated with a destination parameter, e.g. go to the "admin/content/node" page and click on edit for a node. You land at /node/240/edit?destination=admin%2Fcontent%2Fnode where it should be /node/240/edit?destination=admin/content/node.

fago’s picture

fago
Primary language German
Location Vienna
commented

Related issue: #310139: drupal_query_string_encode() should not call drupal_urlencode()

cburschka’s picture

cburschka
they
commented
Status: Needs review » Needs work

I'm doubtful about the first change:

-    return 'destination=' . urlencode($_REQUEST['destination']);
+    return 'destination=' . $_REQUEST['destination'];

PHP decodes request parameters before making them available in $_REQUEST. What this code does is copy a decoded version of destination into a place that needs an encoded one.

The other change makes sense assuming that $path has already been encoded to take care of special characters.

tbm13’s picture

tbm13 commented

I get a redirect to a bogus URL from the login button after enabling clean URLs, and it sounds like my issue is related to the one discussed here. Maybe someone can take a look at the issue I just reported: http://drupal.org/node/480726

c960657’s picture

c960657 commented
Status: Needs work » Fixed

This is no longer an issue in HEAD. drupal_get_destination() has been modified to use the new drupal_http_build_query() that does not encode slashes in the query string.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.