Bug 132664 – [CSS Shapes] Same URL for display and shape-outside can cause CORS problems

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Hans Muller]

Created attachment 231022 [details]
Test case

If an ordinary image for some URL is cached, then a subsequent (and valid) cross-origin request for the same image fails, because the cached image wasn't fetched with CORS.  You can see as much in the attached example, by running the testImage() function.

CSS Shapes apps regularly aggravate this problem, because the same image URL is often used for display (without CORS) and for shape-outside (with CORS).

[Hans Muller]

Created attachment 231272 [details]
Test case.

Slightly revised the test case.  There are a few additional comments, etc.

[Alexey Proskuryakov]

I expect that this will be fairly involved to fix cleanly. WebCore loader is not suited for the same resource being used for multiple purposes, or even for resources used for the same purpose in multiple subframes. The cases that work generally do so by avoiding caching altogether.

Would it be possible to have a way to directly express this use case in markup? If it's a common case, a nice syntax for it may be appropriate.

A workaround would be to use a different URL, of course (e.g. by adding a query such as "?for-shape").

[Theresa O'Connor]

(In reply to comment #2)
> Would it be possible to have a way to directly express this use case in markup?

CSS V&U Level 3 provides for this by enhancing the attr() function:

shape-outside: attr(src url)

See also bug 26609.

[youenn fablet]

(In reply to comment #2)
> I expect that this will be fairly involved to fix cleanly. WebCore loader is not suited for the same resource being used for multiple purposes, or even for resources used for the same purpose in multiple subframes. The cases that work generally do so by avoiding caching altogether.

Resources should probably not be reused when the request origin policy options are not the same.
How about mandating the loader to reload whenever the HTTP origin of the new request does not match the origin of the cached one?

[youenn fablet]

Created attachment 237627 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 237627 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=237627&action=review

> Source/WebCore/ChangeLog:11
> +        CachedResourceLoader now reloads cached resources for which the Origin between the cached request and the new request is not the same.

This is probably an improvement overall, however it's not too great that shape-outside resources won't be cached, and will instead cause two loads on every page load.

> LayoutTests/ChangeLog:3
> +        [CSS Shapes] Same URL for display and shape-outside can cause CORS problems

There doesn't appear to be a test for shape-outside added with this patch. Can it be tested?

[youenn fablet]

(In reply to comment #6)
> Comment on attachment 237627 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=237627&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        CachedResourceLoader now reloads cached resources for which the Origin between the cached request and the new request is not the same.
> 
> This is probably an improvement overall, however it's not too great that
> shape-outside resources won't be cached, and will instead cause two loads on
> every page load.

The changelog is in fact a simplification of what the newly added check does:
it verifies upfront whether the request requires CORS check in which case Reload happens only if cached response is failing CORS check with the request origin.
If a web site adds correct CORS information for the first request, Reload will never happen.

I can update the changelog if that helps.

An alternative/generalization of the current check would be to Reload according the Vary header of the response.


> > LayoutTests/ChangeLog:3
> > +        [CSS Shapes] Same URL for display and shape-outside can cause CORS problems
> 
> There doesn't appear to be a test for shape-outside added with this patch.
> Can it be tested?

Sure, I can try to add one.

[Brent Fulgham]

Hi Youenn. What's the status with this change? It's desperately out of date, so we can't really review it...

[youenn fablet]

(In reply to comment #8)
> Hi Youenn. What's the status with this change? It's desperately out of date,
> so we can't really review it...

I will rebase the patch. I am not yet exactly sure when will be the best time for it. Please let me know if this is somehow blocking

[youenn fablet]

I am not able to reproduce the particular bug for CSS Shapes.
AFAICS, shape-outside URLs are fetched in CORS mode but in anonymous mode, thus without credentials.
img elements load data in No-CORS mode but with credentials.
This difference triggers reloading of the cached resource.

When reloading, all necessary checks are done.

If there is no objection, I'll close that bug as fixed.