Opened 13 years ago

Closed 10 years ago

#16822 closed Bug (fixed)

HTTPRequest::raw_post_data broken for tests

Reported by: Whitney Owned by: nobody
Component: HTTP handling Version:
Severity: Normal Keywords:
Cc: james@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Attachments (1)

rawpostdata.patch (2.7 KB ) - added by James Aylett 12 years ago.
Specific exception patch including test changes

Download all attachments as: .zip

Change History (7)

comment:1 by James Aylett, 12 years ago

Cc: james@… added

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.

by James Aylett, 12 years ago

Attachment: rawpostdata.patch added

Specific exception patch including test changes

comment:2 by David Foster, 12 years ago

Has patch: set

comment:3 by Aymeric Augustin, 12 years ago

Component: Testing frameworkHTTP handling
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

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.

comment:4 by James Aylett, 12 years ago

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…

comment:5 by Whitney Young, 11 years ago

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.

comment:6 by Tim Graham, 10 years ago

Resolution: fixed
Status: newclosed

In 58d555caf527d6f1bdfeab14527484e4cca68648:

Fixed #16822 -- Added RawPostDataException

Thanks jaylett for the patch.

Note: See TracTickets for help on using tickets.
Back to Top