Bug 174730

Summary: [GTK] Epiphany (GNOME Web) says "Error downloading: Service Unavailable." when trying to download an image from discogs.com
Product: WebKit Reporter: Ryan Farmer <rfarmer84>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, aperez, berto, bugs-noreply, bugzilla, cdumez, cgarcia, ews-watchlist, gustavo, mcatanzaro
Priority: P2    
Version: Other   
Hardware: PC   
OS: Linux   
See Also: https://bugs.webkit.org/show_bug.cgi?id=175786
Attachments:
Description Flags
Patch
none
Try to fix mac builds none

Description Ryan Farmer 2017-07-21 15:21:27 PDT
Epiphany (GNOME Web) says "Error downloading: Service Unavailable." when trying to download an image from discogs.com.

Previously, the browser would crash, but the issue causing that was marked fixed in Epiphany 3.24.3.

I tried reopening the bug about the crash, but was told it was a bug in Webkit and to try reporting it here.

For reference,

https://bugzilla.gnome.org/show_bug.cgi?id=778653 - Epiphany bug causing browser crashes when download fails.

https://bugzilla.redhat.com/show_bug.cgi?id=1462677 - My bug about how to reliably hit this bug and completely crash Epiphany.
Comment 1 Adrian Perez 2018-04-25 17:36:10 PDT
I can confirm that the “Error downloading: Service Unavailable”
message when trying to download images from discogs.com still
happens with WebKitGTK+ 2.20.1 + Epiphany 3.28.1.1

The crashes seem to be gone, though — I cannot reproduce them
anymore.
Comment 2 Michael Catanzaro 2018-04-25 19:04:32 PDT
This one's probably because the referrer header isn't sent for downloads
Comment 3 Michael Catanzaro 2018-04-25 19:08:44 PDT
When we create a WebKitDownload, it's totally dissociated from the main resource that you were looking at when you decided to download the thing. That's different from other browsers. It means there's no referrer header, and some websites rely on that to guess whether you're a human or a scraper to block scrapers. I'm not sure if it's even possible to fix WebKitDownload....
Comment 4 Carlos Garcia Campos 2018-04-30 00:43:17 PDT
This is not because of the referrer, but once again because of the user agent. We are not setting the User-Agent HTTP header for download requests (only when resource load is converted to a download). I agree we should probably set the referrer too, but that's not this bug. Downloads are not totally dissociated from the main resource, they are created with an optional initiating page that is indeed used to set the first party domain for cookies, for example. A download started by the context menu always has an initiating page. When using the GTK+ API you can use webkit_web_view_download_uri() instead of webkit_web_context_download_uri() to ensure the download will be created with an initiating page.
Comment 5 Carlos Garcia Campos 2018-04-30 02:08:31 PDT
Created attachment 339107 [details]
Patch
Comment 6 EWS Watchlist 2018-04-30 02:09:52 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 7 Michael Catanzaro 2018-04-30 07:55:35 PDT
Comment on attachment 339107 [details]
Patch

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

r=me on the GTK changes. WebProcessPool changes need an owner

It broke all the Apple builds:

Last 500 characters of output:
asHTTPHeaderField(WebCore::HTTPHeaderName) const", referenced from:
      WebKit::WebProcessPool::download(WebKit::WebPageProxy*, WebCore::ResourceRequest const&, WTF::String const&) in WebProcessPool.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

> Source/WebKit/UIProcess/WebProcessPool.cpp:1217
> +            if (!updatedRequest.hasHTTPHeaderField(HTTPHeaderName::UserAgent))
> +                updatedRequest.setHTTPUserAgent(initiatingPage->userAgent());

I guess we should get Bastien to test to see if this fixes gogs.com, or if we need to set the referer here as well. There's a lot of security policy needed when setting referer, though, so I guess leaving that to a future patch makes sense.
Comment 8 Carlos Garcia Campos 2018-04-30 08:26:15 PDT
Created attachment 339109 [details]
Try to fix mac builds
Comment 9 Bastien Nocera 2018-04-30 08:31:57 PDT
https://bugs.webkit.org/show_bug.cgi?id=175786 might be a dupe.
Comment 10 Bastien Nocera 2018-04-30 08:33:36 PDT
(In reply to Michael Catanzaro from comment #7)
> I guess we should get Bastien to test to see if this fixes gogs.com, or if
> we need to set the referer here as well. There's a lot of security policy
> needed when setting referer, though, so I guess leaving that to a future
> patch makes sense.

Are you confusing discogs.com with gog.com? If so, I think I have a separate bug about that somewhere. Otherwise the URL in the previous comment is it.
Comment 11 Carlos Garcia Campos 2018-04-30 08:39:10 PDT
I can't reproduce the problem with Arcade Flyer Archive event without this patch, so I don't know it it was the same problem.
Comment 12 Carlos Garcia Campos 2018-05-03 01:37:58 PDT
Alex, this patch requires a WebKit2 owner approval, could you check it, please?
Comment 13 Bastien Nocera 2018-05-03 02:35:39 PDT
(In reply to Carlos Garcia Campos from comment #11)
> I can't reproduce the problem with Arcade Flyer Archive event without this
> patch, so I don't know it it was the same problem.

I can reproduce it on discogs still, see:
https://bugs.webkit.org/show_bug.cgi?id=175786#c5
Comment 14 Chris Dumez 2018-05-03 09:11:33 PDT
Comment on attachment 339109 [details]
Try to fix mac builds

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

> Source/WebKit/UIProcess/WebProcessPool.cpp:1216
> +            if (!updatedRequest.hasHTTPHeaderField(HTTPHeaderName::UserAgent))

WebKit2 owner LGTM.
Comment 15 Carlos Garcia Campos 2018-05-04 03:06:08 PDT
(In reply to Chris Dumez from comment #14)
> Comment on attachment 339109 [details]
> Try to fix mac builds
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=339109&action=review
> 
> > Source/WebKit/UIProcess/WebProcessPool.cpp:1216
> > +            if (!updatedRequest.hasHTTPHeaderField(HTTPHeaderName::UserAgent))
> 
> WebKit2 owner LGTM.

Thanks!
Comment 16 Carlos Garcia Campos 2018-05-04 03:14:41 PDT
Committed r231350: <https://trac.webkit.org/changeset/231350>