Fixes to Drupal-created URLs

I have previously made a post related to this problem, but since then I've discovered the full effects it had, and the fix I made revealed another bug elsewhere. My original post can be found at http://drupal.org/node/104688, however that post is now obsolete.

The original problem I discovered was that when clean URLs are enabled, Drupal writes URLs incorrectly when the URL contains an argument. I noticed this first in the pager where the links would not work, but later I found more examples in the administration pages. In these cases, another bug was revealed by my original fix: the destination argument was corrupted.

To illustrate the problem, here is a bad URL as generated bu Drupal:

"http://www.mysite.com/admin/taxonomy/edit/term/4?destination=admin%2Ftax..."

The correct URL is this:

"http://www.mysite.com/admin/taxonomy/edit/term/4&destination=admin/taxon..."

There are two bugs present here. First, the usage of the '?' symbol is wrong, it should be '&' instead. Note that while '?' should be used with the first argument, it is in fact the second here, because we're using clean URLs. The default rewrite rule used to enable clean URLs is to add "index.php?q=" in front of everything, and that is where the first argument comes from.

The second problem became apparent only after I fixed the first. Because the first bug prevented the second argument from working at all, the destination argument in the above example was ignored. After the fix it was no longer ignored and turned out to be corrupted. For some reason, the urlencode function is used to format the destination path, which corrupts it by replacing the '/' symbols with junk. Without either fix the destinations did not work at all and were silently ignored.

There are two fixes that need to be made, both of them in the includes/common.inc file.

For the first bug, line number 1033 of that file reads

return $base . $path . '?' . $query . $fragment

when it should be

return $base . $path . '&' . $query . $fragment

For the second bug, lines number 211 and 220 read

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

when they should be

return 'destination='. $_REQUEST['destination'];
return 'destination='. $path;

I'm not aware of these fixes breaking anything else: I know that they allow the pager links and destination paths to work correctly, which they do not without the fixes. Also note about the first fix that it only concerns cases where there actually are more arguments than one: the front page, where there is no q argument, is handled separately and '?' is used correctly in that case.