[webkit-reviews] review granted: [Bug 132523] http/tests/security/xss-DENIED-xsl-document-redirect.xml fails with NetworkProcess : [Attachment 231036] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 132523: http/tests/security/xss-DENIED-xsl-document-redirect.xml fails with
NetworkProcess
https://bugs.webkit.org/show_bug.cgi?id=132523

Attachment 231036: Patch
https://bugs.webkit.org/attachment.cgi?id=231036&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=231036&action=review


Nice!

> Source/WebCore/xml/XSLTProcessorLibxslt.cpp:132
> -	       requestAllowed =
globalCachedResourceLoader->document()->securityOrigin()->canRequest(response.u
rl());
> +	       if (error.isNull())
> +		   requestAllowed =
globalCachedResourceLoader->document()->securityOrigin()->canRequest(response.u
rl());

I think that this introduces an incorrect behavior in the following scenario:
1. A cross-origin redirect occurs.
2. The resource starts to load, and then connection is dropped. If this happens
at an appropriate time, the data could still be a well-formed XML document.

As error is not null, we don't do the canRequest check, and proceed using the
data.

The function is structured strangely, so I'm not sure if we can do good error
handling here (and we don't report more common errors anyway already). Please
add "else data.clear();", and post a patch for EWS to run tests with that.