Bug 156099 – <a download> does not support Blob URLs

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Brent Fulgham]

The current implementation of <a download> fails to handle Blob URLs properly.

Tested by: http/tests/security/anchor-download-allow-blob.html

[Brent Fulgham]

This is happening because the following fails:

void NavigationState::NavigationClient::decidePolicyForNavigationAction:

(Line 303):
            if ([NSURLConnection canHandleRequest:nsURLRequest.get()]) {

Need to see how Blob URLs are handled elsewhere.

[Radar WebKit Bug Importer]

<rdar://problem/25535520>

[Brady Eidson]

(In reply to comment #1)
> This is happening because the following fails:
> 
> void NavigationState::NavigationClient::decidePolicyForNavigationAction:
> 
> (Line 303):
>             if ([NSURLConnection canHandleRequest:nsURLRequest.get()]) {
> 
> Need to see how Blob URLs are handled elsewhere.

I don't think this is the problem. A known download should never be a naviagtion in the first place.

[Chris Dumez]

I have a WIP patch that seems to work. I'll get it ready for review.

[Chris Dumez]

Created attachment 289736 [details]
WIP patch

[Chris Dumez]

Created attachment 289737 [details]
WIP Patch

[Chris Dumez]

Test page:
http://jsfiddle.net/cW7W5/1579/

[Chris Dumez]

Created attachment 289739 [details]
WIP Patch (with tests)

[Chris Dumez]

Created attachment 289740 [details]
WIP Patch (with more tests)

[Build Bot]

Comment on attachment 289740 [details]
WIP Patch (with more tests)

Attachment 289740 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2135745

New failing tests:
http/tests/security/anchor-download-allow-blob.html
fast/dom/HTMLAnchorElement/anchor-download-unset.html
fast/dom/HTMLAnchorElement/anchor-nodownload.html

[Build Bot]

Created attachment 289741 [details]
Archive of layout-test-results from ews125 for ios-simulator-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Chris Dumez]

Created attachment 289745 [details]
WIP Patch (with more tests)

[Chris Dumez]

Created attachment 289756 [details]
Patch

[Darin Adler]

Comment on attachment 289756 [details]
Patch

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

> Source/WebKit2/NetworkProcess/Downloads/BlobDownloadClient.h:45
> +    // ResourceHandleClient
> +    void didReceiveResponse(WebCore::ResourceHandle*, WebCore::ResourceResponse&&) final;
> +    void didReceiveBuffer(WebCore::ResourceHandle*, Ref<WebCore::SharedBuffer>&&, int reportedEncodedDataLength) final;
> +    void didFinishLoading(WebCore::ResourceHandle*, double finishTime) final;
> +    void didFail(WebCore::ResourceHandle*, const WebCore::ResourceError&) final;

I would have put these overrides inside the private section, since nobody needs to call them directly, only polymorphically through a ResourceHandleClient& to this object.

[Chris Dumez]

Created attachment 289765 [details]
Patch

[Chris Dumez]

*** Bug 156180 has been marked as a duplicate of this bug. ***

[WebKit Commit Bot]

Comment on attachment 289765 [details]
Patch

Clearing flags on attachment: 289765

Committed r206356: <http://trac.webkit.org/changeset/206356>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

This made tests crash on bots: https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/r206363%20(8387)/results.html

Chris, are you available to take a look?

[Chris Dumez]

(In reply to comment #19)
> This made tests crash on bots:
> https://build.webkit.org/results/
> Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/r206363%20(8387)/results.html
> 
> Chris, are you available to take a look?

Yes, looking.

[Chris Dumez]

(In reply to comment #20)
> (In reply to comment #19)
> > This made tests crash on bots:
> > https://build.webkit.org/results/
> > Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/r206363%20(8387)/results.html
> > 
> > Chris, are you available to take a look?
> 
> Yes, looking.

I found the problem. I'll upload a patch shortly.

[Chris Dumez]

(In reply to comment #21)
> (In reply to comment #20)
> > (In reply to comment #19)
> > > This made tests crash on bots:
> > > https://build.webkit.org/results/
> > > Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/r206363%20(8387)/results.html
> > > 
> > > Chris, are you available to take a look?
> > 
> > Yes, looking.
> 
> I found the problem. I'll upload a patch shortly.

Patch to address the crashes is up for review at https://bugs.webkit.org/show_bug.cgi?id=162541.