[Django] #24145: Extend POST redirect error to be raised on PUT and PATCH
Django
-----------------------------------+--------------------
Reporter: samuelcolvin | Owner: nobody
Type: New feature | Status: new
Component: contrib.redirects | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------
As per https://github.com/tomchristie/django-rest-framework/issues/2405
discussion it would be extremely helpful if the exception raised when POST
requests require a redirect
raise RuntimeError((""
"You called this URL via POST, but the URL doesn't end "
"in a slash and you have APPEND_SLASH set. Django can't "
"redirect to the slash URL while maintaining POST data. "
"Change your form to point to %s%s (note the trailing "
"slash), or set APPEND_SLASH=False in your Django "
"settings.") % (new_url[0], new_url[1]))
was extended to PUT and PATCH requests. PUT and PATCH requests have
associated data which is generally lost with a redirect.
Code:
https://github.com/django/django/blob/1.7.2/django/middleware/common.py#L75
--
Ticket URL: <https://code.djangoproject.com/ticket/24145>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: samuelcolvin | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
------------------------------+------------------------------------
Changes (by tomchristie):
* cc: tom@… (added)
* needs_better_patch: => 0
* component: contrib.redirects => Core (Other)
* needs_tests: => 0
* easy: 0 => 1
* needs_docs: => 0
* type: New feature => Bug
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24145#comment:1>
Django
Reporter: samuelcolvin | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
------------------------------+------------------------------------
Comment (by tomchristie):
I'm going to second this one. Our options are:
* Extend the check to also include `PUT` and `PATCH` and continue issuing
the 500 response.
* Extend the check to also include `PUT` and `PATCH`, but use an 307 (or
308) redirect, which is spec'ed as not allowing a rewrite-to-GET request.
See: http://trac.tools.ietf.org/html/draft-ietf-
httpbis-p2-semantics-21#section-7.4.7
Of these, the first is the lesser change. It's unclear to me how widely
supported 307/308 responses are supported by browsers and other clients.
--
Ticket URL: <https://code.djangoproject.com/ticket/24145#comment:2>
Django
Reporter: samuelcolvin | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
------------------------------+------------------------------------
Comment (by samuelcolvin):
Since it was easy I took the chance to try and get myself on the list of
django contributors: https://github.com/django/django/pull/3908
--
Ticket URL: <https://code.djangoproject.com/ticket/24145#comment:3>
Django
Reporter: samuelcolvin | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
------------------------------+------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/24145#comment:4>
Django
Reporter: samuelcolvin | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
------------------------------+------------------------------------
Comment (by arcturusannamalai):
@samuelcolvin please assign this geck to yourself. It is seems to have a
solution you had developed, but looks as if it is not triaged.
--
Ticket URL: <https://code.djangoproject.com/ticket/24145#comment:5>
Django
Reporter: samuelcolvin | Owner: samuelcolvin
Type: Bug | Status: assigned
Component: Core (Other) | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by samuelcolvin):
* status: new => assigned
* owner: nobody => samuelcolvin
--
Ticket URL: <https://code.djangoproject.com/ticket/24145#comment:6>
Django
Reporter: samuelcolvin | Owner:
| samuelcolvin
Type: Bug | Status: assigned
Component: Core (Other) | Version: 1.7
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by berkerpeksag):
* stage: Accepted => Ready for checkin
Comment:
[https://github.com/django/django/pull/3908/ PR #3908] LGTM.
--
Ticket URL: <https://code.djangoproject.com/ticket/24145#comment:7>
Django
Reporter: samuelcolvin | Owner: samuelcolvin
Type: Bug | Status: assigned
Component: Core (Other) | Version: 1.7
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* needs_tests: 0 => 1
* stage: Ready for checkin => Accepted
Comment:
I think some tests wouldn't hurt here (not sure if there's one for the
existing behavior).
--
Ticket URL: <https://code.djangoproject.com/ticket/24145#comment:8>
Django
Reporter: samuelcolvin | Owner:
| samuelcolvin
Type: Bug | Status: assigned
Component: Core (Other) | Version: 1.7
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by timgraham):
* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24145#comment:9>
Django
Reporter: samuelcolvin | Owner:
| samuelcolvin
Component: Core (Other) | Version: 1.7
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"5b74134f27eabf92870e1c5e81f9e4999f113eab"]:
{{{
#!CommitTicketReference repository=""
revision="5b74134f27eabf92870e1c5e81f9e4999f113eab"
Fixed #24145 -- Added PUT & PATCH to CommonMiddleware APPEND_SLASH
redirect error.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24145#comment:10>