Extraneous newlines in large amounts of POST data
|Reported by:||Owned by:||nobody|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||yes|
I ran into a problem doing some stress testing of newforms-admin. I'd bring up a change page for a model that has approx 700 other models edited inline, make no changes to the data (which was all valid), attempt to "save and continue editing", and get seemingly random validation failures for some of the inline-edited fields. I'd get messages about 1-character fields having data that was too long (3 chars), even though all I could see in the field was 1 character, or a message that my foreign key selection wasn't valid, even though it was, etc. Occasionally the save would work, but 4 out of 5 times it would fail with one or more validation errors. Eventually I tracked the problem down to parse_file_upload in django/http/__init__.py (http://code.djangoproject.com/browser/django/trunk/django/http/__init__.py#L100). After adding some code to dump the contents of raw_message to a file, plus the sequence of items appended to POST on line 130, I found that while the post data in raw_message looked fine, sometimes the values returned by submessage.get_payload() had extraneous newlines at the beginning of the value.
Since Django is using Python's email routines here, I searched for relevant Python bugs and found this:
(Someone else using Django already ran into this! But I can't find anything open in Django's trac that matches....)
The Python bug is still open. The last comment there seems to imply that while the provided patch is probably fine, the issue is really a doc problem and that the doc should make it clear that the Python code is expecting a file that has been opened in universal newline mode, where CRLF will be translated to just LF. For the routine that Django is using to construct the email message, I take that to mean that the string passed to email.message_from_string should have just LFs, not CRLFs as it does now. I tried replacing:
msg = email.message_from_string(raw_message)
msg = email.message_from_string(raw_message.replace('\r\n', '\n')
and it does resolve the problem for my case: six successive saves of this model worked fine, whereas without the fix maybe only one would have worked.
Even though I ran into the problem using newforms-admin, the code here is identical on trunk so that is where I think it should be fixed.
Change History (14)
comment:1 Changed 9 years ago by
|Patch needs improvement:||unset|
|Triage Stage:||Unreviewed → Design decision needed|