Bug 103902

Summary: xml-stylesheet XSL is not requested with JavaScript disabled
Product: WebKit Reporter: Jan Pingel <jpingel>
Component: XMLAssignee: Vivek Galatage <vivek.vg>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, eric, japhet, laszlo.gombos, mkwst+watchlist, vivekg, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://5381studios.com/webkit%20xslt%20js-dsisabled%20testcase/test.xml
Attachments:
Description Flags
XML and XSL testcase
none
Patch none

Jan Pingel
Reported 2012-12-03 09:03:59 PST
Loading an XML page with xml-stylesheet with XSL fails to load when JavaScript is disabled. Steps to reproduce: 1) Disable JavaScript 2) Open the attached test case or view it online from: http://5381studios.com/webkit%20xslt%20js-dsisabled%20testcase/test.xml 3) With JS disabled you will not see the XSL transformation result displaying the text "test" on the page. If you open the developer tools and check the network requests it will fail to show any XSL being fetched. Viewing the test case with JavaScript enabled results in the expected XSL transformation and the XSL loading under the network tab of the developer tools. Fails under: Webkit nightly Version 6.0.2 (8536.26.17, 537+) Safari Version 6.0.2 (8536.26.17) Chrome Version 24.0.1312.27 beta Works under: Firefox 17.0.1 Opera 12.10 Internet Explorer 7, 8, 9, 10
Attachments
XML and XSL testcase (2.51 KB, application/zip)
2012-12-03 09:04 PST, Jan Pingel 		no flags
Details
Patch (5.65 KB, patch)
2013-01-22 22:52 PST, Vivek Galatage 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Jan Pingel
Comment 1 2012-12-03 09:04:41 PST
Created attachment 177266 [details] XML and XSL testcase
Jan Pingel
Comment 2 2012-12-06 15:49:46 PST
@Alexey: What revision was this fixed in?
Alexey Proskuryakov
Comment 3 2012-12-06 16:12:38 PST
Sorry, I was trying to mark it confirmed, not fixed.
Vivek Galatage
Comment 4 2013-01-21 12:25:52 PST
I am investigating this issue and found the CachedResourceLoader::canRequest() method has a follow through for the CachedResource::XSLStyleSheet type onto CachedResource::Script in the switch case. But I guess it's a side effect of switch follow through that the XSLT is also blocked when the javascript is blocked. The XSLT style sheet is made in sync with Script as per [1]. But as can be seen in the attached patch [2], there was no check existed for script being enabled/disabled. Whereas the latest code [3] has this check and I think because of this check the transformation is not completed when javascript is disabled. I would be glad to receive the inputs about the findings above and if these assumptions are correct, then we can have the switch case for XSLStyleSheet as: switch (type) { #if ENABLE(XSLT) case CachedResource::XSLStyleSheet: if (!shouldBypassMainWorldContentSecurityPolicy && !m_document->contentSecurityPolicy()->allowStyleFromSource(url)) return false; break; #endif ... } The above fix works fine and the transformation is successful when the javascript is blocked. But I am unaware of any security holes this might open up. Hence requesting about the feedback. Also I need to add the test case(s) depicting the above scenario. I will add all these sooner in a separate patch. Thank you. [1] https://bugs.webkit.org/show_bug.cgi?id=63637 [2] https://bugs.webkit.org/attachment.cgi?id=110889&action=review [3] http://trac.webkit.org/browser/trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp#L362
Adam Barth
Comment 5 2013-01-22 15:58:08 PST
I see. We should probably keep XSLStyleSheet calling allowScriptFromSource on ContentSecurityPolicy but we shouldn't call allowScriptFromSource on FrameLoaderClient. That seems a bit strange, but I think that's what we're supposed to do.
Vivek Galatage
Comment 6 2013-01-22 16:03:51 PST
(In reply to comment #5) Thank you Adam for your comments. > I see. We should probably keep XSLStyleSheet calling allowScriptFromSource on ContentSecurityPolicy but we shouldn't call allowScriptFromSource on FrameLoaderClient. That seems a bit strange, but I think that's what we're supposed to do. Sure, I will change it accordingly.
Vivek Galatage
Comment 7 2013-01-22 22:52:27 PST
Created attachment 184148 [details] Patch
Adam Barth
Comment 8 2013-01-22 23:01:47 PST
Comment on attachment 184148 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=184148&action=review > LayoutTests/http/tests/xsl/xslt-transform-with-javascript-disabled.html:20 > +<body onLoad="shouldNotBeExecuted();"> I like the control
WebKit Review Bot
Comment 9 2013-01-23 01:03:30 PST
Comment on attachment 184148 [details] Patch Clearing flags on attachment: 184148 Committed r140522: <http://trac.webkit.org/changeset/140522>
WebKit Review Bot
Comment 10 2013-01-23 01:03:34 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.