Opened 4 years ago

Closed 4 years ago

Last modified 2 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 4 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 4 years ago by Tim Graham

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 Changed 4 years ago by Carl Meyer

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 Changed 4 years ago by Aymeric Augustin

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 Changed 4 years ago by Tim Graham

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.

Changed 4 years ago by Tim Graham

Attachment: 23960.diff added

comment:5 Changed 4 years ago by Claude Paroz

Owner: changed from nobody to Claude Paroz
Status: newassigned

comment:6 Changed 4 years ago by Collin Anderson

This might fix #17092

comment:7 Changed 4 years ago by Tim Graham

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

PR from Claude looks good.

comment:8 Changed 4 years ago by Claude Paroz <claude@…>

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 Changed 3 years ago by Danilo Bargen

Version: 1.71.9

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

comment:10 Changed 3 years ago by Aymeric Augustin

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

comment:11 Changed 3 years ago by Danilo Bargen

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 Changed 3 years ago by Tim Graham <timograham@…>

In 08b8c469:

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

comment:13 Changed 3 years ago by Tim Graham <timograham@…>

In 6977eaf8:

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

Backport of 08b8c4697112e8dae90e72afc7d85bd31ead0410 from master

comment:14 Changed 3 years ago by Tim Graham <timograham@…>

In f0bf535:

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

Backport of 08b8c4697112e8dae90e72afc7d85bd31ead0410 from master

comment:15 Changed 2 years ago by Tim Graham <timograham@…>

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