Make WordPress Core

Opened 13 years ago

Last modified 20 months ago

#16858 reviewing defect (bug)

Usage of HTTP_HOST in the administration

Reported by: amirhabibi's profile amirhabibi Owned by: dd32's profile dd32
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.1
Component: Administration Keywords: has-patch needs-refresh
Focuses: Cc:

Description

In some files like wp-admin/includes/class-wp-list-table.php (there are more files I think), some links are created by using the HTTP_HOST variable.

It's better to use the defined wordpress URL to create those links.

In our case, we have a wordpress running behind a Squid cache. When you check for HTTP_HOST you get the *real* HOST and not the one with the Squid server.

To bypass the problem, I added in the main config.php a line like this :

$_SERVER['HTTP_HOST'] = 'www.my-visible-http-host.com';

Without this setting and in our case, some links in the admin (next, previous page, re-ordering...) where pointing to the wrong server.

Attachments (2)

16858.patch (1.8 KB) - added by hakre 13 years ago.
Updated patch (missed one)
16858.diff (1.1 KB) - added by dd32 13 years ago.

Download all attachments as: .zip

Change History (24)

#1 @hakre
13 years ago

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

Thanks for reporting the issue.

FWIK the problem you've reported is already known but we were not able to fix this for all installations.

There is however more information in Ticket #9235 that contains some suggestions and code snippets as well to deal with a setup similar to yours.

I therefore close this ticket for now as the solution you put into your issue description is the same suggestion as given in #9235.

#2 @dd32
13 years ago

  • Keywords needs-patch added
  • Resolution duplicate deleted
  • Status changed from closed to reopened

#9235 deals with the IP of the user accessing the site being "wrong" behind a load balancer. This here is a URL being crafted ignoring the rest of the WordPress API (ie. site_url() and friends)

class-wp-list-table.php should really be doing something like $current_url = self_admin_url($_SERVER['REQUEST_URI']); - that however will not work verbatim, as REQUEST_URI will include /wp-admin/ and the list tables may be used in multiple environments..

#3 @hakre
13 years ago

Yes a function is missing inside core that is able to retrieve the current admin page from request that can be used with the $path parameter of self_admin_url().

self_admin_path() ?

If I read #9235 right, then a function that builds anything close to the return value of get_site_url() or get_admin_url() so to be compared against anything and re-use the remainder as $path does not exists and is highly likely not to be written in the future.

However I don't want to suggest to close this ticket as wontfix. Any idea how this could be solved?

#4 @hakre
13 years ago

  • Keywords has-patch reporter-feedback added; needs-patch removed

I've attached a patch that did work on my testbed. It makes use of self_admin_url() and an additional function I named self_admin_path() which returns the $path component based on the current request.

@amirhabibi - please apply that patch on your host (the patch is against current trunk) and see if it solves your problem.

@hakre
13 years ago

Updated patch (missed one)

#6 follow-up: @amirhabibi
13 years ago

Very nice work !

The patch seems to solve the problem on our host.

When testing, I saw the same problem somewhere else :

  • the redirection when trying to preview a draft post (the link is good, the redirection not).

Again, by forcing the HTTP_HOST it works.

#7 @hakre
13 years ago

Replying to amirhabibi:

Very nice work !

The patch seems to solve the problem on our host.

Thanks for the feedback.

When testing, I saw the same problem somewhere else :

  • the redirection when trying to preview a draft post (the link is good, the

redirection not).

I'll take a look if this can be solved with the same approach.

#8 in reply to: ↑ 6 @hakre
13 years ago

Replying to amirhabibi:

When testing, I saw the same problem somewhere else :

  • the redirection when trying to preview a draft post (the link is good, the redirection not).

I have problems to locate the problem. Can you share the link that "is good"?

#9 follow-up: @amirhabibi
13 years ago

In fact it's seems more weird.

The "good" link is "http://www.my-visible-http-host.com/?p=164&preview=true". We use permalinks, but in a preview of a draft it's normal.

When we point to that link, we get an infinite loop of 301 redirects (firebug).

I think the problem is somewhere else.

To understand, I tried something like "http://www.my-visible-http-host.com/?p=154" (a normal, visible post). There was a 301 redirect to the pretty permalink structure, then everything ok.

#10 in reply to: ↑ 9 ; follow-up: @hakre
13 years ago

Replying to amirhabibi:

In fact it's seems more weird.

The "good" link is "http://www.my-visible-http-host.com/?p=164&preview=true". We use permalinks, but in a preview of a draft it's normal.

That's normal, preview links have always this format.

When we point to that link, we get an infinite loop of 301 redirects (firebug).

I think the problem is somewhere else.

I have the feeling that what you describe for the preview redirect is another issue. Please open a new ticket for it and leave the new ticket number here. Looks like something is incompatible on your setup with the wordpress preview feature, probably because of your host configuration.

However, just today I've updated my Better HTTP Redirect Plugin that now contains a debug mode with which you can look what happens behind the scenes in your redirect loop (grab the 1.2.1-beta-1 version).

#11 @hakre
13 years ago

See wp_requested_url() in ticket:8593:8593.2.patch, might be helpful for such cases esp. if we make it filtereable.

#12 follow-up: @dd32
13 years ago

I don't see a need for these links to be absolute url's, most links in the WordPress admin are relative (ie. just edit.php?..)

We should be able to just call add_query_arg() / remove_query_arg() without the $url parameter first.

However, Looking at the other uses of HTTP_HOST in WordPress, if it's not being corrected afterwards, a lot of login related redirections will fail as well.

@dd32
13 years ago

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

Replying to dd32:

I don't see a need for these links to be absolute url's, most links in the WordPress admin are relative (ie. just edit.php?..)

I don't see it either, had running this quite fine: 16932-hotfix-path-mapping.patch

We should be able to just call add_query_arg() / remove_query_arg() without the $url parameter first.

I like the idea and the patch.

However, Looking at the other uses of HTTP_HOST in WordPress, if it's not being corrected afterwards, a lot of login related redirections will fail as well.

They mostly do absolute URIs already as far as I know, the ones which do not, I put on #16909. Absolute URIs normally resolves through the _site_url() which are bound to the setting, not the environment variable and therefore safe.

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

#14 @techgurufloyd
13 years ago

My server is behind a couple of reverse proxies - one sends it to the chache, the other to the app server farm when the cache doesn't have the request. HTTP_HOST will never be accurate in that environment. Shouldn't WordPress use SERVER_NAME instead? Or perhaps offer an option to use SERVER_NAME or HTTP_X_FORWARDED_HOST instead of HTTP_HOST? There's also the option of using the server name I put in the WP options.

The ugly hack I'm using in wp-config right now to get around this:

//UGLY HACK:
$_SERVER['HTTP_HOST_ORIG'] = $_SERVER['HTTP_HOST']; //Might as well keep it around, but whatever.
$_SERVER['HTTP_HOST'] = $_SERVER['SERVER_NAME'];    //The important part.

Would WordPress be open to some kind of configuration option that selects which Header to use? If so, I can generate a patch that replaces every instance of HTTP_HOST with something that uses such an option.

Another idea would be to set $_SERVER['WP_HOST'] to be whichever header is chosen, and do a quick sed to change HTTP_HOST to WP_HOST. That still feels hackish, but it would work. Whatever solution is chosen, I'm looking for something that gets in the core.

#15 in reply to: ↑ 10 @amirhabibi
13 years ago

Replying to hakre:

I have the feeling that what you describe for the preview redirect is another issue. Please open a new ticket for it and leave the new ticket number here. Looks like something is incompatible on your setup with the wordpress preview feature, probably because of your host configuration.

Yes. You were right. The preview problem was caused by the Squid server refusing cookies.

Thanks a lot for all ;-)

#16 @tobias
12 years ago

  • Cc t@… added

This ticket is a duplicate or strongly related to http://core.trac.wordpress.org/ticket/18944#comment:7

And at http://core.trac.wordpress.org/ticket/17168#comment:15 I suggested not to use HTTP_HOST in the core at all but to switch to a variable that is configured in wp-config at one central place.

#17 @DrewAPicture
9 years ago

  • Owner set to dd32
  • Status changed from reopened to reviewing

@dd32: Care to followup here?

#18 @chriscct7
8 years ago

  • Keywords reporter-feedback removed

#19 @sumuga
3 years ago

Does this patch still work ? I was checking the class WP_List_Table in 5.5.3 and the changes in this patch does not seem to be available there.

#20 @desrosj
21 months ago

  • Keywords needs-refresh added
  • Milestone set to Awaiting Review

Re-adding a milestone to this one.

The most recent patch also fails to apply cleanly to current trunk, so needs a refresh.

#21 @piotrczapla2jb
20 months ago

I would like to add that this issue is affecting WordPress running under github codespaces, which is quite unfortunate as codespaces could be the easiest way to work with WordPress. Here is the specific issue: https://github.com/jungleboogie-pl/vscode-wordpress/issues/2

When executed under codespaces WordPress thinks it is running under https://localhost as it does not respect $_SERVER['HTTP_X_FORWARDED_HOST'].

The code that cause issues is here:
wp-admin/includes/misc.php, line 1349

And it seems it is being used verbatim in a few other places:
GitHub search (nav-menu-template.php, class-wp-list-table.php and misc.php)

Last edited 20 months ago by sabernhardt (previous) (diff)

#22 @sabernhardt
20 months ago

Related (possibly duplicate): #53998

Note: See TracTickets for help on using tickets.