Check for empty scheme before accessing URLSchemeMaps in SchemeRegistry
Created attachment 82159 [details] Patch
Comment on attachment 82159 [details] Patch This change is correct, but can you please construct a test case that shows the problem? Normally we require regression tests for all bug fixes.
Comment on attachment 82159 [details] Patch Rejecting attachment 82159 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-4', 'land-a..." exit_code: 2 Last 500 characters of output: ebKit 2d3c60d..e45ab5e master -> origin/master M LayoutTests/platform/chromium/test_expectations.txt M LayoutTests/ChangeLog M Tools/DumpRenderTree/TestNetscapePlugIn/main.cpp M Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp M Tools/ChangeLog M Tools/Scripts/webkitpy/layout_tests/port/chromium_win.py r78359 = e45ab5efb940172a265712fe69fb923eef21d2df (refs/remotes/trunk) First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/trunk. Full output: http://queues.webkit.org/results/7869732
Thanks for the quick review. It's not currently possible to exercise this code with a layout test. For shouldTreatURLSchemeAsSecure, the only caller is (in FrameLoader.cpp): if (!url.isValid || SchemeRegistry::shouldTreatURLSchemeAsSecure(url.protocol()) so the case is never hit. The situation is similar for shouldLoadURLSchemeAsSecure in MainResourceLoader.cpp: return url.isEmpty() || SchemeRegistry::shouldLoadURLSchemeAsEmptyDocument(url.protocol()); Finally, shouldTreatURLSchemeAsNoAccess has no callers at all. Nevertheless, I think this patch is a good idea, to avoid new, less careful callers causing crashes. I just ran into this problem when trying to get https://bugs.webkit.org/show_bug.cgi?id=53529 landed (SecurityOrigin.cpp calls these methods without pre-validating the scheme). And it appears to me that Adam Barth recently resolved the same issue for another method in http://trac.webkit.org/changeset/76637.
Comment on attachment 82159 [details] Patch I think the cq might be sick. It did that to me yesterday as well.
Comment on attachment 82159 [details] Patch Clearing flags on attachment: 82159 Committed r78367: <http://trac.webkit.org/changeset/78367>
All reviewed patches have been landed. Closing bug.