when adding a redirect from an url that contains a question mark all following slashes will be changed to %2F when saved. furthermore an equal-sign will be appended.

e.g.:
oldpath/index.php?/content.html
becomes
oldpath/index.php?%2Fcontent.html=

the module's Clean URL-option has no effect on this behaviour

Comments

jaydub’s picture

Status: Active » Needs review
StatusFileSize
new2.15 KB

I spent some time on this one and there are several issues involved here a couple of which my patch attempt to address.

Attempts to add a redirect source of:
oldpath/index.php?/content.html

result in the following issues:

* '/' is converted to %2F
* An '=' is added to the query string key '/content.html'
* The above redirect as added won't actually successfully redirect.

The first issue my patch does not address but this actually shouldn't be an issue really as the forward slash being encoded will not result in the redirect failing itself. The conversion is done in drupal_http_build_query() in this case.

The second issue is due to the fact that the query string, in this case '/content.html' has no value. It is just a key in the query string key/value pair sense. This itself shouldn't be an issue but at a certain point redirect module calls drupal_http_build_query() which when checking for a key only query string value looks for a value of NULL.

    // If a query parameter value is NULL, only append its key.
    elseif (!isset($value)) {
      $params[] = $key;
    }
    else {
      // For better readability of paths in query strings, we decode slashes.
      $params[] = $key . '=' . str_replace('%2F', '/', rawurlencode($value));
    }

The query string by the time it is passed to drupal_http_build_query has already been parsed as ('key' => '') instead of ('key' => NULL) and so the equals sign ends up getting added.

The third issue where the redirect itself was failing is rather interesting. If the query string has a key or keys where the key name has a period in it (such as '/content.html') then PHP converts the period in the key name to an underscore (so '/content_html'). This is discussed here for more details: http://stackoverflow.com/questions/68651/can-i-get-php-to-stop-replacing...

Since the redirect in the database as originally added stores the actual entered query string string of '/content.html' but PHP rewrites the query string by the time redirect attempts to look for a redirect the query string is never matched. The fix in this case is look for query string keys are reported by PHP with underscores and add a version of the key with underscores converted to periods.

Finally if redirect is configured to pass through the query string to the redirected path then the query as stored in the redirect object should be passed to drupal_get_query_parameters() rather than calling w/o a query as $_GET will again be used which results in the query string key conversion of periods to underscores.

Status: Needs review » Needs work

The last submitted patch, 1415348-redirect-query-string-issues.patch, failed testing.

jaydub’s picture

Version: 7.x-1.0-beta4 » 7.x-1.x-dev
jaydub’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 1415348-redirect-query-string-issues.patch, failed testing.

jaydub’s picture

Priority: Major » Normal
whthat’s picture

Pulled the second issue out of this patch into its own issue. In an attempt to deliver a usable patch for who just need the "=" in the query string key issue resolved.

See: = is added URL query string after ?key without a value

wylbur’s picture

Status: Needs work » Closed (outdated)

Closing this as Outdated as Drupal 7 is EOL. But do reopen this if this is still applicable to D11.