Issue metadata
Sign in to add a comment
|
webRequestBlocking loses to Save As webpage complete
Reported by
jon.jwil...@gmail.com,
Jan 12
|
||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/71.0.3578.80 Chrome/71.0.3578.80 Safari/537.36 Steps to reproduce the problem: 1. Load attached extension 2. Visit http://hi.eewe.us/mary.html 3. Ctrl-S to save page, /tmp/mary.html What is the expected behavior? /tmp/mary_files/mary.jpg should not exist What went wrong? /tmp/mary_files/mary.jpg does exist even though the Chrome extension would like to block it. Did this work before? Yes I wish I knew. It worked in Chrome 51, and I think many Chrome after that. Chrome version: 71.0.3578.80 Channel: stable OS Version: Flash Version:
,
Jan 14
Able to reproduce the issue on reported version# 71.0.3578.80, # 71.0.3578.98 and latest chrome# 73.0.3670.0 using Mac 10.12.6 and Ubuntu 14.04, hence providing Bisect Info Bisect Info: ================ Good build: 71.0.3567.0 Bad build: 71.0.3568.0 You are probably looking for a change made after 595530 (known good), but no later than 595531 (first known bad). https://chromium.googlesource.com/chromium/src/+log/b6ebcffbb09ced4d0a544b15e4e8dfc0c0414873..c66f36032447e28a08591874395ca8fbeb47ba19 Change-Id: I90f37a46e851c04fa949a1427665ecf8286210af Reviewed-on: https://chromium-review.googlesource.com/1242296 @Karan Bhatia: Please confirm the issue and help in re-assigning if it is not related to your change. Adding ReleaseBlock-Stable for M-71, feel free to remove it if not applicable. Thanks!
,
Jan 14
[bulk update] Just a heads up, M72 stable is about 2 weeks away. This issue is marked as RB-Stable. Please take a look.
,
Jan 14
+Devlin for his thoughts on this. The extension isn't able to intercept the request since it's a browser initiated non-navigation request. We can probably change the web request api to allow extensions to intercept download requests. Considerations: - Do we think the web request api is a good place for this? We do have the downloads api which allows extensions to cancel downloads, however it won't trigger the onCreated event for subresource requests of the html file. Also, jon.jwilkes@ can you elaborate on your use case? Does the downloads api suffice for your use case?
,
Jan 14
This is interesting. I'm curious of a few things here: - When we initiate the download request, do we include user cookies, etc? - Does the "Save page as" request look at the current HTML tree and download any present subresources (rather than e.g. making a new request to the server for the full HTML file and associated resources)? If so, how tricky would it be to see which requests were canceled, and not try to fetch those? (Would that break any other use cases?) I'm not sure this need block stable, but as Karan mentioned, if you can expand on the impact and your use case, that would be helpful in prioritization.
,
Jan 14
If the downloads api does not empower an extension to block mary.jpg from being downloaded, it is insufficient. The use case is parental controls, or in my particular case, self-controls: See http://www.pluckeye.net/ .
,
Jan 14
>> When we initiate the download request, do we include user cookies, etc? Not sure, can you elaborate on how that's significant? >> - Does the "Save page as" request look at the current HTML tree and download any present subresources (rather than e.g. making a new request to the server for the full HTML file and associated resources)? It seems to be making a new request to the server (that's why the bug). >> If so, how tricky would it be to see which requests were canceled, and not try to fetch those? (Would that break any other use cases?) This would require modifying the download/save-as code and I think it would be easier to fix this on the extensions side.
,
Jan 14
>> When we initiate the download request, do we include user cookies, etc? > Not sure, can you elaborate on how that's significant? If the request includes cookies, then it's a bit more of a potential privacy risk (since extensions that might be trying to prevent certain data from being leaked to certain sites no longer could). If it's just a vanilla, uncredentialed request, there's less concern. >> Does the "Save page as" request look at the current HTML tree and download any present subresources (rather than e.g. making a new request to the server for the full HTML file and associated resources)? > It seems to be making a new request to the server (that's why the bug). Note that this might not be as cut-and-dried. It could be that we save the current HTML served, but make another request for every subresource. If that's the case, it might be easier to filter out canceled requests. If it is a full new request, it's a bit harder. >> If so, how tricky would it be to see which requests were canceled, and not try to fetch those? (Would that break any other use cases?) > This would require modifying the download/save-as code and I think it would be easier to fix this on the extensions side. Probably so, yeah. Just curious because it's nice to avoid exposing browser-initiated requests to extensions when we can (it'd be great to have it be an invariant). If we could solve this by having the save-as code check the resources of the current HTML file, it would mean we don't have to expose the browser-initiated request to the extension. (But, not the end of the world if we have to)
,
Jan 15
>> If the request includes cookies, then it's a bit more of a potential privacy risk (since extensions that might be trying to prevent certain data from being leaked to certain sites no longer could). If it's just a vanilla, uncredentialed request, there's less concern. Good point, I'll confirm. I'll dig a bit more into the implementation.
,
Jan 16
So I tried two kinds of download requests: 1. Save page (This bug) Here the subresource requests are initiated by the browser and hence they won't be intercepted by the extension due to my change (c#2). These aren't proxied with network service (I remember cduvall@ making a change after my change removing the proxying of browser initiated non navigation requests). 2. Right click, Save Link or click on a link to download (e.g. zip file). Without network service, these should be intercepted by extensions since they are renderer initiated. With network service these aren't being proxied, which seems to be a regression with network service. I think we'll want these requests to be intercepted by extensions. cduvall@/jam@ do you have context into why 2) isn't being proxied and what might it take to proxy 1) with the network service. Even after these are proxied, we'll need plumbing to identify requests which correspond to browser initiated downloads.
,
Jan 16
Also, I don't think this is critical enough to be RBS. It'll be a nice to merge a fix if possible. Removing the label. Let me know if anyone thinks differently.
,
Jan 16
(6 days ago)
This probably stopped working for network service after http://crrev.com/c/1254725. One way to solve this is to add ContentBrowserClient::WillCreateURLLoaderFactory call to proxy the URLLoaderFactory to the places where the URLLoaderFactories are used, I believe case 1) is from here[1] and case 2) is from here[2]. 1. https://cs.chromium.org/chromium/src/content/browser/download/save_file_manager.cc?l=247&rcl=96fa6c14aa7e7e8902d6cb703ea2502e0c13215c 2. https://cs.chromium.org/chromium/src/components/download/internal/common/resource_downloader.cc?l=163&rcl=efcb843fdd5fefaf4f0c106f04f308ed1aba7919 |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by phanindra.mandapaka@chromium.org
, Jan 12