Bug 184490 – REGRESSION (r221839): Submitting a form with XMLHttpRequest fails when the form has an empty file input element

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Eugene]

Created attachment 337684 [details]
request body

Bug is present only in Safari 11. Safari 9 and 10 work well.

1. We have a form with enctype="multipart/form-data"
2. The form has input[type=file]
3. If the file is not selected, Safari does not send the full request, stopping at the last line of the request.
4. Request body attached, header "Content-Length: 715"
5. The behavior does not depend on the version of the HTTP protocol:

HTTP/1.1
2018/04/11 00:35:35 [debug] 13989#0: *838 http read client request body
2018/04/11 00:35:35 [debug] 13989#0: *838 http client request body recv 669
2018/04/11 00:35:35 [debug] 13989#0: *838 http client request body rest 715

HTTP/2
2018/04/10 14:18:40 [info] 8478#0: *34 client prematurely closed stream: only 669 out of 715 bytes of request body received

[Steve]

Created attachment 337720 [details]
HTML test case demonstrating bug

I believe I've also experienced this issue. A little more information to help elaborate on this issue:

* We only see this issue when submitting a form with enctype="multipart/form-data" and using FormData to send data in the XMLHttpRequest object.
* The issue only arises when the file field is left empty. A selected file allows normal submission.
* Only happened after upgrading to macOS 10.13.4 and Safari 11.1

I've attached an HTML test case that recreates the error. The file contains the following 4 examples:
* Normal form submission, non-AJAX
* AJAX form submission with no file data
* AJAX form submission using FormData to submit file data
* XMLHttpRequest submission using FormData to submit file data.

Expected behavior:
For all forms to submit a valid request and receive response data. Response data would show up beneath each form, respectively.

Actual behavior:
* The non-AJAX form submits properly with or without a file selected and always returns a valid response
* The AJAX form that sends no file data submits properly and always returns a valid response
* The AJAX form using FormData to send file data creates a valid request when file data is selected. When no file is selected and the form is submitted the browser hangs and doesn't appear to send a valid request.
* The XMLHttpRequest form using FormData to send file data creates a valid request when file data is selected. When no file is selected and the form is submitted the browser hangs and doesn't appear to send a valid request

[Steve]

Created attachment 337723 [details]
HTML test case demonstrating bug

Added https to "HTML test case demonstrating bug" test case to remove warnings when testing.

[Eugene]

Yep, we are experiencing this bug with XMLHttpRequest form using FormData.

[Radar WebKit Bug Importer]

<rdar://problem/39385169>

[Ryosuke Niwa]

Another report of this issue on Twitter: https://qiita.com/yuya_presto/items/65be91b0255af49f0396

[Matt]

I’m experiencing this as well. First noticed with 11.1 but then found in 11.2. Posted about it on StackOverkow back on 4/2 (https://stackoverflow.com/questions/49614091/safari-11-1-ajax-xhr-form-submission-fails-when-inputtype-file-is-empty) and filed a bug report with Apple.

[Tadeu Zagallo]

Created attachment 338620 [details]
Patch

[EWS Watchlist]

Comment on attachment 338620 [details]
Patch

Attachment 338620 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/7417306

New failing tests:
http/tests/fileapi/xhr-send-form-data-mimetype-normalization.html
http/tests/local/formdata/send-form-data-with-sliced-file.html
imported/w3c/web-platform-tests/XMLHttpRequest/formdata.htm
imported/w3c/web-platform-tests/XMLHttpRequest/formdata-blob.htm
http/tests/fileapi/xhr-send-form-data-filename-escaping.html
http/tests/misc/form-blob-challenge.html

[EWS Watchlist]

Created attachment 338625 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 338620 [details]
Patch

Attachment 338620 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/7417614

New failing tests:
http/tests/fileapi/xhr-send-form-data-mimetype-normalization.html
imported/w3c/web-platform-tests/service-workers/service-worker/fetch-request-xhr.https.html
http/tests/misc/form-blob-challenge.html
imported/w3c/web-platform-tests/XMLHttpRequest/formdata-blob.htm
http/tests/fileapi/xhr-send-form-data-filename-escaping.html
imported/w3c/web-platform-tests/XMLHttpRequest/formdata.htm

[EWS Watchlist]

Created attachment 338626 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 338620 [details]
Patch

Attachment 338620 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/7417764

New failing tests:
http/tests/fileapi/xhr-send-form-data-mimetype-normalization.html
imported/w3c/web-platform-tests/service-workers/service-worker/fetch-request-xhr.https.html
imported/w3c/web-platform-tests/XMLHttpRequest/formdata.htm
imported/w3c/web-platform-tests/XMLHttpRequest/formdata-blob.htm
http/tests/fileapi/xhr-send-form-data-filename-escaping.html
http/tests/misc/form-blob-challenge.html

[EWS Watchlist]

Created attachment 338629 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4

[EWS Watchlist]

Comment on attachment 338620 [details]
Patch

Attachment 338620 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/7418332

New failing tests:
http/tests/fileapi/xhr-send-form-data-mimetype-normalization.html
http/tests/local/formdata/send-form-data-with-sliced-file.html
imported/w3c/web-platform-tests/XMLHttpRequest/formdata.htm
imported/w3c/web-platform-tests/XMLHttpRequest/formdata-blob.htm
http/tests/fileapi/xhr-send-form-data-filename-escaping.html
http/tests/misc/form-blob-challenge.html

[EWS Watchlist]

Created attachment 338631 [details]
Archive of layout-test-results from ews115 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Tadeu Zagallo]

Created attachment 338646 [details]
Patch

[Geoffrey Garen]

Comment on attachment 338646 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 338646 [details]
Patch

Clearing flags on attachment: 338646

Committed r230963: <https://trac.webkit.org/changeset/230963>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

Comment on attachment 338646 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=338646&action=review

> Source/WebCore/platform/network/FormData.cpp:227
> +    else if (file.size())
>          appendBlob(file.url());

This doesn't look like the same check that is described in the ChangeLog ("a file but has no path"). A special case for empty files seems like it can't be right, can it?

[Tadeu Zagallo]

(In reply to Alexey Proskuryakov from comment #20)
> Comment on attachment 338646 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=338646&action=review
> 
> > Source/WebCore/platform/network/FormData.cpp:227
> > +    else if (file.size())
> >          appendBlob(file.url());
> 
> This doesn't look like the same check that is described in the ChangeLog ("a
> file but has no path"). A special case for empty files seems like it can't
> be right, can it?

The problem I found (and maybe should have update the ChangeLog to describe it) was that Blobs created from JavaScript are also instances of File at this point, so the logic here was already: if it is a file and has no path, it must be a blob. My idea was to change it to: If it is a file and has no path and is not empty, it must be a blob. This also excludes empty blobs, but that should be ok since the empty blob would not change the request in any way.

[Alexey Proskuryakov]

> the empty blob would not change the request in any way.

I find this surprising, but don't have the time to test and prove this right or wrong. Won't there be a section with a MIME type and file name, and empty content? Or in other words, empty files do exist and can be uploaded (e.g. can be attached with form submission), so why can't empty blobs be uploaded?

[Tadeu Zagallo]

> Won't there be a section with a MIME type and file name, and empty content?

Oh, yes. This information has already been added to the output at this point, regardless of whether it is a file or a blob. With this change, we just skip adding the contents of the blob to the output, but it has 0 bytes, which is what I meant by "the empty blob would not change the request in any way". Sorry if my first comment wasn't very clear.