Bug 156099

Summary:

<a download> does not support Blob URLs

Product:

WebKit

Reporter:

Brent Fulgham <bfulgham>

Component:

WebKit Misc.

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, adam, allan.jardine, ap, beidson, bfulgham, cdumez, cgarcia, chrisjshull, commit-queue, roustem, wanderingcoder, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

URL:

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

See Also:

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

Bug Depends on:

156057, 156180, 162541, 162543

Bug Blocks:

156056, 162535

Attachments:

Description	Flags
WIP patch	none
WIP Patch	none
WIP Patch (with tests)	none
WIP Patch (with more tests)	buildbot: commit-queue-
Archive of layout-test-results from ews125 for ios-simulator-elcapitan-wk2	none
WIP Patch (with more tests)	none
Patch	none
Patch	none

Brent Fulgham

Reported 2016-03-31 23:54:45 PDT

The current implementation of <a download> fails to handle Blob URLs properly. Tested by: http/tests/security/anchor-download-allow-blob.html

Attachments
WIP patch (24.71 KB, patch) 2016-09-23 20:26 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
WIP Patch (24.72 KB, patch) 2016-09-23 20:29 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
WIP Patch (with tests) (29.46 KB, patch) 2016-09-23 22:48 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
WIP Patch (with more tests) (34.06 KB, patch) 2016-09-23 23:14 PDT, Chris Dumez	buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews125 for ios-simulator-elcapitan-wk2 (deleted) 2016-09-24 00:39 PDT, Build Bot	no flags	Details
WIP Patch (with more tests) (32.64 KB, patch) 2016-09-24 09:25 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (38.25 KB, patch) 2016-09-24 13:42 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (38.65 KB, patch) 2016-09-24 19:08 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.

Brent Fulgham

Comment 1 2016-04-03 23:50:39 PDT

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

Comment 2 2016-04-04 13:20:42 PDT

<rdar://problem/25535520>

Brady Eidson

Comment 3 2016-04-05 11:36:51 PDT

(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

Comment 4 2016-09-23 18:31:45 PDT

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

Chris Dumez

Comment 5 2016-09-23 20:26:13 PDT

Created attachment 289736 [details] WIP patch

Chris Dumez

Comment 6 2016-09-23 20:29:45 PDT

Created attachment 289737 [details] WIP Patch

Chris Dumez

Comment 7 2016-09-23 20:49:21 PDT

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

Chris Dumez

Comment 8 2016-09-23 22:48:21 PDT

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

Chris Dumez

Comment 9 2016-09-23 23:14:42 PDT

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

Build Bot

Comment 10 2016-09-24 00:39:02 PDT

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

Comment 11 2016-09-24 00:39:07 PDT

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

Comment 12 2016-09-24 09:25:22 PDT

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

Chris Dumez

Comment 13 2016-09-24 13:42:20 PDT

Created attachment 289756 [details] Patch

Darin Adler

Comment 14 2016-09-24 17:54:06 PDT

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

Comment 15 2016-09-24 19:08:19 PDT

Created attachment 289765 [details] Patch

Chris Dumez

Comment 16 2016-09-24 19:09:20 PDT

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

WebKit Commit Bot

Comment 17 2016-09-24 19:41:57 PDT

Comment on attachment 289765 [details] Patch Clearing flags on attachment: 289765 Committed r206356: <http://trac.webkit.org/changeset/206356>

WebKit Commit Bot

Comment 18 2016-09-24 19:42:02 PDT

All reviewed patches have been landed. Closing bug.

Alexey Proskuryakov

Comment 19 2016-09-25 17:37:54 PDT

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

Comment 20 2016-09-25 17:38:21 PDT

(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

Comment 21 2016-09-25 17:51:19 PDT

(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

Comment 22 2016-09-25 19:07:32 PDT

(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.

Note You need to log in before you can comment on or make changes to this bug.