[Django] #29364: CommonMiddleware.get_full_path_with_slash should raise exception for POST / PUT / PATCH requests even if settings.DEBUG = False
Django
PUT / PATCH requests even if settings.DEBUG = False
-----------------------------------------+------------------------
Reporter: nileshtrivedi | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 2.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-----------------------------------------+------------------------
https://docs.djangoproject.com/en/dev/_modules/django/middleware/common/#CommonMiddleware
Redirecting POST / PUT / PATCH to GET in should_redirect_with_slash() is
undesirable behavior even if APPEND_SLASH = True and DEBUG = False.
--
Ticket URL: <https://code.djangoproject.com/ticket/29364>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
PUT / PATCH requests even if settings.DEBUG = False
Reporter: Nilesh Trivedi | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: HTTP handling | Version: 2.0
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Tim Graham):
* component: Uncategorized => HTTP handling
* type: Bug => Cleanup/optimization
Comment:
It's not immediately obvious to me why the proposed behavior (which would
invoke `handler500` and display an "internal server error" page) is better
than the current behavior. The original commit,
3aa6b0556f74b823673bc854c3c8be92d8fbabf0, does not have much information.
Perhaps the reason for only doing the check in debug mode is that the
problem will usually be detected in development. Doing the check
unconditionally could allow malicious requests to fill error logs.
--
Ticket URL: <https://code.djangoproject.com/ticket/29364#comment:1>
Django
PUT / PATCH requests even if settings.DEBUG = False
Reporter: Nilesh Trivedi | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: HTTP handling | Version: 2.0
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Claude Paroz):
In that situation, returning a 404 instead could be acceptable.
--
Ticket URL: <https://code.djangoproject.com/ticket/29364#comment:2>
Django
PUT / PATCH requests even if settings.DEBUG = False
Reporter: Nilesh Trivedi | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: HTTP handling | Version: 2.0
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Tim Graham):
I would like the better understand the rationale for the proposed change.
Why is the current behavior problematic and why would returning a 404 be
better?
--
Ticket URL: <https://code.djangoproject.com/ticket/29364#comment:3>
Django
PUT / PATCH requests even if settings.DEBUG = False
Reporter: Nilesh Trivedi | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: HTTP handling | Version: 2.0
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Nilesh Trivedi):
I don't know whether 500 is better or 404. Either of this would be an
improvement over the current behavior which redirects to GET (with slash
appended) instead. Here is how our customers ran into this:
We have a route /sequence_items/ which supports both GET and POST requests
but the contract for both is different. GET returns the existing sequence
of items, whereas POST allows you to modify the sequence.
Our customer sent a POST request to /sequence_items (notice the missing
slash). Their intention was to change the sequence, so they sent the
correct request type, but slash was missed. Current behavior of Django,
added the slash, but ALSO changed the request type to GET (by sending a
301/302 redirect instead of 307) which meant that the addition of slash
silently changed the semantics of the API call. An error of some kind
would have been better.
Hope this helps.
--
Ticket URL: <https://code.djangoproject.com/ticket/29364#comment:4>
Django
PUT / PATCH requests even if settings.DEBUG = False
Reporter: Nilesh Trivedi | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: HTTP handling | Version: 2.0
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Carlton Gibson):
I'd learn towards wontfix. I'd prefer to see a user land solution here
before altering the built-in behaviour.
Some thoughts:
* Disable `APPEND_SLASH`. This gives you your `404`. It may not actually
be desired behaviour.
* As per the CommonMiddleware docstring, subclass CommonMiddleware and
overriding the response_redirect_class attribute, returning the `307`
response. (Whether that's wise I can't say.)
* Otherwise subclassing CommonMiddleware to allow a more sophisticated
(conditional) handling.
* Adjust URL pattern to make trailing slash optional.
* (... and so on...)
--
Ticket URL: <https://code.djangoproject.com/ticket/29364#comment:5>
Django
PUT / PATCH requests even if settings.DEBUG = False
Reporter: Nilesh Trivedi | Owner: nobody
Cleanup/optimization |
Component: HTTP handling | Version: 2.0
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Tim Graham):
* status: new => closed
* resolution: => wontfix
Comment:
Sounds fine to me.
--
Ticket URL: <https://code.djangoproject.com/ticket/29364#comment:6>