search

gyng / save-in

WebExtension for saving media, links, or selections into user-defined directories
MIT License
201 stars 25 forks source link

parse content-disposition #73

Open def00111 opened 6 years ago

def00111 commented 6 years ago

You can use his parser: https://github.com/Rob--W/open-in-browser/blob/master/extension/content-disposition.js

bodograumann commented 6 years ago

Yes please add this. Currently :filename: does not contain the correct filename send by content-disposition.

gyng commented 6 years ago

Thanks for the link. It looks better than what I have right now. I'll look into using that when I get the chance to do so.

gyng commented 6 years ago

I've swapped in the parser linked in the first post in 2.7.0, hopefully this will fix some of the issues you're having with renaming. If it still doesn't work please link a test case (if possible) so I can try things out. This shouldn't break existing renames, but if it does do post those as well. I'll close this issue soon if all goes well.

bodograumann commented 6 years ago

Sorry, but this still does not work for me. Unfortunately the problematic download is not public. Also I'm not really sure how to debug this. In the settings under ”Dynamic Downloads (Advanced)” the point ”Last download“ seems to only show the download URL, but I want to match against the :filename:. I would suggest displaying all variable values that occur in a capture: clause and maybe even those that are used in into:. That would help immensely in using and debugging the patterns.

gyng commented 6 years ago

I think it's something I used to have, but removed. The list of all variable values is probably something I'll add again.

On Chrome, the final filename (after Content-Disposition) is provided by the browser through downloads.onDeterminingFilename.

On Firefox, the content disposition is obtained through a separate HTTP HEAD request and can fail for a variety of reasons. Disregarding invalid header values, attempting to get the Content-Disposition can also fail if the server rejects HEAD requests, or requires credentials.

It looks like I'm not sending credentials with this fetch request, so I'll add that in and see if it helps. If your renaming works in Chrome but not in Firefox then this fetch request is somehow failing.

Is the Content-Disposition header for your download valid? I can rule that out if that's the case. You can also check the exact request if you drop down into addon debugging and look at the network tab (for the background script).

bodograumann commented 6 years ago

I am indeed using Firefox and it seems like the Host is liking neither your HEAD nor your GET request ;-) I wasn’s even aware something was saved, because I selected “Exclusively use routing and renaming rules” and thought the download would be aborted when nothing matched. So now I can see (also in the network view you so kindly pointed out), that the download request is leading to a redirection to the log-in page.

The difference between a regular download and one with save-in are the Upgrade-Insecure-Requests: 1 and Referer: […] header. I think you should definitely add the referrer. That may already fix the problem for me.

gyng commented 6 years ago

Thanks for the details. I've published a quick update 2.7.1 that passes credentials: true along with the Content-Disposition header fetch request. It should not fix the issue you're having as it still should not pass the Referer header.

Moving on, I'd like to explore having an option to send the request from the content script itself. It looks like there might have been some updates with content.fetch(?) sending Referer headers according to https://developer.mozilla.org/en-US/Add-ons/WebExtensions/Content_scripts. I'll prioritise this as something to explore when I get some time to look into it.

From version 58 onwards extensions that need to perform requests that behave as if they were sent by the content itself can use content.XMLHttpRequest and content.fetch() instead.

gyng commented 5 years ago

Just to update: there's an option to send Referer headers by domain (Firefox right now, think it was broken in Chrome 73). Maybe that solves any issue you have with this.