Code

Opened 3 years ago

Closed 2 years ago

#15496 closed Bug (fixed)

"Content-Transfer-Encoding: base64" not honored when uploading files

Reported by: gene@… Owned by: nobody
Component: HTTP handling Version: master
Severity: Normal Keywords: base64 file upload
Cc: tomchristie Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When the client POST-s a file in base64 encoding, Django does not decode the file correctly.

How to Repeat

Upload a file to a Django view in an HTML form with method="POST" and enctype="multipart/form-data"; inside the MIME part for the file, encode the body in base64 and indicate the encoding in the Content-Transfer-Encoding: header.

Expected Result

The uploaded file, when examined on disk, is decoded (by Django).

Actual Result

The uploaded file, when examined on disk, is still base64-encoded.

Analysis

In module django.http.multipartparser, MultiPartParser.parse() iterates over each part as returned by a Parser object. The Parser object yields a header dictionary, which MultiPartParser.parse() captures in meta_data. parse() then fetches the Content-Transfer-Encoding header from meta_data into transfer_encoding.

transfer_encoding, as returned by meta_data.get(), is not simply a string such as 'base64' but really a 2-tuple (value, params). Also, the header value (the first element of the returned tuple) is not whitespace-trimmed.

As a result, if the part contained 'Content-Transfer-Encoding: base64', meta_data.get('Content-Transfer-Encoding') does not return just 'base64' but a 2-tuple (' base64', {}):

  • The header has no parameters, hence an empty parameter dictionary;
  • The value portion retains the leading space, right after the colon (:) in the raw header.

parse() needs the whitespace-trimmed version of the first element of the tuple; the attached patch converts the returned tuple into the string.

Attachments (2)

django-base64-upload-patch.diff (667 bytes) - added by gene@… 3 years ago.
base64-upload-with-tests.diff (3.4 KB) - added by claudep 3 years ago.

Download all attachments as: .zip

Change History (13)

Changed 3 years ago by gene@…

comment:1 Changed 3 years ago by russellm

  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 3 years ago by tomchristie

  • Cc tomchristie added

comment:3 Changed 3 years ago by lrekucki

  • Severity set to Normal
  • Type set to Bug

Changed 3 years ago by claudep

comment:4 Changed 3 years ago by claudep

  • Easy pickings unset
  • Needs tests unset
  • Version changed from 1.2 to SVN

comment:5 Changed 3 years ago by jezdez

  • Triage Stage changed from Accepted to Ready for checkin

comment:6 Changed 3 years ago by jezdez

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

In [16176]:

Fixed #15496 -- Corrected handling of base64 file upload encoding. Thanks, gene and Claude Paroz.

comment:7 Changed 3 years ago by anonymous

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • UI/UX unset

My multipart post data has only one field that's base64 encoded. I applied this patch over the stock 1.3 release and found that it screwed up the other parameters, with their default encoding. Maybe I am doing something wrong. Here's my post request. Works great without the patch, I just have to unencode the chunks of the file in my filehandler rather than relying on the automatic behavior which, I agree, would be preferred.

Content-Type: multipart/form-data; boundary=RANDOM_STRING_BOUNDARY
--RANDOM_STRING_BOUNDARY
Content-Disposition: form-data; name="csrfmiddlewaretoken"

1e9c92a33c9a70fc335f93f8f78e20cd
--RANDOM_STRING_BOUNDARY
Content-Disposition: form-data; name="settings_group_id"

257
--RANDOM_STRING_BOUNDARY
Content-Disposition: form-data; name="serial_number"

10001
--RANDOM_STRING_BOUNDARY
Content-Disposition: form-data; name="image_file"; filename="2.tif"
Content-Type: application/octet-stream
Content-Transfer-Encoding: base64

[...base64 encoded data...]
--RANDOM_STRING_BOUNDARY--

comment:8 Changed 3 years ago by claudep

  • Resolution set to needsinfo
  • Status changed from reopened to closed

I tried to extend the test to include non base-64 data, I double-checked the code which detects the encoding, and I don't see how other data may be affected by this bug's patch. Of course, I may be missing something. But please give us more details about how your other parameters are screwed up. What do you get in resulting request.POST?

comment:9 Changed 3 years ago by anonymous

Thanks for looking into it. One of these days I'll get back to it and repeat my test. The above is my actual POST data... it should be easy enough for me to repeat the test when I get a free hour. (Ha! A free hour!) Thank you again for double-checking the patch.

comment:10 Changed 2 years ago by aaugustin

  • Resolution needsinfo deleted
  • Status changed from closed to reopened

This could be backported to 1.3 on the grounds that it's a data-loss bug.

comment:11 Changed 2 years ago by aaugustin

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

In [17546]:

[1.3.x] Fixed #15496 -- Corrected handling of base64 file upload encoding. Backport of r16176 from trunk.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.