WordPress.org

Make WordPress Core

Opened 12 years ago

Closed 7 years ago

Last modified 7 years ago

#10743 closed defect (bug) (worksforme)

WP rewrite rule bug with & in url path

Reported by: stephdau Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Permalinks Keywords:
Focuses: Cc:

Description

We've noticed a peculiar bug in the standard rewrite rules for wp if & is included in any url, as part of the path, not the query string.

EGs:

The rewrite rule serves the default page, without returning a 404.

My best guess at this time is that what is happening is that the inclusion of & in a rewritten path is actually seen as a query param (eg: treated as /index.php?&nbsp=...)

Attachments (1)

10743.patch (515 bytes) - added by stephdau 12 years ago.
patch for /wp-includes/rewrite.php

Download all attachments as: .zip

Change History (14)

#1 @scribu
12 years ago

  • Keywords needs-patch added
  • Milestone changed from Unassigned to 2.9

#2 @stephdau
12 years ago

So, a bit of context: we found the above issue when the msn bot got stuck in an infinite loop on an install because of a bad link from somwhere that ended up being sent to the server as "&nbsp/blah" instead of "/blah".

Having the & anywhere else in the url does not create an issue, only when the 1st character after the root of the wp install.

So in my local install, I fixed it with the following rule, which simply redirects offending url to the same one without the ampersand, therefore giving the proper wp 404 response:

RewriteRule ^&(.*)$ $1 [R=301,L]

As in:

# BEGIN WordPress
<IfModule mod_rewrite.c>
RewriteEngine On
RewriteBase /~epsi/wptrunk/
RewriteRule ^&(.*)$ $1 [R=301,L]
RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-d
RewriteRule . /~epsi/wptrunk/index.php [L]
</IfModule>
# END WordPress

See attached patch for suggested wp-includes/rewrite.php fix.

@stephdau
12 years ago

patch for /wp-includes/rewrite.php

#3 @scribu
12 years ago

  • Keywords has-patch added; needs-patch removed

#4 @ryan
12 years ago

Do setups that don't use mod_rewrite experience the same problem (pathinfo, IIS rewrite). If so we should solve it in WP's query handler.

#5 @ryan
12 years ago

Perhaps $_SERVERQUERY_STRING? is being set for these requests, thus confusing WP::handle_404().

#6 @stephdau
12 years ago

#7 @stephdau
12 years ago

As far as having the fix in the query handler: there are up and down sides to that, but it'd be just as valid a solution, and if nothing else, it'd be easier to control, and for upgrades, since a .htaccess solution (as provided in patch) would have the downside of people having to physically go to the permalinks settings page and re-saving their configs (unless handled in the upgrade, which unless I'm mistaken, isn;t a best practice).

#8 @stephdau
12 years ago

And although it's not a core thing, we might also want to try things out with donncha's wp-super-cache.

#9 @ryan
12 years ago

  • Milestone changed from 2.9 to 3.0

#10 @nacin
12 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 3.0 to Future Release

No patch, moving out of 3.0 for now.

#11 @ryan
7 years ago

  • Owner ryan deleted
  • Status changed from new to assigned

#12 @loushou
7 years ago

  • Resolution set to worksforme
  • Status changed from assigned to closed

This problem is already solved in 4.0. Looks like a simple patch to the WP_MatchesMapRegex() class solved the problem:

  public function callback($matches) {
    $index = intval(substr($matches[0], 9, -1));
    return ( isset( $this->_matches[$index] ) ? urlencode($this->_matches[$index]) : '' );
  }

The urlencode bit, on the return line, resolves the & issue.

This should be closed.

#13 @ocean90
7 years ago

  • Keywords needs-patch removed
  • Milestone Future Release deleted
Note: See TracTickets for help on using tickets.