Ticket #1953 (closed defect: fixed)
[PATCH] Add request filter to handle file uploads from buggy Flash clients
| Reported by: | Chris Arndt | Owned by: | Chris Arndt |
|---|---|---|---|
| Priority: | normal | Milestone: | 1.1b2 |
| Component: | TurboGears | Version: | 1.1 HEAD |
| Severity: | normal | Keywords: | needs tests, needs review |
| Cc: |
Description (last modified by Chris Arndt) (diff)
As described in this post on the TurboGears mailing list, Flash does not terminate the last part in a multipart request correctly with CRLF and CherryPy will timeout while parsing the request, waiting for it multipart part to finish.
The attached patch implements the workaround from this CherryPy ticket. The patch was included in CP 3 but hasn't made it into CP 2.3 so it's probably best to handle this in TurboGears.
The patch provides no test yet, since I have no idea yet, how to set up a broken request to test the workaround, but field testing shows that TG with the patch is able to handle file uploads from buggy Flash clients correctly, whereas without, it throws a timeout exception.
Attachments
-
flash-multipart-fix.diff
(7.4 KB) -
added by Chris Arndt 3 years ago.
-
safemultipart-filter.diff
(7.7 KB) -
added by Chris Arndt 3 years ago.
- New version of patch. Renamed FlashMultipartFilter to SafeMultipartFilter
-
FlashUploadTest-1.0-py2.5.egg
(117.1 KB) -
added by Chris Arndt 3 years ago.
-
test_mp_upload.py
(1.6 KB) -
added by Chris Arndt 3 years ago.
-
dev.cfg
(1.8 KB) -
added by Chris Arndt 3 years ago.
-
test_safemultipart.py
(2.6 KB) -
added by Chris Arndt 3 years ago.
Change History
Changed 3 years ago by Chris Arndt
-
attachment
safemultipart-filter.diff
added
New version of patch. Renamed FlashMultipartFilter to SafeMultipartFilter
comment:2 Changed 3 years ago by Chris Arndt
- Status changed from new to assigned
- Owner changed from anonymous to Chris Arndt
- Description modified (diff)
I'm having trouble writing a working unit test case for this patch, so I have written a small example project that servers as a demonstration of the problem and as a functional test for the solution (if you apply the safemultipart-filter.diff patch).
Download & install the example project, apply the patch to TG 1.1 and run the app with TG 1.1. Try out the two different upload pages. One posts to the URL /okupload1 that has the SafeMultpartFilter enabled, the other posts to /failupload1 where the filter is not active. Please note that the bug manifests itself only when using a Mac OS Flash client''
You can also download the attached script test_mp_upload.py that sends a crafted, malformed multipart request to simulate a Mac OS Flash client to the same two URLs.
I have also attached my attempt at writing a unit test for this, but there are two problem with testutil.
- There is a bug in wsgiref.validate.InputWrapper: it's readline method does not accept arguments and thus breaks the MultipartWrapper from my patch which uses the optional size argument that the standard readline method of file objects accepts.
- Even when fixing wsgiref I can not reproduce the bug with the testutil request setup. Apparentyl webtest.TestApp.do_request() somehow mangles the request so that it is correctly parsed by CherryPy, even if the original multipart request body was malformed.
If anybody has an idea how to write a proper unit test for this I'd be grateful to hear about it!
comment:4 Changed 3 years ago by Chris Arndt
The bug in wsgiref is here: http://bugs.python.org/issue3834
comment:6 Changed 3 years ago by Chris Arndt
One caveat: the example project will NOT work with Flash Player 10 which was released recently. I'll add an updated example soon.
comment:7 Changed 3 years ago by faide
PJE filed the wsgiref ticket as invalid. This means we won't get support for the tests... Shall we go ahead and commit as-is ? Do you intend to add support for flash 10 ?
comment:8 Changed 3 years ago by Chris Arndt
We can submit this without a proper test, the example project shows that it works. We just won't have regression protection.
Adding Flash 10 support is not a TurboGears matter, it is the FancyUploader? Flash/JS that breaks, so there should be nothing to change one this is fixed.
comment:10 Changed 3 years ago by Chris Arndt
- Status changed from assigned to closed
- Resolution set to fixed
Added documentation to 1.1. configuration reference at http://docs.turbogears.org/1.1/Configuration.
Created new ticket #2032 for the test issue.
