[Django] #21447: post parse error no longer correctly represented.

0 views
Skip to first unread message

Django

unread,
Nov 15, 2013, 11:32:56 AM11/15/13
to django-...@googlegroups.com
#21447: post parse error no longer correctly represented.
-------------------------------+--------------------
Reporter: tomchristie | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
This line was unintentionally commented out:
https://github.com/django/django/blob/master/django/http/request.py#L245
as part of ticket #21189

--
Ticket URL: <https://code.djangoproject.com/ticket/21447>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Nov 15, 2013, 1:44:39 PM11/15/13
to django-...@googlegroups.com
#21447: post parse error no longer correctly represented.
-------------------------------+--------------------------------------

Reporter: tomchristie | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by tomchristie):

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0


Comment:

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/21447#comment:1>

Django

unread,
Nov 15, 2013, 2:57:20 PM11/15/13
to django-...@googlegroups.com
#21447: post parse error no longer correctly represented.
---------------------------------+------------------------------------

Reporter: tomchristie | Owner: nobody
Type: Bug | Status: new
Component: HTTP handling | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Changes (by bmispelon):

* cc: bmispelon (added)
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted


Comment:

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.

--
Ticket URL: <https://code.djangoproject.com/ticket/21447#comment:2>

Django

unread,
Nov 15, 2013, 7:11:00 PM11/15/13
to django-...@googlegroups.com
#21447: post parse error no longer correctly represented.
---------------------------------+------------------------------------
Reporter: tomchristie | Owner: nobody
Type: Bug | Status: closed

Component: HTTP handling | Version: master
Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Changes (by Baptiste Mispelon <bmispelon@…>):

* status: new => closed
* resolution: => fixed


Comment:

In [changeset:"ceecc962ad8f6bbbc2b989aec53eee6c6cca04b9"]:
{{{
#!CommitTicketReference repository=""
revision="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.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/21447#comment:3>

Reply all
Reply to author
Forward
0 new messages