Make WordPress Core

Opened 14 years ago

Closed 12 years ago

Last modified 7 years ago

#4732 closed defect (bug) (wontfix)

Standardize REQUEST_URI, was: $guessurl wrong in install.php

Reported by: ar0n Owned by:
Milestone: Priority: normal
Severity: major Version: 2.2
Component: Permalinks Keywords: has-patch reporter-feedback
Focuses: Cc:


Apache >1.3 has a function implemented that gets the real uri named original_uri() (util_script.c).

Let's say wordpress is located at http://example.com/wordpress .

A standard request looks like this:

GET /wordpress/wp-admin/install.php HTTP/1.1

Okey ... now apache original_uri(r) returns /wp-admin/index.php
So PHP global variable $_SERVER[ 'REQUEST_URI' ]; looks like "/wp-admin/index.php"

upgrade-functions.php , upgrade-schema.php:

$guessurl = preg_replace('|/wp-admin/.*|i', '', $schema . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI']);

$guessurl is "http://example.com/wordpress" cool :)

But what if the request is forwarded by squid with default config in accel vhost mode.

Here goes ...

The request is:

GET http:///example.com/wordpress/wp-admin/install.php HTTP/1.1

Apache original_uri(r) returns http:///example.com/wordpress/wp-admin/install.php which is legal.

PHP global variable $_SERVERREQUEST_URI? is "http:///example.com/wordpress/wp-admin/install.php"

upgrade-functions.php , upgrade-schema.php:

$guessurl = preg_replace('|/wp-admin/.*|i', '', $schema . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI']);

$guessurl is "http://example.comhttp://example.com/wordpress" not cool :(

The problem with this that values 'siteurl' and 'home' will be wrong in the options table.

This code solves the problem (replaceing it in both places):

$guessurl = eregi($schema . $_SERVER['HTTP_HOST'], $_SERVER['REQUEST_URI']) ? preg_replace('|/wp-admin/.*|i', '', $_SERVER['REQUEST_URI']) : preg_replace('|/wp-admin/.*|i', '', $schema . $_SERVER['HTTP_HOST'] . $_SERVER['REQUEST_URI']);

APACHE: v2.2.4
PHP: 5.2.4RC1-dev
SQUID: 2.6

Aron Szabo

Attachments (4)

guessurl.patch (1.5 KB) - added by hansengel 14 years ago.
Patch to fix $guessurl
req-uri.patch (484 bytes) - added by azaozz 14 years ago.
4732.diff (511 bytes) - added by Denis-de-Bernardy 13 years ago.
anchor the regex + strip the port too
4732.2.diff (512 bytes) - added by Denis-de-Bernardy 13 years ago.
anchor the regex + strip the port too + allow for 3 (?) slashes after the protocol

Download all attachments as: .zip

Change History (23)

#1 @ryan
14 years ago

  • Keywords has-patch added

#2 @westi
14 years ago

/me wonders out loud ...

Are there any other places where we assume the string we recieve via the GET request has had the http://domain.name stripped from the front?

Should we also move the duplicated code to a function rather than copy and pasting the fix into two places maybe something like:

guess_blog_root_url in wp-admin/includes/upgrade.php that returns the full root url for us.

#3 @markjaquith
14 years ago

If this is a problem we should probably just standardize $_SERVER['REQUEST_URI'] early on in WP load.

#4 @Nazgul
14 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 2.2.3 to 2.3 (trunk)

#5 @ryan
14 years ago

  • Milestone changed from 2.3 to 2.4 (next)

14 years ago

Patch to fix $guessurl

#6 @hansengel
14 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

Patched with rev. 6568 (latest ATM).

#7 @sciolizer
14 years ago

  • Cc sciolizer@… added

I ran into the same problems with squid changing the REQUEST_URI, but I found that if you have access to the squid.conf file, you can workaround the problem (until the patch becomes mainstream) by changing the cache_peer line to include the "originserver" option. You must have the rproxy patch, or be using Squid v3.

For example:

cache_peer parent 80 7 no-query round-robin no-digest originserver

This will prevent squid from altering the REQUEST_URI, and is probably a good idea regardless of whether Wordpress uses REQUEST_URI or not, since other sites might be depending on it.

My workaround only works if you are using one squid server. If you are using multiple squid servers, then there is the possibility that the REQUEST_URI will be changed at the first one and not reverted at the last one. (I'm no squid expert, and I haven't tried this setup.)

Also, I would like to point out that the use of REQUEST_URI in the Wordpress source is much more systemic than just schema.php and upgrade.php. An rgrep on my copy returns 33 hits (version 2.3 of Wordpress).

#8 @azaozz
14 years ago

  • Keywords $guessurl install.php needs-testing removed
  • Summary changed from $guessurl wrong in install.php to Standardize REQUEST_URI, was: $guessurl wrong in install.php

Not sure how often this can happen but when it happens, it's probably a show stopper. Like markjaquith, I think it's best to standardize it early as at last count REQUEST_URI is used 43 times.

14 years ago

#9 @sambauers
14 years ago

Issues discussed here will be impacted by the implementation of #7129, which proposes to move the discussed regular expressions to a single function.

#10 @ryan
14 years ago

(In [8182]) wp_guess_url() and install styling fixes from sambauers. fixes #7129 see #4732

#11 @Denis-de-Bernardy
13 years ago

req-uri works, the regexp is potentially slow if the server gets hammered.

13 years ago

anchor the regex + strip the port too

13 years ago

anchor the regex + strip the port too + allow for 3 (?) slashes after the protocol

#12 follow-up: @Denis-de-Bernardy
13 years ago

  • Keywords needs-testing added

@ar0n: Can you test this latest patch against trunk?

#13 @Denis-de-Bernardy
13 years ago

  • Keywords reporter-feedback added; needs-testing removed

@sciolizer - can you check if this is still needed and confirm the patch works if it is?

#14 @Denis-de-Bernardy
13 years ago

  • Component changed from Administration to Permalinks

#15 in reply to: ↑ 12 @hakre
12 years ago

I just tested the latest patch againt trunk, still applies clean.

#16 @ryan
12 years ago

The replacement argument for preg_replace() is missing.

#17 @azaozz
12 years ago

  • Resolution set to wontfix
  • Status changed from new to closed

Is this still a problem? Seems caused by improper squid config. Feel free to reopen with proper patch if this is still common.

#18 @hakre
11 years ago

Related: #16932

#19 @DrewAPicture
7 years ago

  • Milestone 2.9 deleted
Note: See TracTickets for help on using tickets.