Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#23960 closed Cleanup/optimization (fixed)

HTTP standard no longer requires the Location header to be an absolute URI

Reported by: Carl Meyer Owned by: Claude Paroz
Component: HTTP handling Version: 1.9
Severity: Normal Keywords:
Cc: mail@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

RFC 2616 required the Location header (in redirect responses) to be an absolute URI. In Django, we have django.http.utils.fix_location_header() to unconditionally ensure this.

RFC 2616 has now been superseded by RFC 7231, which allows relative URIs in Location (recognizing the actual practice of user agents, almost all of which support them): http://tools.ietf.org/html/rfc7231#section-7.1.2

We should remove django.http.utils.fix_location_header().

Since user agents almost universally allow relative Location (I'm not aware of any that don't), I don't believe this change requires a deprecation path, but it should of course be noted in the release notes.

Attachments (1)

23960.diff (4.1 KB ) - added by Tim Graham 10 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 by Tim Graham, 10 years ago

This was discussed on IRC I think, and Matt Robenolt drafted a patch using a setting to control the behavior -- not sure the justification for doing that over just dropping it as you've proposed.

comment:2 by Carl Meyer, 10 years ago

If we were going to continue to provide support for this in Django, it seems like a built-in middleware would make more sense than a dedicated setting.

But are there really good enough reasons to be doing this server-side that there needs to be built-in support in Django?

comment:3 by Aymeric Augustin, 10 years ago

I would just drop the unconditional fix and document the change in the release notes.

(In fact I consider all unconditional response_fixes to be response_breaks. We removed the two IE-specific ones in [3800f637]. I bet we'll end up removing them all.)

comment:4 by Tim Graham, 10 years ago

Has patch: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

I started this, but it seems it will take more than 5 minutes as the test client currently relies on always receiving an absolute URI. I haven't looked into what's required to fix it.

by Tim Graham, 10 years ago

Attachment: 23960.diff added

comment:5 by Claude Paroz, 10 years ago

Owner: changed from nobody to Claude Paroz
Status: newassigned

comment:6 by Collin Anderson, 10 years ago

This might fix #17092

comment:7 by Tim Graham, 10 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

PR from Claude looks good.

comment:8 by Claude Paroz <claude@…>, 10 years ago

Resolution: fixed
Status: assignedclosed

In a0c2eb46:

Fixed #23960 -- Removed http.fix_location_header

Thanks Carl Meyer for the report and Tim Graham for the review.

comment:9 by Danilo Bargen, 9 years ago

Version: 1.71.9

Changing the version to "1.9", as that's the release that first contains the fix.

comment:10 by Aymeric Augustin, 9 years ago

The version field usually represents the first version in which a bug is detected. It doesn't matter very much anyway.

comment:11 by Danilo Bargen, 9 years ago

Cc: mail@… added

Oops, I didn't realize that. Well, since it's not really a "bug" per se, it probably won't hurt in this case :) I'll try to remember for the future though.

comment:12 by Tim Graham <timograham@…>, 8 years ago

In 08b8c469:

Refs #23960 -- Documented how to restore absolute redirect URLs.

comment:13 by Tim Graham <timograham@…>, 8 years ago

In 6977eaf8:

[1.10.x] Refs #23960 -- Documented how to restore absolute redirect URLs.

Backport of 08b8c4697112e8dae90e72afc7d85bd31ead0410 from master

comment:14 by Tim Graham <timograham@…>, 8 years ago

In f0bf535:

[1.9.x] Refs #23960 -- Documented how to restore absolute redirect URLs.

Backport of 08b8c4697112e8dae90e72afc7d85bd31ead0410 from master

comment:15 by Tim Graham <timograham@…>, 8 years ago

In 0f454f5:

Refs #23960 -- Removed the host parameter for SimpleTestCase.assertRedirects().

Per deprecation timeline.

Note: See TracTickets for help on using tickets.
Back to Top