HttpResponseRedirect uses/allows relative URIs for the HTTP Location header, which is forbidden by HTTP

[henryk@…]

Moin,

Using the HttpResponseRedirect class (for example as in part 4 of the Django tutorial) may create a Location: header in the HTTP response with a relative URI, which is forbidden by RFC 2616, section 14.30 (an absolute URI is required). Call me a perfectionist, but I think that should be fixed.

The best solution would probably be to automatically create an absolute URI if the URI that is used to create the Redirect is a relative one, so that the apps don't have to deal with it (and stay decoupled from the project). Alternatively update the tutorial and docs with information on how to create an absolute URI.

-- 
Henryk Plötz
Grüße aus Berlin

[ian@…]

while relative redirects might not be in the HTTP spec, most browsers work with them.

which hostname do you propose to use for the absolute URI?
django is not really in a position in most cases to know what the right one is, unless your application can figure it out, and in some cases can not be determined when django gets the request.

[hugo]

The host to use could just be sites.get_current().domain - that's what it's there for, to get the current running site (based on the SITE_ID setting).

[Adrian Holovaty]

Just one small problem with using sites.get_current().domain -- it assumes the developer has edited the sites table. For something as core as redirects, I'm not sure we would want to assume that, because it could confuse people.

[henryk@…]

Moin,

one easy way to get developers to edit the site table is to put information and recommendation on it into the tutorial. I for one don't know anything about it yet and didn't see any obvious documentation on how to use it. Another possibility: the URI scheme (http or https) and host name should be deducible from the WSGI environment.

So a good plan is: Leave the site unconfigured in the default configuration and take the values that the browser provided (and maybe even log a warning once in a while). As soon as a site and base URI are correctly configured these settings should override the browser provided values, because the configured values presumably give the canonical URI. That's also basically the way Apache handles the issue.

Related issue: There should be a dead easy and clearly documented way to retrieve the absolute URI of the current site or even app, and a relative URI to the root of the current site/app.

-- 
Henryk Plötz
Grüße aus Berlin

[tbarta@…]

adding self to cc list

[tyson@…]

I agree with Henryk, there should be a clearly defined method to retrieve the abosulute URI for the the current site/app, using the same logic that the Apache webserver uses to resolve the server name.

HttpResponseRedirect should parse redirect_to and determine if it is absolute or relative. If relative, use os.path.join and os.path.normpath or similar in conjunction with the method specified by Henryk to create the absolute URI.

FYI, Safari is a picky browser that (correctly) barfs on relative URIs.

[Simon G. <dev@…>]

I've made a patch which checks if the sites framework has been changed from its default setting, if yes, then HttpResponseRedirect will return an absolute URL. If NOT, then we'll keep the relative redirect.

[Simon G. <dev@…>]

Note that I'm not sure what the best thing to do when there's multiple sites installed - I've never used them.

[Simon G. <dev@…>]

Here's a patch that takes into account the SITE_ID setting in the settings.py, but this may be getting too dependant on settings etc.

[forest@…]

Added self to Cc.

[Chris Beaven]

Forget about using the optional sites. If we use a piece of response middleware we can get the scheme and host from the request.

Doesn't this make more sense?

[Malcolm Tredinnick]

Getting this right is not an optional feature, so I think it should be in the core message handling path, not something you might accidentally remove if you remove the middleware. It's not compulsory to have CommonMiddleware installed.

[Chris Beaven]

Fair enough, but it is still a much better fit than relying on sites. I'm thinking perhaps (and had actually originally implemented before I joined it with ComonMiddleware) we could have a compulsary_middleware property of the BaseHandler which contained some processors that were always enabled and activated before any ones from settings.MIDDLEWARE_CLASSES - I think the streaming upload was going to need a "compulsary" middleware too.

[Chris Beaven]

Ok, here's a take on the same thing without using the CommonMiddleware. Comes with tests this time.

This patch does two things, but since the second relies on the first I'm naughtily submitting them together.

1. New function request.build_absolute_uri(location)

2. New BaseCommonMiddleware attached to the BaseHandler

[Chris Beaven]

Actually, the patch calls it BaseCompulsaryMiddleware, but I like the name BaseMandatoryMiddleware more now I think about it.

See also: #4986, which will make get_host a bit more functional too.

[Ludvig Ericson <ludvig.ericson@…>]

Sorry if I'm being blatantly ignorant, but kind sires, can we not simply try to find the HTTP Host header, which is in fact mandatory in HTTP 1.1, and most HTTP 1.0 clients send it anyway? If it doesn't exist, then the client leaves us no choice but to violate the HTTP RFCs. Not entirely sure how this would be done with Django, but the concept is there.

On a slightly related note, I don't get why you need absolute URLs, what bothers me most is the fact that you have to specify the protocol (scheme if you will) - makes it even harder as we have to specify HTTP vs. HTTPS.

+add self to Cc

[Chris Beaven]

Replying to Ludvig Ericson <ludvig.ericson@gmail.com>:

Sorry if I'm being blatantly ignorant, but kind sires, can we not simply try to find the HTTP Host header, which is in fact mandatory in HTTP 1.1, and most HTTP 1.0 clients send it anyway?

Did you read my patch? That's pretty much what it does.

If it doesn't exist, then the client leaves us no choice but to violate the HTTP RFCs. Not entirely sure how this would be done with Django, but the concept is there.

Regarding getting it, get_host is how it is done in Django. And #4986 will make it a bit smarter still to fall back if the HTTP Host isn't provided.

On a slightly related note, I don't get why you need absolute URLs, what bothers me most is the fact that you have to specify the protocol (scheme if you will) - makes it even harder as we have to specify HTTP vs. HTTPS.

We're just trying to follow the HTTP spec, and this is the wrong place to contest that ;). If you check out my patch, you'll see that it's all handled pretty well (and automatically).

[Thomas Güttler <hv@…>]

Please fix this. Redirects must be absolute.

I get very strange results with Apache and SCGI:
Redirects after POST: The URL in the browser (firefox 1.5.04) does
not change, but the content of the post result gets displayed.

(See also ​http://barryp.org/blog/feeds/atom/scgi/)

[James Bennett]

Chris is there a reason why you wrote this as a middleware instead of just putting the functionality directly into Django's HTTP handler? It's kind of confusing to have a middleware which nonetheless forces itself to be always enabled...

[Thomas Güttler <hv@…>]

The patch redirect_absoluteuri.patch works for me. The SCGI redirect problems are gone.
I looked at the source and I think it could be commited.

redirect_absoluteuri.2.patch builds an own middleware. I think that is not necessary.
Absolute redirects should be enabled by default.

The patches from Simon G. don't work, since they only work for http, but not for https.

[Chris Beaven]

Replying to ubernostrum:

Chris is there a reason why you wrote this as a middleware instead of just putting the functionality directly into Django's HTTP handler? It's kind of confusing to have a middleware which nonetheless forces itself to be always enabled...

1. Logic abstraction

2. Ability to easily extend/override in a HTTP Handler subclass (probably YAGNI, I know)

It's only confusing if you're looking at source - I wouldn't bother putting it in documentation.

[Chris Beaven]

Thomas, the redirect shouldn't be optional. So the problem with just attaching it to the common middleware is that it can be disabled.

2.patch attaches the new middleware directly to Django's base HTTP handler - you don't manually attach it in your settings.

It also adds the cool new function request.build_absolute_uri(location) ;)

[Chris Beaven]

a rogue import snuck in there somehow

[Malcolm Tredinnick]

(In [6164]) Fixed #987 -- Convert relative URI portions into absolute URIs in HTTP Location headers. Based on a patch from SmileyChris.

[Michael Nelson]

Am I right in saying that since this change, all unit tests using something along the lines of:

self.assertRedirects(response, '/accounts/login/')

will currently need to include the actual site's url (or a settings variable) like:

self.assertRedirects(response, settings.HOME_URL + '/accounts/login/') (certainly seems to be the case for me!)?

If so, I'm guessing assertRedirects needs to be updated so that it only checks the path of the responseLocation (although this won't always be true either, for eg where people are using middleware subdomain mapping to do things like ​http://me.example.com/inbox/ -> ​http://example.com/u/me/inbox/ etc.).

Or is it better to leave as is so that the unit-tests are coupled to site? Thoughts?

Thanks for any feedback!
-Michael

[Michael Nelson]

Sorry, my mistake... I'd forgotten that I'm explicitly setting the HTTP_HOST header in my base test-case class so that I can do the middleware subdomain mapping mentioned above.

-Michael