Make WordPress Core

Opened 14 years ago

Closed 11 years ago

#13909 closed defect (bug) (maybelater)

HTTP redirect should return a hypertext fragment

Reported by: hakre's profile hakre Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: General Keywords: has-patch
Focuses: Cc:

Description

When WordPress redirects via the API (wp_redirect), a useragent which does not automatically redirect (e.g. configuration because of usability or security considerations), get's a blank page delivered (which is of no use for the user). This is because only HTTP headers are set for the redirect, but not a HTTP body. Normally Webservers and Webapplications deliver a body as well.

Example: Redirect on the google.com domain

Command: curl -i http://google.com/

Output:

HTTP/1.1 301 Moved Permanently
Location: http://www.google.com/
Content-Type: text/html; charset=UTF-8
Date: Tue, 15 Jun 2010 18:11:12 GMT
Expires: Thu, 15 Jul 2010 18:11:12 GMT
Cache-Control: public, max-age=2592000
Server: gws
Content-Length: 219
X-XSS-Protection: 1; mode=block

<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://www.google.com/">here</A>.
</BODY></HTML>

Redirect of google.com to www.google.com; next to the headers a body is returned (some simple HTML containing the link to the redirected page).

This method is often used as a fall-back for automatic methods — if the visitor's browser does not support the automatic redirect method, the visitor can still reach the target document by following the link. URL redirection (From Wikipedia, the free encyclopedia)

Example: Redirect on a wordpress domain

This is against a trunk version wordpress setup. I made a setup with a domain www.webroot.loc and webroot.loc. The blog is w/o the www. so www.webroot.loc get's redirected to webroot.loc:

Command: curl -i http://www.webroot.loc/wordpress/

Output:

HTTP/1.1 301 Moved Permanently
Date: Tue, 15 Jun 2010 18:12:38 GMT
Server: Apache
X-Pingback: http://webroot.loc/wordpress/xmlrpc.php
Location: http://webroot.loc/wordpress/
Transfer-Encoding: chunked
Content-Type: text/html; charset=UTF-8

There is no http body in the response.

To increase the overall usability of wordpress a body should returned as well.

Attachments (4)

13909.patch (1.7 KB) - added by hakre 14 years ago.
Better HTTP 1.1 redirect - HTML in HTTP-Body comparable to Google;
13909.2-301-redirect-example.jpg (19.8 KB) - added by hakre 14 years ago.
display of the example hypertext implemetation in wp_redirect_hypertext()
13909.2.2.patch (4.8 KB) - added by hakre 14 years ago.
full flavored implementation (corrected; 5)
13909.2.patch (4.8 KB) - added by hakre 14 years ago.
full flavored implementation (corrected; 5)

Download all attachments as: .zip

Change History (22)

#1 @hakre
14 years ago

  • Version set to 3.0

In this patch, the fix is to provide a simple body as well. I've taken google.com as an example and implemented that HTML fragment.

#2 @hakre
14 years ago

  • Keywords tested added

With the patch applied I get the following response:

Command: curl -i http://www.webroot.loc/wordpress/

Output:

HTTP/1.1 301 Moved Permanently
Date: Tue, 15 Jun 2010 19:17:51 GMT
Server: Apache
X-Pingback: http://webroot.loc/wordpress/xmlrpc.php
Location: http://webroot.loc/wordpress/
Transfer-Encoding: chunked
Content-Type: text/html; charset=UTF-8

<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="http://webroot.loc/wordpress/">here</A>.
</BODY></HTML>

#3 @hakre
14 years ago

After reading some papers about redirects and for what they can be mis-used, this might be considered as a valid insecurity countermeasure and a site usability improvement.

#4 @markjaquith
14 years ago

Do you have some examples of user agents that don't follow 3xx redirects?

#5 @hakre
14 years ago

Opera.

http://help.opera.com/Windows/10.53/en/network.html

"Enable automatic redirection"

You can disable automatic redirection. A user with this setting disabled for security reasons sees a blank page with wordpress which degrades the usability of the wordpress based website.

#6 @hakre
14 years ago

Firefox (per Addon)

#7 follow-up: @dd32
14 years ago

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

This will not fare well in its current state.

Take a Plugin activation command for example, Current workflow is Request->Redirect(error occured)->Activate plugin->Redirect(final activated)->End of process -> Redirect occurs to the last url

With this patch, Output will be generated after the first redirect, causing the 2nd redirect to fail (as headers will already be sent).

For front end queries it may make sense to offer some form of redirection text, I'm not supporting it on the back end however, Its an extra level of complication which just isn't needed.

A user disabling their redirection handling is bound to suffer because of their choices, Opera for example, generates a content near exact to that example HTML you provided transparently when it blocks a redirection. Its the browsers option to ignore Redirects, Its the browsers responsibility to handle that redirect..

Moving this out of 3.1 to Future Release, pending a commitor taking this for 3.1

#8 in reply to: ↑ 7 @hakre
14 years ago

Replying to dd32:

This will not fare well in its current state.

Take a Plugin activation command for example, Current workflow is Request->Redirect(error occured)->Activate plugin->Redirect(final activated)->End of process -> Redirect occurs to the last url

With this patch, Output will be generated after the first redirect, causing the 2nd redirect to fail (as headers will already be sent).

Good point, I tested that now. It creates fatal errors because of redeclaration(functions and classes if the plugin defines those and which plugin does not, right?).

But it reveals as well that plugin activation does not work with manual redirects. That might seem odd, but I'll add more details below.

For front end queries it may make sense to offer some form of redirection text, I'm not supporting it on the back end however, Its an extra level of complication which just isn't needed.

It even should be added. I'll explain that below. But you're right, I put this on the agenda because of front end usage.

A user disabling their redirection handling is bound to suffer because of their choices,

Do not blame the user if you do not have a valid reason I tend to say. In this case, it's not the user to blame for and I will show that below.

Opera for example, generates a content near exact to that example HTML you provided transparently when it blocks a redirection.

The generated page by Opera you write about - even I know it - I do not have it in 10.53 in my tests reg. this ticket. But that's something to discuss in the Opera forums and should not be the point here. As well as it shouldn't be the point to discuss how any specific browser is implementing redirects. That's web done wrong.

Its the browsers option to ignore Redirects, Its the browsers responsibility to handle that redirect..

Instead of assuming stuff I suggest to talk about HTTP now. I made that error in the first place as well, but I'm willing to correct that now.


So my apologies: I did not read the specification before implementing my version of the redirect. That's mainly because I approached this from the usability side, by a personal experience. And then I compared it to only google w/o taking a deeper look why google or apache might do it that way and not in another way. dd32 comments let me investigate that further. So this is where I am now:

The standard this is based on is HTTP 1.1 / RFC 2616 (and somehow even HTTP 1.0 / RFC 1945 but I leave this out for the moment).

The specification is pretty straight forward about redirects and what SHOULD/MUST/CAN a browser do on the various response codes. Generally spoken, wordpress can not take it for granted that a redirect is carried out in response of a GET request:

The action required MAY be carried out by the user agent without interaction with the user

(10.3 Redirection 3xx - RFC 2616)

That is a may. And further for 302 responses:

the entity of the response SHOULD contain a short hypertext note with a hyperlink to the new URI(s).

(10.3.3 302 Found - RFC 2616)

Note the should here.

Therefore: A webapplication can not expect that a user-agent automatically redirect (because it's completely optional or "truly optional" as the RFCs specify it). So it makes sense to provide a short hypertext note with a hyperlink to the new URI in case of a 302 redirect. Otherwise the user is left alone.

So what does that mean?

a) The redirect like currently done in WP_Redirect does not reflect the SHOULD in HTTP 1.1 for 302 redirects properly.

b) Same for my patch. I must update my patch to go conform with 302 etc. .

c) Automatic redirects are truly optional on GET and a webapplication should deal properly with that if it aims for being conform with HTTP 1.1.

That for a start. I'll update the patch and continue my read on the RFC to learn more. Maybe there is a solution by using propper status codes and this can be easily solved while gaining a better conformity with HTTP 1.1 as well. I think especially "c)" is the hardest to deal with.

@hakre
14 years ago

Better HTTP 1.1 redirect - HTML in HTTP-Body comparable to Google;

#9 @hakre
14 years ago

I just updated the patch. It does now better reflect HTTP 1.1 but it does not solve the problems with plugin activation.

Despite the error message, plugin activation actually works. It somehow differs based on the browser you use with this (and even the previous) patch.

So Plugin activation works with manual redirects (even when opera generates the message [I have it now again] and even when the patch generates the hypertext).

I think about an additional parameter on the redirect function to enable that output. That could make the introduction more failsafe.

#10 @hakre
14 years ago

While doing more HTTP tests I found out that plugins doing output on activation break the redirects for deactivating plugins as well (if the UA does not redirect w/o user interaction). That is a side-effect of HTTP 1.1 / Header only redirects while the plugin creates the HTTP body (by accident). Related: #12089

#11 @hakre
14 years ago

Related: #13955

#12 @hakre
14 years ago

  • Keywords has-patch added; needs-patch removed

To summarize since this is now some says older:

  • HTTP specifies a hypertext fragment should be output on redirects (Status Codes 3xx) - WordPress does not conform with that currently.
  • Doing so globally would break the Plugin activation functionality because it operates with overwriting headers in case of not crashing.
  • WP_Redirect does handle even non-redirect stati. That's violating HTTP specs, valid redirect status codes have to be in the 300-399 range.

So WP_Redirect must be put in context when used to redirect. Attached patch reflects that by introducing a new parameter to control hypertext output, a filter to let plugins control that as well as a introducing a new pluggable function and a new filter to offer control for the redirect hypertext design.

Additionally the function has been improved to prevent wrong-filtered status codes to accidently used as those are not releated to redirects (non 3xx codes handling).

@hakre
14 years ago

display of the example hypertext implemetation in wp_redirect_hypertext()

@hakre
14 years ago

full flavored implementation (corrected; 5)

@hakre
14 years ago

full flavored implementation (corrected; 5)

#13 @hakre
14 years ago

Related: #14062

#14 @hakre
14 years ago

  • Summary changed from Redirect does not return a HTTP body to HTTP redirect should return a hypertext fragment

#15 @hakre
14 years ago

  • Component changed from General to HTTP

#16 @hakre
14 years ago

To develop further I've now created a plugin that deals with it properly. It can be used as a Must Use or as a normal one but needs PHP 5 to work.

PoC: Better HTTP Redirects (WordPress Plugin), I'm starting to deploy this on some sites.

#17 @voyagerfan5761
14 years ago

  • Cc WordPress@… added

#18 @dd32
11 years ago

  • Component changed from HTTP to General
  • Milestone Future Release deleted
  • Resolution set to maybelater
  • Status changed from new to closed

Due to lack of traction, the side effects it would have upon WordPress, and the general ability of HTTP clients to handle this scenario themselves, I don't think this is something we need right now.

Note: See TracTickets for help on using tickets.