[webkit-dev] Blob changes to SecurityOrigin.cpp

[Adam Barth]

On Fri, Sep 3, 2010 at 4:21 PM, Adam Barth <abarth at webkit.org> wrote:
> On Fri, Sep 3, 2010 at 4:16 PM, Jian Li <jianli at chromium.org> wrote:
>> On Fri, Sep 3, 2010 at 4:08 PM, Adam Barth <abarth at webkit.org> wrote:
>>> On Fri, Sep 3, 2010 at 4:02 PM, Jian Li <jianli at chromium.org> wrote:
>>> > On Fri, Sep 3, 2010 at 3:43 PM, Adam Barth <abarth at webkit.org> wrote:
>>> >> On Fri, Sep 3, 2010 at 3:19 PM, Jian Li <jianli at google.com> wrote:
>>> >> I don't quite understand what this code is trying to do:
>>> >>
>>> >> bool SecurityOrigin::canLoad(const KURL& url, const String& referrer,
>>> >> Document* document)
>>> >> {
>>> >> #if ENABLE(BLOB)
>>> >>    if (url.protocolIs("blob") && document) {
>>> >>        SecurityOrigin* documentOrigin = document->securityOrigin();
>>> >>        RefPtr<SecurityOrigin> targetOrigin =
>>> >> SecurityOrigin::create(url);
>>> >>        return documentOrigin->isSameSchemeHostPort(targetOrigin.get());
>>> >>    }
>>> >> #endif
>>> >>
>>> >> Why should canLoad care about isSameSchemeHostPort?  In the past,
>>> >> canLoad's job was to stop web sites from loading content from your
>>> >> local file system (e.g., in frames or as images).
>>> >
>>> > Per the File API spec, the blob URL should not be accessed from the page
>>> > in
>>> > other security domain. Checking isSameSchemeHostPort will help us
>>> > enforce
>>> > this policy when other page in different domain that tries to load this
>>> > URL
>>> > from the cache. Probably I need to put better comment here or maybe find
>>> > a
>>> > better place to do such check.
>>>
>>> Sorry canLoad is poorly named.  I'm working on a patch now to rename
>>> it to something more sensible (canDisplay, perhaps?).  We use canLoad
>>> to control things like <frame src="..."> and <img src="...">.  When
>>> you say "not accessible," do you mean that they shouldn't be able to
>>> be displayed with the image element?
>>
>> Yes. Let me use an example to explain this.
>> Suppose page1 (http://foo/page1) creates a blob URL "blob://id1" from an
>> image file. In page1, blob://id1 is loaded and put into cache when we
>> replace any img element. In page2 (http://bar/page2), it grabs this blob URL
>> in some way and uses it towards its img element. We should not allow this
>> happen per the File API spec. Without this check in canLoad(), page2 has no
>> problem to use it directly from the cache.
>
> Got it.  Thanks for explaining it to me.
>
> So, this code doesn't quite do what you'd like it to do.  The problem
> is that some of the callers of SecurityOrigin::canLoad assume that it
> applies only for local URLs.  That means they'll skip the check you
> added and plow ahead with some kinds of loads.

Also, have you added test cases that include redirects to blob URLs?
The code that handles redirects might not be smart enough to handle
this additional complexity.

Adam