[Webkit-unassigned] [Bug 49548] WebCore cache stores duplicate copies of subresources with URL fragments

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=49548


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #76566|review?                     |review+
               Flag|                            |




--- Comment #4 from Alexey Proskuryakov <ap at webkit.org>  2010-12-14 14:24:03 PST ---
(From update of attachment 76566)
View in context: https://bugs.webkit.org/attachment.cgi?id=76566&action=review

r=me. Please double check for invalid URLs being passed around - I don't think that we have a good general policy of who should check for those, so they can creep up.

Can any assertions for !hasFragmentIdentifier() be added in code that's behind MemoryCache interface? I think that these could be quite valuable.

> WebCore/loader/cache/CachedResource.cpp:119
> -    ASSERT(url().isNull() || cache()->resourceForURL(url()) != this);
> +    ASSERT(url().isNull() || cache()->resourceForURL(KURL(ParsedURLString, url())) != this);

Is url() always a valid parsed URL string here? Otherwise, KURL(ParsedURLString, url()) can assert.

> WebCore/loader/cache/CachedResourceLoader.cpp:160
> +    return cache()->requestUserCSSStyleSheet(this, KURL(ParsedURLString, url), charset);

Is url guaranteed to be a parsed URL string? If it is, perhaps the caller could pass a KURL object?

This code looks dangerous. Note that we use completeURL() in other CachedResourceLoader methods.

> WebCore/loader/cache/MemoryCache.cpp:225
> +    if (!originalURL.hasFragmentIdentifier())
> +        return originalURL;

I would add a comment explaining that invalid URLs are returned unchanged. Perhaps even to a declaration in header, not sure.

> WebCore/loader/cache/MemoryCache.cpp:227
> +    // Data urls must be unmodified and it is also safer to keep them for custom protocols.

Isn't this a bug in data URL parsing? Sounds like these shouldn't have fragments.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.