Opened 10 years ago

Closed 10 years ago

#21447 closed Bug (fixed)

post parse error no longer correctly represented.

Reported by: Tom Christie Owned by: nobody
Component: HTTP handling Version: dev
Severity: Release blocker Keywords:
Cc: Baptiste Mispelon Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

This line was unintentionally commented out: https://github.com/django/django/blob/master/django/http/request.py#L245 as part of ticket #21189

Change History (3)

comment:1 by Tom Christie, 10 years ago

Pull request is https://github.com/django/django/pull/1924

As it happens, even with _mark_post_parse_error() commented out, the build_request_repr *does* correctly mark <could not parse>, *at a least* in the parse error case I've provided, as request.POST ends up re-parsing the request. However it's not clear if there are cases such as parse errors that only occur when the stream has been fully or partially read where it would simply end up repr as a blank request.POST. That's difficult to replicate as you need to provide a multipart content that contains a file field, with an invalid base64 transfer encoding. (I havn't quite figured out how to do that.)

Given that this is clearly an erroneous commenting out, I think we clearly need to revert it in any case.

comment:2 by Baptiste Mispelon, 10 years ago

Cc: Baptiste Mispelon added
Has patch: set
Patch needs improvement: set
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Yes, this was clearly a mistake on my part.

The patch looks good but as you said, the included test doesn't actually fail when self._mark_post_parse_error() is commented out which is kind of a problem.

I'll try to see if I can find a way to have the test fail and if I can't, I'll merge your pull request directly.

Thanks for catching this early.

comment:3 by Baptiste Mispelon <bmispelon@…>, 10 years ago

Resolution: fixed
Status: newclosed

In ceecc962ad8f6bbbc2b989aec53eee6c6cca04b9:

Fixed #21447 -- Restored code erroneously removed in 20472aa827669d2b83b74e521504e88e18d086a1.

Also added some tests for HttpRequest.repr.
Note that the added tests don't actually catch the accidental code
removal (see ticket) but they do cover a codepath that wasn't tested
before.

Thanks to Tom Christie for the report and the original patch.

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