Bug 45159

Summary: REGRESSION (r66452): Sending of multipart forms with files is broken
Product: WebKit Reporter: Naofumi Kagami <naofumi>
Component: FormsAssignee: Jian Li <jianli>
Status: RESOLVED FIXED    
Severity: Critical CC: ap, egarcia, jianli
Priority: P1 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

Description Naofumi Kagami 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.
Comment 1 Naofumi Kagami 2010-09-02 19:28:51 PDT
Created attachment 66456 [details]
Test multipart form
Comment 2 Naofumi Kagami 2010-09-02 19:30:18 PDT
Created attachment 66457 [details]
Webkit inspector screenshot without Content-Type
Comment 3 Naofumi Kagami 2010-09-02 19:31:04 PDT
Created attachment 66458 [details]
Safari screenshot with Content-Type
Comment 4 Alexey Proskuryakov 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.
Comment 5 Jian Li 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.
Comment 6 Jian Li 2010-09-03 14:17:42 PDT
Created attachment 66539 [details]
Proposed Patch
Comment 7 Alexey Proskuryakov 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.
Comment 8 Alexey Proskuryakov 2010-09-03 14:29:31 PDT
Created attachment 66542 [details]
nc output
Comment 9 Alexey Proskuryakov 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.
Comment 10 Jian Li 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.
Comment 11 Alexey Proskuryakov 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.
Comment 12 Jian Li 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.
Comment 13 Alexey Proskuryakov 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!
Comment 14 Jian Li 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.