Bug 94918

Created attachment 160387 [details] Patch

Comment on attachment 160387 [details] Patch I'm surprised this doesn't cause any tests to fail. Won't this mess up data URLs?

Rather than using SecurityOrigin, I wonder if we should use the hasInnerURL and extraceInnerURL functions (those names aren't 100% right).

(In reply to comment #2) > (From update of attachment 160387 [details]) > I'm surprised this doesn't cause any tests to fail. Won't this mess up data URLs? I'm pretty sure all the data tests simply check that the protocol-only source 'data:' works. In that case, the SecurityOrigin returns 'data:' as the protocol, so it works (he says, confidently, without having looked at either the tests or the code). > Rather than using SecurityOrigin, I wonder if we should use the hasInnerURL and extraceInnerURL functions (those names aren't 100% right). Neither 'shouldUseInnerURL' nor 'extractInnerURL' are public at the moment. I vaguely remember having a similar discussion when I tweaked 'blob:'s mixed-content behavior... *shrug* This patch has the advantage of not touching SecurityOrigin, but I'm happy to add those methods to SecurityOrigin's public signature if you'd like. :)

Created attachment 160572 [details] Patch

(In reply to comment #5) > Created an attachment (id=160572) [details] > Patch I've adjusted SecurityOrigin to include these two static functions for innerURL extraction. This patch also moves the innerURL magic out of CSPSource::match and into CSPSourceList::match, which should be more efficient, and adds a 'data:' URL test, as it doesn't look like we were explicitly testing it before.

Comment on attachment 160572 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=160572&action=review > Source/WebCore/page/ContentSecurityPolicy.cpp:247 > + KURL urlToMatch = SecurityOrigin::shouldUseInnerURL(url) ? SecurityOrigin::extractInnerURL(url) : url; urlToMatch -> effectiveURL > Source/WebCore/page/SecurityOrigin.cpp:195 > + if (SecurityOrigin::shouldUseInnerURL(url)) > + return adoptRef(new SecurityOrigin(SecurityOrigin::extractInnerURL(url))); "SecurityOrigin::" isn't needed here because SecurityOrigin::create is already in the SecurityOrigin namespace. > Source/WebCore/page/SecurityOrigin.cpp:225 > - if (shouldUseInnerURL(url) && SchemeRegistry::shouldTreatURLSchemeAsSecure(extractInnerURL(url).protocol())) > + if (SecurityOrigin::shouldUseInnerURL(url) && SchemeRegistry::shouldTreatURLSchemeAsSecure(SecurityOrigin::extractInnerURL(url).protocol())) ditto

Created attachment 160768 [details] Patch

Comment on attachment 160768 [details] Patch Clearing flags on attachment: 160768 Committed r126785: <http://trac.webkit.org/changeset/126785>

All reviewed patches have been landed. Closing bug.