Followup to https://bugs.webkit.org/show_bug.cgi?id=174803 now that https://bugs.webkit.org/show_bug.cgi?id=95698 is fixed.
Created attachment 316983 [details] Patch
Comment on attachment 316983 [details] Patch Attachment 316983 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4241503 New failing tests: imported/w3c/web-platform-tests/fetch/api/cors/cors-no-preflight.any.worker.html imported/w3c/web-platform-tests/fetch/api/cors/cors-basic.any.html imported/w3c/web-platform-tests/fetch/api/cors/cors-basic.any.worker.html imported/w3c/web-platform-tests/fetch/api/cors/cors-origin.any.worker.html imported/w3c/web-platform-tests/fetch/api/cors/cors-origin.any.html imported/w3c/web-platform-tests/fetch/api/cors/cors-no-preflight.any.html
Created attachment 316991 [details] Archive of layout-test-results from ews105 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 316983 [details] Patch Attachment 316983 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4243532 New failing tests: fast/dom/StyleSheet/detached-sheet-owner-node-link.html
Created attachment 317075 [details] Archive of layout-test-results from ews114 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 316983 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316983&action=review r=me, but you may want to tighten up the extension checking. > Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:540 > String extension = filename.substring(filename.reverseFind('.') + 1); There must be a better way to get a file's extension. However this does pretty much match what `File::computeNameAndContentType` does it in WebCore/fileapi/File.cpp. • What if this is a hidden file: ~/.bashrc • What if this doesn't have an extension? /tmp/foo • Is the filename here just a name or a path? /tmp/.hidden/foo I think you will still at the very least want to handle if reverseFind returns WTF::notFound.
(In reply to Joseph Pecoraro from comment #6) > Comment on attachment 316983 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=316983&action=review > > r=me, but you may want to tighten up the extension checking. > > > Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:540 > > String extension = filename.substring(filename.reverseFind('.') + 1); > > There must be a better way to get a file's extension. > > However this does pretty much match what `File::computeNameAndContentType` > does it in WebCore/fileapi/File.cpp. > > • What if this is a hidden file: ~/.bashrc > • What if this doesn't have an extension? /tmp/foo > • Is the filename here just a name or a path? /tmp/.hidden/foo MIME type inference will fail in all of these cases, so it doesn't really matter. > I think you will still at the very least want to handle if reverseFind > returns WTF::notFound. Yep.
Committed r220222: <http://trac.webkit.org/changeset/220222>
<rdar://problem/33707206>
Comment on attachment 316983 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316983&action=review >>> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:540 >>> String extension = filename.substring(filename.reverseFind('.') + 1); >> >> There must be a better way to get a file's extension. >> >> However this does pretty much match what `File::computeNameAndContentType` does it in WebCore/fileapi/File.cpp. >> >> • What if this is a hidden file: ~/.bashrc >> • What if this doesn't have an extension? /tmp/foo >> • Is the filename here just a name or a path? /tmp/.hidden/foo >> >> I think you will still at the very least want to handle if reverseFind returns WTF::notFound. > > MIME type inference will fail in all of these cases, so it doesn't really matter. I think it’s not great style to do this rather than using a helper function that can do it right. > Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:541 > + if (allowedFileExtensions.contains(extension)) Is this meant to be case sensitive? Or ASCII case insensitive?
Comment on attachment 316983 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316983&action=review >>>> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:540 >>>> String extension = filename.substring(filename.reverseFind('.') + 1); >>> >>> There must be a better way to get a file's extension. >>> >>> However this does pretty much match what `File::computeNameAndContentType` does it in WebCore/fileapi/File.cpp. >>> >>> • What if this is a hidden file: ~/.bashrc >>> • What if this doesn't have an extension? /tmp/foo >>> • Is the filename here just a name or a path? /tmp/.hidden/foo >>> >>> I think you will still at the very least want to handle if reverseFind returns WTF::notFound. >> >> MIME type inference will fail in all of these cases, so it doesn't really matter. > > I think it’s not great style to do this rather than using a helper function that can do it right. The patch I landed handles the notFound case. >> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:541 >> + if (allowedFileExtensions.contains(extension)) > > Is this meant to be case sensitive? Or ASCII case insensitive? This is a great point that Joe and I both missed, and it applies to the MIME types as well. I'll fix this in a followup. WebCore does convertToASCIILowercase on the accept values, so we need to do that for the filenames that we get via WebDriver and any MIME types that we infer. https://html.spec.whatwg.org/multipage/input.html#file-upload-state-(type=file)