[webkit-reviews] review denied: [Bug 74706] [Qt] QtWebKit disregards LocalContentCanAccessFileUrls setting : [Attachment 136925] Proposed patch + layout test + Qt auto test (v4)

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Bruno Abinader
<bruno.de_oliveira at basyskom.de>'s request for review:
Bug 74706: [Qt] QtWebKit disregards LocalContentCanAccessFileUrls setting
https://bugs.webkit.org/show_bug.cgi?id=74706

Attachment 136925: Proposed patch + layout test + Qt auto test (v4)
https://bugs.webkit.org/attachment.cgi?id=136925&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=136925&action=review


> Source/WebCore/loader/SubframeLoader.cpp:282
> +    if (Settings* settings = m_frame->settings()) {
> +	   // Document is allowed to load local resources, but the settings
> +	   // explicitly disables local file access.
> +	   if (ownerElement->document()->securityOrigin()->isLocal() &&
!settings->allowFileAccessFromFileURLs()) {
> +	       RefPtr<SecurityOrigin> targetOrigin =
SecurityOrigin::create(url);
> +	       if (!targetOrigin.get()->isUnique() &&
!document()->securityOrigin()->canAccess(targetOrigin.get())) {
> +		   FrameLoader::reportLocalLoadFailed(m_frame, url.string());
> +		   return 0;
> +	       }
> +	   }
> +    }

This isn't the right place for this logic.  The fix for this bug should be
inside the SecurityOrigin::canDisplay call on the previous line.

I don't fully understand what the problem is, but I suspect this isn't the
right way to fix this issue.  Hopefully the next iteration of this patch will
make things clearer.