Bug 161943

Summary: input.type cannot be set to "file" after being set to another type
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: DOMAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, cdumez, commit-queue, darin, dbates, rniwa, sam
Priority: P2 Keywords: WebExposed
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews121 for ios-simulator-elcapitan-wk2
none
Patch none

Chris Dumez
Reported 2016-09-13 19:39:34 PDT
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.
Attachments
Patch (33.87 KB, patch)
2016-09-13 21:16 PDT, Chris Dumez
no flags
Archive of layout-test-results from ews121 for ios-simulator-elcapitan-wk2 (deleted)
2016-09-14 01:00 PDT, Build Bot
no flags
Patch (51.68 KB, patch)
2016-09-14 08:56 PDT, Chris Dumez
no flags
Chris Dumez
Comment 1 2016-09-13 21:16:28 PDT
Build Bot
Comment 2 2016-09-14 01:00:11 PDT
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
Comment 3 2016-09-14 01:00:15 PDT
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
Comment 4 2016-09-14 08:56:49 PDT
WebKit Commit Bot
Comment 5 2016-09-14 09:44:10 PDT
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 6 2016-09-14 09:58:14 PDT
Comment on attachment 288823 [details] Patch Clearing flags on attachment: 288823 Committed r205912: <http://trac.webkit.org/changeset/205912>
Chris Dumez
Comment 7 2016-09-14 09:58:19 PDT
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 8 2016-09-14 10:05:37 PDT
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
Comment 9 2016-09-14 10:07:12 PDT
(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
Comment 10 2016-09-14 12:00:57 PDT
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
Comment 11 2016-09-14 14:56:13 PDT
(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().
Note You need to log in before you can comment on or make changes to this bug.