Opened 12 years ago
Closed 10 years ago
#19036 closed Bug (fixed)
Big base64-encoded files uploading
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | File uploads/storage | Version: | dev |
Severity: | Release blocker | Keywords: | |
Cc: | 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
I'm trying to upload image file using ajax & browser FileReader.readAsDataURL API. In most cases it fails with MultiPartParserError: Could not decode base64 data: Error('Incorrect padding',).
When django cut of header from 'multipart/form-data' part with encoded file data, it does not check length of the remaining data and pass it to decoder. Length of passed data is (FileUploadHandler chunk size) - (removed header length)
which is not divides by 4 in most cases.
It is reproduced by testcase I attached. It is based on standard Django test_base64_upload test but reads 512Kb from /dev/urandom instead of short string.
Attachments (4)
Change History (14)
by , 12 years ago
Attachment: | big_base64_upload added |
---|
comment:1 by , 12 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 12 years ago
Not sure. I tried to make a patch by modifying MultiPartParser
, where it must be processed in my opinion (variant 1), but was confused a little with streams there. I did not spent much time with this part of Django previously and do not realize full request processing routine in details yet. But I think, second variant is less acceptable because it will be returning to data processing after finishing and will require doubling of code (using low-level calls in higher-level abstractions) what will complicate the structure of handlers.
comment:3 by , 12 years ago
Has patch: | set |
---|
Just attached a patch that seems to work (testing welcome). My only worry is about memory management when doing chunk += over_chunk
. Hopefully the Python implementation is smart and do not copy too much stuff in memory.
comment:4 by , 12 years ago
Severity: | Normal → Release blocker |
---|
Added blocker flag, as currently 3 of 4 base64 file uploads are doomed to fail.
comment:5 by , 12 years ago
Regarding memory usage, this should be good enough for release. After 1.5, it might be possible to use base64.encode together with StringIO, or even adjusting read-sizes in streaming code.. I doubt the complexity it adds will be worth the performance gains though.
comment:6 by , 12 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:7 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:9 by , 10 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
This bug is present also in Django 1.4.13 which is mentioned on Django webpage as still supported.
comment:10 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
The 1.4 branch is only receiving security fixes at this time.
Confirmed. Do you have any idea about the cleaner way to solve this?
Some ideas:
file_complete
upload handler method)