The fix in Bug 167696 was not correct. Rather than adding calls to URL decoding in the FileSystem class, we should be making sure that the callers of these functions are passing well-formed file paths. This change fixes the calls sites in SecurityOrigin to avoid this problem, and removes the improperly URL decoding from FileSystem.cpp.
<rdar://problem/30380075>
<rdar://problem/30380080>
Created attachment 300761 [details] Patch
Comment on attachment 300761 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=300761&action=review > Tools/TestWebKitAPI/Tests/WebCore/FileSystem.cpp:69 > + system("mkdir resources"); > + system("echo \"<!DOCTYPE html><html><body>Hello</body></html>\" > resources/CrossPartitionFileTest.html"); > + system("hdiutil create otherVolume.dmg -srcfolder resources/ -ov > /dev/null"); > + system("hdiutil attach otherVolume.dmg > /dev/null"); This seems more invasive than I'd like the tests to be. Seems likely to cause more problems that it's worth it.
(In reply to comment #4) > Comment on attachment 300761 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=300761&action=review > > > Tools/TestWebKitAPI/Tests/WebCore/FileSystem.cpp:69 > > + system("mkdir resources"); > > + system("echo \"<!DOCTYPE html><html><body>Hello</body></html>\" > resources/CrossPartitionFileTest.html"); > > + system("hdiutil create otherVolume.dmg -srcfolder resources/ -ov > /dev/null"); > > + system("hdiutil attach otherVolume.dmg > /dev/null"); > > This seems more invasive than I'd like the tests to be. Seems likely to > cause more problems that it's worth it. Do you have a suggestion for creating multiple volumes during testing? Or do you think the potential downside means we should not test this code path?
> Or do you think the potential downside means we should not test this code path? That would be my inclination, yes.
Created attachment 300773 [details] Patch (removed volume-creation test logic)
Note: The Windows build failure seems to be due to a bad commit to WebCore::CredentialStorage, and not this particular patch.
Committed r211763: <http://trac.webkit.org/changeset/211763>