Bug 45159

Summary:

REGRESSION (r66452): Sending of multipart forms with files is broken

Product:

WebKit

Reporter:

Naofumi Kagami <naofumi>

Component:

Forms

Assignee:

Jian Li <jianli>

Status:

RESOLVED FIXED

Severity:

Critical

CC:

ap, egarcia, jianli

Priority:

Keywords:

Regression

Version:

528+ (Nightly build)

Hardware:

Mac (Intel)

OS:

OS X 10.6

Attachments:

Description	Flags
Test multipart form	none
Webkit inspector screenshot without Content-Type	none
Safari screenshot with Content-Type	none
Proposed Patch	darin: review+, jianli: commit-queue-
nc output	none

Naofumi Kagami

Reported 2010-09-02 19:28:11 PDT

On the latest few builds of Webkit, multipart forms are not sending the "Content-Type:" header. Steps to confirm. 1. Open the attachment html file (just a simple multipart form) and send an image file (jpg, png or whatever). 2. Submit (you should come back to the same page) 3. Open up the "Web Inspector" in WebKit. 4. Check the "Headers" from the "Resources" tab. The current Safari Release (5.0.1) has the correct "Content-Type:" header. The most recent Webkit nightly (r66680) does not have this header. It is completely missing.

Attachments
Test multipart form (279 bytes, text/html) 2010-09-02 19:28 PDT, Naofumi Kagami	no flags	Details
Webkit inspector screenshot without Content-Type (119.34 KB, image/png) 2010-09-02 19:30 PDT, Naofumi Kagami	no flags	Details
Safari screenshot with Content-Type (122.13 KB, image/png) 2010-09-02 19:31 PDT, Naofumi Kagami	no flags	Details
Proposed Patch (5.54 KB, patch) 2010-09-03 14:17 PDT, Jian Li	darin: review+ jianli: commit-queue-	Details Formatted Diff Diff
nc output (302 bytes, text/plain) 2010-09-03 14:29 PDT, Alexey Proskuryakov	no flags	Details
View All Add attachment proposed patch, testcase, etc.

Naofumi Kagami

Comment 1 2010-09-02 19:28:51 PDT

Created attachment 66456 [details] Test multipart form

Naofumi Kagami

Comment 2 2010-09-02 19:30:18 PDT

Created attachment 66457 [details] Webkit inspector screenshot without Content-Type

Naofumi Kagami

Comment 3 2010-09-02 19:31:04 PDT

Created attachment 66458 [details] Safari screenshot with Content-Type

Alexey Proskuryakov

Comment 4 2010-09-03 12:16:06 PDT

The posted form serialization is completely broken - file content comes first without even a boundary line before it. I didn't try to regress this, but r66452 seems to be the only suspicious revision lately.

Jian Li

Comment 5 2010-09-03 13:52:20 PDT

(In reply to comment #4) > The posted form serialization is completely broken - file content comes first without even a boundary line before it. > > I didn't try to regress this, but r66452 seems to be the only suspicious revision lately. I have a fix for content-type missing problem. Alex, I do not see the problem you mentioned: file content comes without a boundary line before it. The attached screen shot does not show this problem. Neither can I reproduce this in my local machine. Could you please tell me how you reproduce it? Thanks.

Jian Li

Comment 6 2010-09-03 14:17:42 PDT

Created attachment 66539 [details] Proposed Patch

Alexey Proskuryakov

Comment 7 2010-09-03 14:26:18 PDT

> Could you please tell me how you reproduce it? I just ran "nc -l 8000" on command line, and changed form action to http://127.0.0.1:8000/, so I could see what was actually sent.

Alexey Proskuryakov

Comment 8 2010-09-03 14:29:31 PDT

Created attachment 66542 [details] nc output

Alexey Proskuryakov

Comment 9 2010-09-03 15:50:08 PDT

This patch has been landed in <http://trac.webkit.org/changeset/66773>, but please take a look at the above comments.

Jian Li

Comment 10 2010-09-03 16:06:55 PDT

(In reply to comment #9) > This patch has been landed in <http://trac.webkit.org/changeset/66773>, but please take a look at the above comments. I tried your way to dump the response but I did not see any problem. I think if this is broken (the boundary string is missing), we should see a lot of broken layout tests. Can you double-check that you can reproduce this locally without any other interfering changes? Can you attach the output if you do see such bad things happen? Thanks.

Alexey Proskuryakov

Comment 11 2010-09-03 16:19:42 PDT

> Can you attach the output Yes, it's already attached. I'm seeing this with a local build of r66738, no local changes.

Jian Li

Comment 12 2010-09-03 16:42:25 PDT

(In reply to comment #11) > > Can you attach the output > > Yes, it's already attached. I'm seeing this with a local build of r66738, no local changes. I ran the same script as yours and did not see any problem. Here is what I saw: POST / HTTP/1.1 Host: 127.0.0.1:8000 ... ------WebKitFormBoundaryJZxbkQBhniZYn9IA Content-Disposition: form-data; name="username" 123 ------WebKitFormBoundaryJZxbkQBhniZYn9IA Content-Disposition: form-data; name="submitfile"; filename="45159.html" Content-Type: text/html <html> ... ------WebKitFormBoundaryJZxbkQBhniZYn9IA Content-Disposition: form-data; name="submit" submit ------WebKitFormBoundaryJZxbkQBhniZYn9IA-- Can you check if the following tests are running OK in your local machine? run-web-tests --debug http/tests/local/formdata Thanks.

Alexey Proskuryakov

Comment 13 2010-09-03 16:55:19 PDT

OK, I see what happened - control codes in my GIF file ate some lines of terminal output, exactly as many as it was needed to hide the boundary. The actual content sent over the wire is not malformed. Sorry for my stubborn confusion!

Jian Li

Comment 14 2010-09-03 17:05:57 PDT

(In reply to comment #13) > OK, I see what happened - control codes in my GIF file ate some lines of terminal output, exactly as many as it was needed to hide the boundary. The actual content sent over the wire is not malformed. > > Sorry for my stubborn confusion! Glad to hear that this mystery is solved. I should not get this stupid bug in first. Now we have better test to guard against this from happening again.

Note You need to log in before you can comment on or make changes to this bug.