Opened 12 years ago

Closed 10 years ago

Last modified 10 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: dev
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 by Ram Rachum, 12 years ago

Sorry for the butchered Python, I meant:

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

comment:2 by Jannis Leidel, 12 years ago

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

Formatting updates.

comment:3 by Ram Rachum, 12 years ago

What is needed now to add this to Django?

comment:4 by Hiroki Kiyohara, 12 years ago

Cc: hirokiky@… added

https://github.com/django/django/pull/657

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

comment:5 by Claude Paroz, 12 years ago

Needs documentation: set
Patch needs improvement: set

comment:6 by Hiroki Kiyohara, 12 years ago

Owner: changed from nobody to Hiroki Kiyohara
Status: newassigned

comment:7 by anonymous, 12 years ago

Needs documentation: unset
Patch needs improvement: unset

I added the documentaiton and tests.

comment:8 by Claude Paroz, 12 years ago

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

comment:9 by Claude Paroz <claude@…>, 12 years ago

Resolution: fixed
Status: assignedclosed

In e94f405d9499d310ef58b7409a98759a5f5512b0:

Fixed #18558 -- Added url property to HttpResponseRedirect*

Thanks coolRR for the report.

comment:10 by Hiroki Kiyohara, 12 years ago

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

comment:11 by Ram Rachum, 12 years ago

Thank you hirokiky and claudep!

comment:12 by tevans, 10 years ago

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 by tevans, 10 years ago

Resolution: fixed
Status: closednew

comment:14 by Claude Paroz, 10 years ago

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 by tevans, 10 years ago

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