Opened 8 years ago

Closed 6 years ago

Last modified 6 years ago

#18558 closed New feature (fixed)

Supply `url` property to `HttpResponseRedirect` and `HttpResponsePermanentRedirect`

Reported by: Ram Rachum Owned by: Claude Paroz
Component: HTTP handling Version: master
Severity: Normal Keywords:
Cc: hirokiky@… 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 (last modified by Jannis Leidel)

Currently the way to get the URL that a HttpResponseRedirect is redirecting to requires doing response['Location']. This is not so intuitive. There's no reason to remember the HTML header name when dealing with a redirect response.

Instead I propose this property:

url = property(lambda self: self['Location'])

Change History (15)

comment:1 Changed 8 years ago by Ram Rachum

Sorry for the butchered Python, I meant:

    url = property(lambda self: self['Location'])

comment:2 Changed 8 years ago by Jannis Leidel

Description: modified (diff)
Triage Stage: UnreviewedAccepted
Type: UncategorizedNew feature

Formatting updates.

comment:3 Changed 8 years ago by Ram Rachum

What is needed now to add this to Django?

comment:4 Changed 8 years ago by Hiroki Kiyohara

Cc: hirokiky@… added

I opened a pull-request. Fixed the implementation and tests.

comment:5 Changed 8 years ago by Claude Paroz

Needs documentation: set
Patch needs improvement: set

comment:6 Changed 8 years ago by Hiroki Kiyohara

Owner: changed from nobody to Hiroki Kiyohara
Status: newassigned

comment:7 Changed 8 years ago by anonymous

Needs documentation: unset
Patch needs improvement: unset

I added the documentaiton and tests.

comment:8 Changed 8 years ago by Claude Paroz

Owner: changed from Hiroki Kiyohara to Claude Paroz
Triage Stage: AcceptedReady for checkin

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

Resolution: fixed
Status: assignedclosed

In e94f405d9499d310ef58b7409a98759a5f5512b0:

Fixed #18558 -- Added url property to HttpResponseRedirect*

Thanks coolRR for the report.

comment:10 Changed 8 years ago by Hiroki Kiyohara

Thanks coolRR and claudep.
I am glad to contribute to Django at first time.

comment:11 Changed 8 years ago by Ram Rachum

Thank you hirokiky and claudep!

comment:12 Changed 6 years ago by tevans

Hi, I think this change is incorrect.

It makes HttpResponse(status=302, content='Foo') behave differently to HttpResponseRedirect(content='Foo').

This can be seen by using the test client, and asking it to follow redirects. It will attempt to access the url property on the response, but because the response is a redirect that is not derived from HttpResponseRedirectBase, this will fail.

comment:13 Changed 6 years ago by tevans

Resolution: fixed
Status: closednew

comment:14 Changed 6 years ago by Claude Paroz

Resolution: fixed
Status: newclosed

HttpResponseRedirect(content='Foo') is not valid, the redirect_to parameter being mandatory. I think that the proper answer would be not to use a plain HttpResponse for an HTTP redirection. If you have a valid use case for using an HttpResponse(status=302) instead of an HttpResponseRedirect, please open a new ticket (where you can reference this one).

comment:15 Changed 6 years ago by tevans

If creating a redirect manually from a HttpResponse is incorrect, why is it possible to create a HttpResponse with status 301, 302, 303 or 307?

The problem is that the test client uses both the API of HttpResponse to check whether the response is a redirect or not, and then it uses the API of HttpResponseRedirectBase to determine the redirection location. You think that is not a bug?

It only comes around because someone has decided that knowing the URL for a redirect is in the Location header is needless knowledge, it is better to instead learn a distinct API just for this framework. Also, lets make it so that that API prevents previously working and still currently documented as working code from being treated correctly, and ignore bug reports on it because pseudo code didn't have all the arguments specified.

Yay progress!

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