Opened 3 years ago

Closed 11 months ago

Last modified 11 months ago

#18558 closed New feature (fixed)

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

Reported by: coolRR Owned by: claudep
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 jezdez)

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 3 years ago by coolRR

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Sorry for the butchered Python, I meant:

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

comment:2 Changed 3 years ago by jezdez

  • Description modified (diff)
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to New feature

Formatting updates.

comment:3 Changed 3 years ago by coolRR

What is needed now to add this to Django?

comment:4 Changed 3 years ago by hirokiky

  • Cc hirokiky@… added

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

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

comment:5 Changed 3 years ago by claudep

  • Needs documentation set
  • Patch needs improvement set

comment:6 Changed 3 years ago by hirokiky

  • Owner changed from nobody to hirokiky
  • Status changed from new to assigned

comment:7 Changed 3 years ago by anonymous

  • Needs documentation unset
  • Patch needs improvement unset

I added the documentaiton and tests.

comment:8 Changed 3 years ago by claudep

  • Owner changed from hirokiky to claudep
  • Triage Stage changed from Accepted to Ready for checkin

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

  • Resolution set to fixed
  • Status changed from assigned to closed

In e94f405d9499d310ef58b7409a98759a5f5512b0:

Fixed #18558 -- Added url property to HttpResponseRedirect*

Thanks coolRR for the report.

comment:10 Changed 3 years ago by hirokiky

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

comment:11 Changed 3 years ago by coolRR

Thank you hirokiky and claudep!

comment:12 Changed 11 months 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 11 months ago by tevans

  • Resolution fixed deleted
  • Status changed from closed to new

comment:14 Changed 11 months ago by claudep

  • Resolution set to fixed
  • Status changed from new to closed

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 11 months 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