Opened 4 years ago

Closed 20 months 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 jaylett 4 years ago.
Specific exception patch including test changes

Download all attachments as: .zip

Change History (7)

comment:1 Changed 4 years ago by jaylett

  • Cc james@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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.

Changed 4 years ago by jaylett

Specific exception patch including test changes

comment:2 Changed 4 years ago by davidfstr

  • Has patch set

comment:3 Changed 4 years ago by aaugustin

  • Component changed from Testing framework to HTTP handling
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

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 Changed 4 years ago 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…

comment:5 Changed 3 years ago 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.

comment:6 Changed 20 months ago by timo

  • Resolution set to fixed
  • Status changed from new to closed

In 58d555caf527d6f1bdfeab14527484e4cca68648:

Fixed #16822 -- Added RawPostDataException

Thanks jaylett for the patch.

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