Make WordPress Core

Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#16932 closed defect (bug) (invalid)

Media Upload top navigation producing wrong URLs when WordPress is run behind a ReverseProxy

Reported by: tploch's profile tploch Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.1
Component: Administration Keywords:
Focuses: Cc:

Description

The following scenario is in place:

WordPress v3.1 is running on http://example.com. Since the target server is handling very sensitive data, WordPress is integrated into another site ( http://othersite.com/blog ) via a mod_rewrite ReverseProxy Directive:

RewriteEngine On
RewriteRule blog(.*) http://example.com$1 [P]

Both WordPress and Blog URLs are set to 'http://othersite.com/blog'.

Now when creating a new article and using the top ajax link to upload an image, the resulting iframe has a top navigation that now shows following (absolute) href attribute:

'/wp-admin/media-upload.php?post_id=1116&type=image&tab=type'

This will fail in my scenario, since the browser would translate that to 'http://othersite.com/wp-admin/media-upload.php?post_id=1116&type=image&tab=type' while the correct URL should be 'http://othersite.com/blog/wp-admin/media-upload.php?post_id=1116&type=image&tab=type'.

I think prepending the link with the configuration blog URL should fix this, and I tried to do this, but unfortunately I am not that skilled to pinpoint the location of this matter.

Thanks in advance to take this into consideration.

Attachments (2)

wp_admin_screen.png (127.1 KB) - added by tploch 13 years ago.
Screenshot Media Upload top navigation
16932-hotfix-path-mapping.patch (975 bytes) - added by hakre 13 years ago.
fix for a broken path mapping

Download all attachments as: .zip

Change History (18)

@tploch
13 years ago

Screenshot Media Upload top navigation

#1 @tploch
13 years ago

  • Cc t.ploch@… added

#2 @tploch
13 years ago

Ok, I tracked down the problem to the following line of code

wp-admin/includes/media.php - function the_media_upload_tabs() - lines 81 - 82

$href = add_query_arg(array('tab'=>$callback, 's'=>false, 'paged'=>false, 'post_mime_type'=>false, 'm'=>false));
$link = "<a href='" . esc_url($href) . "'$class>$text</a>";

When changing this to the following code, it is failsafe since the absulute URL is used:

$href = add_query_arg(array('tab'=>$callback, 's'=>false, 'paged'=>false, 'post_mime_type'=>false, 'm'=>false));
$link = "<a href='" . esc_url(home_url() . $href) . "'$class>$text</a>";

#3 @nacin
13 years ago

You should probably spoof REQUEST_URI in wp-config in a reverse proxy situation. This isn't the only location it is used in core, and it comes down to a server configuration issue.

#4 @hakre
13 years ago

Thanks for reporting the issue. I see some related Issues which might contain some helpful information for you: #16858, #9235, #8593.

So far I'll take a look in the area you highlighted as it looks related to some of the work I already did today.

#5 @hakre
13 years ago

Line 81 in /wp-admin/includes/media.php is making use add_query_arg() with one parameter only. The docblock of add_query_arg() has not documented the second parameter optional, however, the function deals with that circumstance (one parameter only). It's complicated to read and understand that function.

#6 @nacin
13 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

The third parameter (or second, when the first is an array of key/value pairs) is optional. When omitted, it uses REQUEST_URI.

This is not a bug in WordPress. It affects every use of REQUEST_URI.

#7 @hakre
13 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

I wish you would not close tickets that fast Nacin.

#8 @nacin
13 years ago

  • Resolution set to invalid
  • Status changed from reopened to closed

It's an invalid ticket, hakre.

Last edited 13 years ago by nacin (previous) (diff)

#9 @hakre
13 years ago

It's just I'm still working on it nacin.

@hakre
13 years ago

fix for a broken path mapping

#10 @hakre
13 years ago

Attached you find a patch for the add_query_arg() function. I've worked extensively with is over the last hours (#16943) and I think it's pretty safe to basname the request uri as the default request uri at least as much as esc_url() this will allow later on.

I've played with the patch applied a bit it runs well from my end but keep in mind that you should test this thoroughly.

On my server I've even changed the path of the request_uri extensively and that change makes more robust agains these mapping problems.


Added 2011-03-24 08:53 UTC

Related: #11136 - Looks like the Media Upload Tabs are more common to not work under such situations.

Last edited 13 years ago by hakre (previous) (diff)

#11 @nacin
13 years ago

It's not just add_query_arg() that uses REQUEST_URI. This is a server configuration issue.

#12 @tploch
13 years ago

Erm, how is this a server configuration issue if you are not even checking for X-Forwarded-For and X-Forwarded-Host headers? I tried setting the Host header via the 'ProxyPreserveHost On' directive, but that broke the whole app. Relying on REQUEST_URI is bad practice and should be avoided in the first place. And telling me to spoof REQUEST_URI in wp-config.php is surely not the right way to solve this.

If you have a user specifying the root url himself, why not use this base url? I don't really understand why you say it's a server issue while this is clearly a bad handling on apllication level...

#13 @dd32
13 years ago

Erm, how is this a server configuration issue if you are not even checking for X-Forwarded-For and X-Forwarded-Host headers?

In this case, it doesnt sound like those headers make a difference. REQUEST_URI only contains the path being requested, ie. on your system it *should* be /blog/wp-admin/media-upload.php?post_id=1116 but seems to be set to /wp-admin/media-upload.php?post_id=1116

Dump out $_SERVER and check the paths, see if they all match up as they should.

#14 @tploch
13 years ago

Well, I changed the routing now by basically passing the whole URI containing the /blog part to the proxy rewrite, and on the target server have another rewrite rewriting it from /blog/foo/bar.php to /foo/bar.php

REQUEST_URI now reads /blog/wp-admin/media-upload.php?post_id=1116, but stuff still breaks.

i.E. When being on the media dashboard, the pagination uses the wrong domain http://example.com/blog/wp-admin/upload.php?paged=1 while it should be http://otherdomain.com/blog/wp-admin/upload.php?paged=1, so fixing REQUEST_URI did not fix the general problem, which is inconsistent usage of hosts throughout the app.

This time the HTTP_X_FORWARDED_HOST header, if this is existent, it should take precedence over the configured home url.

For me is another problem, there is no way that HTTP_X_FORWARDED headers tell you the protocol of the original request, and in my case i Proxy through SSL, so just inspecting this header is not enough.

Probably this setup is beyond the scope of what WordPress should be able to do, but if the HTTP_X_FORWARDED_HOST header was sent, you could inspect the referer protocol (which will be set to the proxy domain).

#15 follow-up: @dd32
13 years ago

the pagination uses the wrong domain http://example.com/blog/wp-admin/upload.php?paged=1 while it should be http://otherdomain.com/blog/wp-admin/upload.php?paged=1, so fixing REQUEST_URI did not fix the general problem

sounds like that's because of the usage of HTTP_HOST #16858 (HTTP_HOST should have never been used) - That'll be getting fixed.

Probably this setup is beyond the scope of what WordPress should be able to do

Once you start to use another ReverseProxy/Load balancer setup you'll find that they can also use different fieldnames, for example, HTTP_X_ORIGINAL_HOST (Which is what a IIS-based setup uses IIRC)

#16 in reply to: ↑ 15 @tploch
13 years ago

Replying to dd32:

Once you start to use another ReverseProxy/Load balancer setup you'll find that they can also use different fieldnames, for example, HTTP_X_ORIGINAL_HOST (Which is what a IIS-based setup uses IIRC)

One could use $_SERVER['SERVER_SOFTWARE'] to determine which headers/variables to inspect. Well, I hope the HTTP_HOST issue will be fixed in some time, so thanks to all for your help :)

Best regards
Thomas

Note: See TracTickets for help on using tickets.