[Django] #16822: HTTPRequest::raw_post_data broken for tests
Django
---------------------------+-----------------------------------
Reporter: Whitney | Owner: nobody
Type: Uncategorized | Status: new
Milestone: | Component: Testing framework
Version: | Severity: Normal
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------+-----------------------------------
The `HTTPRequest::raw_post_data` method does not work for the test suite
any more. It seems that the exception is caused due to the changes in
django.http (`__init__.py`) during processing of a multipart request.
Previously it assigned `self._raw_post_data`, and not it does not. The
following change fixes the problem, but I don't know if it's exactly what
you'd want to do:
{{{
Index: django/http/__init__.py
===================================================================
--- django/http/__init__.py (revision 16821)
+++ django/http/__init__.py (working copy)
@@ -272,6 +272,7 @@
# Use already read data
data = StringIO(self._raw_post_data)
else:
+ self._raw_post_data = ''
data = self
try:
self._post, self._files =
self.parse_file_upload(self.META, data)
}}}
I would have assigned this to version 1.3.1, but there is no choice for
that.
--
Ticket URL: <https://code.djangoproject.com/ticket/16822>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Whitney | Owner: nobody
Type: | Status: new
Uncategorized | Component: Testing framework
Milestone: | Severity: Normal
Version: | Keywords:
Resolution: | Has patch: 0
Triage Stage: | Needs tests: 0
Unreviewed | Easy pickings: 0
Needs documentation: 0 |
Patch needs improvement: 0 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
Changes (by jaylett):
* cc: james@… (added)
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
This was introduced in [15939] deliberately, to close #15679, but I'm not
happy with the behaviour. There are utility reasons (in middleware, or
error handling) why you might want to read the raw post data independent
of what's happened before. This is particularly causing me problems with
sentry.
As I understand it, it's an assumed invariant that you won't access
raw_post_data having read it once, but the problem is that there are two
paths to reading it, one of which (non-multipart) populates the
_raw_post_data memo (and hence you *can* access raw_post_data as many
times subsequently as you like) and one of which (multipart) doesn't, the
latter on the basis that multipart data can be "big" and it'd chew up
memory.
I don't think raising the (untyped) exception "You cannot access
raw_post_data after reading from request's data stream" is helpful,
because (a) it's not always true, (b) it can't easily be specifically
caught as an expected failure in error handling code such as sentry.
Perhaps an Exception subclass that can be explicitly caught would be a
better choice?
I'd suggest changing the component to HTTP handling because I've seen this
in the wild, not just in tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/16822#comment:1>
Django
Type: Uncategorized | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by davidfstr):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/16822#comment:2>
Django
Reporter: Whitney | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Changes (by aaugustin):
* type: Uncategorized => Bug
* component: Testing framework => HTTP handling
* stage: Unreviewed => Accepted
Comment:
Raising a more specific exception appears to be a good idea, and I will
accept the ticket at least for this change.
However, I don't understand how it resolves the original problem. A test
case would be useful.
--
Ticket URL: <https://code.djangoproject.com/ticket/16822#comment:3>
Django
Reporter: Whitney | Owner: nobody
Component: HTTP handling | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Comment (by jaylett):
It doesn't resolve the original problem, but it allows test writers to do
something sane rather than bomb out. Admittedly, without a precise test
case for the original report I may be misunderstanding what Whitney's
problem was…
--
Ticket URL: <https://code.djangoproject.com/ticket/16822#comment:4>
Django
Reporter: Whitney | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Whitney Young):
I'm pretty sure I opened this one because I just came back to the project
that had the problem and ended up here after a Google search.
The exception that's being thrown now is really helpful. What I did in my
tests was just fake the HTTP body (it didn't even need content in my
tests… I just didn't want to change the actual code). Here's what I did:
{{{
def setUp(self):
self.body_function = HttpRequest.body
HttpRequest.body = property(lambda self: '')
super(MyTestCase, self).setUp()
def tearDown(self):
HttpRequest.body = self.body_function
super(MyTestCase, self).setUp()
}}}
In my opinion, this can be closed.
--
Ticket URL: <https://code.djangoproject.com/ticket/16822#comment:5>
Django
Reporter: Whitney | Owner: nobody
Type: Bug | Status: closed
Component: HTTP handling | Version:
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* status: new => closed
* resolution: => fixed
Comment:
In 58d555caf527d6f1bdfeab14527484e4cca68648:
Fixed #16822 -- Added RawPostDataException
Thanks jaylett for the patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/16822#comment:6>