Opened 4 years ago

Closed 4 years ago

#21447 closed Bug (fixed)

post parse error no longer correctly represented.

Reported by: Tom Christie Owned by: nobody
Component: HTTP handling Version: master
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


This line was unintentionally commented out: as part of ticket #21189

Change History (3)

comment:1 Changed 4 years ago by Tom Christie

Pull request is

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 Changed 4 years ago by Baptiste Mispelon

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 Changed 4 years ago by Baptiste Mispelon <bmispelon@…>

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

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

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