WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 4 years ago

Last modified 3 years ago

#4732 closed defect (bug) (wontfix)

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

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

Description

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
OS: BSD
PHP: 5.2.4RC1-dev
SQUID: 2.6

Yours,
Aron Szabo

Attachments (4)

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

Download all attachments as: .zip

Change History (22)

comment:1 ryan7 years ago

  • Keywords has-patch added

comment:2 westi7 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.

comment:3 markjaquith7 years ago

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

comment:4 Nazgul7 years ago

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

comment:5 ryan7 years ago

  • Milestone changed from 2.3 to 2.4 (next)

hansengel6 years ago

Patch to fix $guessurl

comment:6 hansengel6 years ago

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

Patched with rev. 6568 (latest ATM).

comment:7 sciolizer6 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 127.0.0.1 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).

comment:8 azaozz6 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.

azaozz6 years ago

comment:9 sambauers6 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.

comment:10 ryan6 years ago

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

comment:11 Denis-de-Bernardy5 years ago

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

Denis-de-Bernardy5 years ago

anchor the regex + strip the port too

Denis-de-Bernardy5 years ago

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

comment:12 follow-up: Denis-de-Bernardy5 years ago

  • Keywords needs-testing added

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

comment:13 Denis-de-Bernardy5 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?

comment:14 Denis-de-Bernardy5 years ago

  • Component changed from Administration to Permalinks

comment:15 in reply to: ↑ 12 hakre5 years ago

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

comment:16 ryan4 years ago

The replacement argument for preg_replace() is missing.

comment:17 azaozz4 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.

comment:18 hakre3 years ago

Related: #16932

Note: See TracTickets for help on using tickets.