301 vs 308 in default redirect class for CommonMiddleware

158 views
Skip to first unread message

Matthieu Bonnefoy

unread,
Sep 18, 2018, 7:24:49 AM9/18/18
to Django developers (Contributions to Django itself)
Hi there,

I am wondering why the default redirect class in the common middleware is a 301 Moved Permanently response.

I just got the issue of a POST request being changed as a GET request by the redirect and found it quite confusing.
It seems that a 308 Permanent Redirect (by the way the name of the redirect subclass is now a bit confusing) is now widely supported and would be a better option.

   +-------------------------------------------+-----------+-----------+
   |                                           | Permanent | Temporary |
   +-------------------------------------------+-----------+-----------+
   | Allows changing the request method from   | 301       | 302       |
   | POST to GET                               |           |           |
   | Does not allow changing the request       | 308       | 307       |
   | method from POST to GET                   |           |           |
   +-------------------------------------------+-----------+-----------+

What do you think?

Thanks,
Matthieu

Adam Johnson

unread,
Sep 18, 2018, 7:55:17 AM9/18/18
to django-d...@googlegroups.com
This is historical, 307 and 308 were added later, and I think we couldn't change it without breaking backwards compatibility. That said, 307 and 308 classes could be added to django.http.response and the documentation could recommend their use. Mozilla docs show that they're compatible with all tracked web browsers so most Django sites could use them.

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/a9f807b8-d31c-4729-a9bc-ac80274d4590%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


--
Adam

Matthieu Bonnefoy

unread,
Sep 23, 2018, 1:56:18 AM9/23/18
to Django developers (Contributions to Django itself)
Hi Adam,

Thanks for your reply.

I agree, I think that would be a nice addition. The issue we’ll have is that the current naming of the HttpResponsePermanentRedirect (301 Moved Permanently) will be confusing.

existing
- HttpResponsePermanentRedirect (301 Moved Permanently) => should probably be HttpResponseMovedPermanently
- HttpResponseRedirect (302 Found) => should probably be HttpResponseFound

new additions
- HttpResponseTemporaryRedirect (307 Temporary Redirect) => probably ok with the existing naming for 302
- ? (308 Permanent Redirect) ==> naming conflict with the existing 301, HttpResponsePermanentRedirect308? HttpResponsePermanentRedirection? HttpResponsePermanentlyRedirected? … not super convinced by these suggestions. What do you think?

I could add the new responses and mention them in the documentation here with the recommendation
https://docs.djangoproject.com/en/2.1/ref/request-response/#httpresponse-subclasses

We could also mention the 308 redirect at the end of the class docstring here
https://docs.djangoproject.com/en/2.1/_modules/django/middleware/common/

Cheers,
Matthieu

Adam Johnson

unread,
Sep 26, 2018, 6:03:44 PM9/26/18
to django-d...@googlegroups.com
I don't think it's worth the deprecation path to rename the old classes - it doesn't seem like there's a substantial benefit to using the different codes and getting everyone to change their imports is a lot of work to impose on the world.

There must be some name for the 308 one that would work, but I can't think of one off the top of my head.

The new class could also be mentioned in the 301/302 classes' docstrings.


For more options, visit https://groups.google.com/d/optout.


--
Adam
Reply all
Reply to author
Forward
0 new messages