1459859 - Empty <input type=file> is represented incorrectly in FormData

Status: RESOLVED FIXED

(Core :: DOM: Forms, enhancement, P3)

Description

[Anne (:annevk)]

Test: https://github.com/w3c/web-platform-tests/pull/10865. (Discovered while making an editorial change to the HTML Standard: https://github.com/whatwg/html/pull/3658.)

Andrea, you probably find this interesting (and might have even complained about the ambiguity to me a long time ago).

Comment 1

[Andreu Botella]

I've been looking into this, and I thought this was simply a Firefox bug, but when I went down the blame I found myself lost in a lot of changes and regressions in late 2015 / early 2016, and then I found https://github.com/whatwg/html/issues/476#issuecomment-197878222, which argues this is expected behavior and anything else breaks websites?

For context, until https://github.com/whatwg/html/pull/3658, the spec's behavior for an empty <input type=file> was to have the entry list value set to an empty string, so its multipart/form-data serialization would not have a filename or a Content-Type header. That PR changed it to make the corresponding entry list value a File object with empty contents, the empty string as filename and application/octet-stream as the MIME type.

The list of FormData-related bugs around that time is huge, and I'm having trouble keeping track of things, but it seems like every report of websites being broken (bug 1246375, bug 1247538, bug 1250148 comment 4) was about the representation in the form payload, not about what FormData.prototype.get returns. And even though what FormData.prototype.get returns seems to have changed back and forth, it looks like the last few changes were only meant to unbreak websites that depended on the form payload, deferring the fix for FormData.prototype.get to some later time that apparently never came (see bug 1246375 comment 17 and the next comment).

In any case, if FormData.prototype.get returning a File object used to break websites, I very much doubt that it still does, since that's what every other major browser does.

But NI'ing baku and smaug because I'm almost certainly missing some context here.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

It is actually a bit hard to interpret this bug.
Is this only about get? If so, changing that to work like in other browsers doesn't sound too scary.

Comment 3

[Anne (:annevk)]

I'm not sure about comment 1 as as far as I can tell 3658 didn't change anything. The specification references bug 529859 which suggests that an empty file is supposed to be generated when no files are selected and that Firefox implements that. The test I wrote that is referenced in OP tests that FormData is aligned with that.

Comment 4

[Andreu Botella]

(In reply to Olli Pettay [:smaug] from comment #2)

It is actually a bit hard to interpret this bug.
Is this only about get? If so, changing that to work like in other browsers doesn't sound too scary.

The OP is indeed cryptic, but Firefox matches the spec in the form payload (https://wpt.fyi/results/html/semantics/forms/form-submission-0/form-data-set-empty-file.window.html) and the linked WPT PR only tests for the get behavior.

(In reply to Anne (:annevk) from comment #3)

I'm not sure about comment 1 as as far as I can tell 3658 didn't change anything. The specification references bug 529859 which suggests that an empty file is supposed to be generated when no files are selected and that Firefox implements that. The test I wrote that is referenced in OP tests that FormData is aligned with that.

It actually did, see https://github.com/whatwg/html/pull/3658#discussion_r185425985. Before that, a straightforward implementation of the spec would have the form entry have the empty string as a value and "application/octet-stream" as the type (which was set to the input's type attribute elsewhere). In bug 1187157 comment 7, bzbarsky missed the fact that that was a bug, for example.

Comment 5

[Andreu Botella]

Attachment: Bug 1459859 - Treat an empty <input type=file> as an empty File in FormData. r=smaug

This patch changes the value of an empty <input type=file> control as obtained from the FormData API, which used to be an empty string, to be a File object with empty contents, whose name is the empty string and whose type is "application/octet-stream".

Comment 6

[Andreu Botella]

While I was working on this, I noticed that in Chromium and Webkit, getting the lastModified attribute of a File object corresponding to the empty <input type=file> returns the current date and time every time (identical to calling Date.now()). This is not specified in the HTML spec, but the File API spec requires implementations to do that if the last modification time isn't known.

I've looked through the BlobImpl subclasses, and none that's already defined seem to do that (instead throwing, I believe, if the filesystem returns an error). I suspect the webcompat risk for this is zero, so it could be done at some later point, or in a separate revision.

Comment 7

[Andreu Botella]

The revision was approved over two weeks ago, but I don't have permissions to land it. Is there any reason why it wasn't landed at the time?

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Because no one knew it needs to land? :)
(I don't get any email from phabricator)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/57563a99fe53
Treat an empty <input type=file> as an empty File in FormData. r=smaug

Comment 10

[Natalia Csoregi [:nataliaCs]]

Backed out for failures on test_ext_webrequest_upload.html

backout: https://hg.mozilla.org/integration/autoland/rev/800be82c412211573a79692e6fec2b7f7ba12240

push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=57563a99fe53e03cd730ccdfc8840c961594f3ba&searchStr=mochitest-plain&selectedTaskRun=AFdmaXR3TxyCOr2EWl7miA.0

failure log: https://treeherder.mozilla.org/logviewer?job_id=334274699&repo=autoland&lineNumber=6215

[task 2021-03-24T11:52:39.231Z] 11:52:39 INFO - TEST-PASS | toolkit/components/extensions/test/mochitest/test_ext_webrequest_upload.html | Intercepted upload http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=%7B%22%5C%22special%5C%22+ch%EF%BF%BDrs%22%3A%5B%22sp%EF%BF%BDcial%22%5D%2C%22testFile%22%3A%5B%22testFile.pdf%22%5D%2C%22emptyFile%22%3A%5B%7B%7D%5D%2C%22textInput1%22%3A%5B%22value1%22%5D%7D&enctype=multipart%2Fform-data #943 {""special" chrs":["spcial"],"testFile":["testFile.pdf"],"emptyFile":[{}],"textInput1":["value1"]} have a requestBody
[task 2021-03-24T11:52:39.231Z] 11:52:39 INFO - Buffered messages finished
[task 2021-03-24T11:52:39.233Z] 11:52:39 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/mochitest/test_ext_webrequest_upload.html | Upload http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=%7B%22%5C%22special%5C%22+ch%EF%BF%BDrs%22%3A%5B%22sp%EF%BF%BDcial%22%5D%2C%22testFile%22%3A%5B%22testFile.pdf%22%5D%2C%22emptyFile%22%3A%5B%7B%7D%5D%2C%22textInput1%22%3A%5B%22value1%22%5D%7D&enctype=multipart%2Fform-data #943 matches form data. - Expected: {""special" chrs":["spcial"],"testFile":["testFile.pdf"],"emptyFile":[{}],"textInput1":["value1"]}, Actual: {""special" chrs":["spcial"],"testFile":["testFile.pdf"],"emptyFile":[""],"textInput1":["value1"]}
[task 2021-03-24T11:52:39.234Z] 11:52:39 INFO - SimpleTest.ok@SimpleTest/SimpleTest.js:417:16
[task 2021-03-24T11:52:39.235Z] 11:52:39 INFO - testHandler@SimpleTest/ExtensionTestUtils.js:68:18
[task 2021-03-24T11:52:39.235Z] 11:52:39 INFO - testResult@SimpleTest/ExtensionTestUtils.js:82:18
[task 2021-03-24T11:52:39.236Z] 11:52:39 INFO - onCompleted 943 http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=%7B%22%5C%22special%5C%22+ch%EF%BF%BDrs%22%3A%5B%22sp%EF%BF%BDcial%22%5D%2C%22testFile%22%3A%5B%22testFile.pdf%22%5D%2C%22emptyFile%22%3A%5B%7B%7D%5D%2C%22textInput1%22%3A%5B%22value1%22%5D%7D&enctype=multipart%2Fform-data
[task 2021-03-24T11:52:39.239Z] 11:52:39 INFO - 944 http://mochi.test:8888/favicon.ico
[task 2021-03-24T11:52:39.240Z] 11:52:39 INFO - onCompleted 944 http://mochi.test:8888/favicon.ico
[task 2021-03-24T11:52:39.240Z] 11:52:39 INFO - GECKO(1798) | [Parent 1798, Socket Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80470002 (NS_BASE_STREAM_CLOSED): file /builds/worker/checkouts/gecko/netwerk/base/nsFileStreams.cpp:82
[task 2021-03-24T11:52:39.241Z] 11:52:39 INFO - 945 http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=%7B%22%5C%22special%5C%22+ch%EF%BF%BDrs%22%3A%5B%22sp%EF%BF%BDcial%22%5D%2C%22testFile%22%3A%5B%22testFile.pdf%22%5D%2C%22emptyFile%22%3A%5B%7B%7D%5D%2C%22textInput1%22%3A%5B%22value1%22%5D%2C%22blobAsFile%22%3A%5B%22blobAsFile.csv%22%5D%2C%22formDataField%22%3A%5B%22some+value%22%5D%7D&enctype=multipart%2Fform-data&xhr=1
[task 2021-03-24T11:52:39.242Z] 11:52:39 INFO - onBeforeRequest upload: http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=%7B%22%5C%22special%5C%22+ch%EF%BF%BDrs%22%3A%5B%22sp%EF%BF%BDcial%22%5D%2C%22testFile%22%3A%5B%22testFile.pdf%22%5D%2C%22emptyFile%22%3A%5B%7B%7D%5D%2C%22textInput1%22%3A%5B%22value1%22%5D%2C%22blobAsFile%22%3A%5B%22blobAsFile.csv%22%5D%2C%22formDataField%22%3A%5B%22some+value%22%5D%7D&enctype=multipart%2Fform-data&xhr=1 {"formData":{""special" chrs":["spcial"],"testFile":["testFile.pdf"],"emptyFile":[""],"textInput1":["value1"],"blobAsFile":["blobAsFile.csv"],"formDataField":["some value"]}}
[task 2021-03-24T11:52:39.242Z] 11:52:39 INFO - TEST-PASS | toolkit/components/extensions/test/mochitest/test_ext_webrequest_upload.html | Intercepted upload http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_WebRequest_page3.html?trigger=form&upload=%7B%22%5C%22special%5C%22+ch%EF%BF%BDrs%22%3A%5B%22sp%EF%BF%BDcial%22%5D%2C%22testFile%22%3A%5B%22testFile.pdf%22%5D%2C%22emptyFile%22%3A%5B%7B%7D%5D%2C%22textInput1%22%3A%5B%22value1%22%5D%2C%22blobAsFile%22%3A%5B%22blobAsFile.csv%22%5D%2C%22formDataField%22%3A%5B%22some+value%22%5D%7D&enctype=multipart%2Fform-data&xhr=1 #945 {""special" chrs":["spcial"],"testFile":["testFile.pdf"],"emptyFile":[{}],"textInput1":["value1"],"blobAsFile":["blobAsFile.csv"],"formDataField":["some value"]} have a requestBody

Comment 11

[Andreu Botella]

Fixed. Now the revision needs to land again.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4aaf8fe02184
Treat an empty <input type=file> as an empty File in FormData. r=smaug,robwu

Comment 13

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/4aaf8fe02184