Bug 161943 – input.type cannot be set to "file" after being set to another type

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Chris Dumez]

input.type cannot be set to "file" after being set to another type. This behavior does not match the HTML specification or the behavior of Firefox and Chrome.

[Chris Dumez]

Created attachment 288767 [details]
Patch

[Build Bot]

Comment on attachment 288767 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/html/semantics/forms/the-input-element/type-change-state.html
imported/w3c/web-platform-tests/html/dom/reflection-forms.html

[Build Bot]

Created attachment 288780 [details]
Archive of layout-test-results from ews121 for ios-simulator-elcapitan-wk2

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

[Chris Dumez]

Created attachment 288823 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 288823 [details]
Patch

Rejecting attachment 288823 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 288823, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
ayoutTests/imported/w3c/ChangeLog
error: Error building trees

Failed to run "['git', 'commit', '--all', '-F', '-']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

You have both LayoutTests/imported/w3c and LayoutTests/imported/w3c/ChangeLog
You have both LayoutTests/imported/w3c and LayoutTests/imported/w3c/ChangeLog
error: Error building trees

Failed to run "['git', 'commit', '--all', '-F', '-']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
Current branch master is up to date.

Full output: http://webkit-queues.webkit.org/results/2072941

[Chris Dumez]

Comment on attachment 288823 [details]
Patch

Clearing flags on attachment: 288823

Committed r205912: <http://trac.webkit.org/changeset/205912>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

This was originally done for security reasons. I want to be sure there is no way to get a file input into a state where it points to an arbitrary file because it is first set into a state while it has another type.

[Chris Dumez]

(In reply to comment #8)
> This was originally done for security reasons. I want to be sure there is no
> way to get a file input into a state where it points to an arbitrary file
> because it is first set into a state while it has another type.

The comment was saying this was security reasons but also indicated that this would not work in WebKit anyway and was there for compatibility reasons. Also note that I added a layout test that covers the case that was mentioned in the original comment.

[Darin Adler]

Sounds like you have the bases covered. I would also look at the various functions and try to think about whether there's any restrictions we put on file elements once their type is set that could somehow be bypassed by this.

[Chris Dumez]

(In reply to comment #10)
> Sounds like you have the bases covered. I would also look at the various
> functions and try to think about whether there's any restrictions we put on
> file elements once their type is set that could somehow be bypassed by this.

FileInputType does not allow setting the value to anything else than the empty string:
bool FileInputType::canSetValue(const String& value)
{
    // For security reasons, we don't allow setting the filename, but we do allow clearing it.
    // The HTML5 spec (as of the 10/24/08 working draft) says that the value attribute isn't
    // applicable to the file upload control at all, but for now we are keeping this behavior
    // to avoid breaking existing websites that may be relying on this.
    return value.isEmpty();
}

In any case, the value attribute actually does not matter when the input type is "file" because what we actually rely on is FileInputType::m_fileList which is set by FileInputType::setFiles(), which is set by FileInputType::filesChosen() which is called by the FileChooser.
We don't construct m_fileList from the value ever. Whe requesting the value of a FileInputType, we use: "C:\\fakepath\\" + m_fileList->item(0)->name().