Skip to content
This repository has been archived by the owner on Jan 24, 2021. It is now read-only.

Don't touch other downloads with the download filename handler #189

Closed
CollinChaffin opened this issue Jun 22, 2019 · 9 comments
Closed

Comments

@CollinChaffin
Copy link

Please see this picture which occurs when downloading FROM SITES OTHER THAN TWITTER:

SNAG_6-22-2019_09-24-21

Not only that, it is auto-generating (aka overriding) the native auto-generated download name to "download.htm". This was on a non-Twitter site when attempting to ALT-click a file to force FDM to download it.

IMO this is a HUGE security issue. Why on earth does this extension need this permission?:

SNAG_6-22-2019_09-28-22

Yes, I have no issue granting full permission to Twitter sites, and yes if THOSE sites are all it acts upon (including downloads) then I also have no issue granting permissions for manipulating Twitter downloads (but actually prefer it didn't at all). I do NOT, however, want this extension touching ANYTHING outside of those sites and clearly it is due to either a bug or abuse of this blanket global download permission. This is beyond a bug that really needs to be corrected ASAP.

@CollinChaffin
Copy link
Author

Also see my comment on #150 a simple url rewrite is all that is required I have userscripts that do this without ANY browser permissions or actions whatsoever.

@jorgegonzalez
Copy link
Contributor

Feel free to open a PR if this is a simple fix

@sindresorhus
Copy link
Owner

sindresorhus commented Jun 22, 2019

We use the permission to fix the image filename:

// Fix the extension when right-click saving a tweet image
browser.downloads.onDeterminingFilename.addListener((item, suggest) => {
suggest({
filename: item.filename.replace(/\.(jpg|png)_(large|orig)$/, '.$1')
});
});
I didn't realize it would affect other sites. That's very surprising.

The fix would be to check the absolute URL that initiated the download in that handler and make sure it's from Twitter: https://developers.chrome.com/extensions/downloads#type-DownloadItem


Removing it is not a good fix as it's a useful feature. Guarding it is enough.

@sindresorhus sindresorhus changed the title Why on earth is refined Twitter managing ALL my downloads? Don't touch other downloads with the download filename handler Jun 22, 2019
@mtlaird
Copy link

mtlaird commented Jun 23, 2019

You don't need to fix the filename, anymore. Twitter changed the way they load images, so they don't display images as "jpg:large" or "jpg:orig" anymore. This is what a twitter filename looks like to me, now:

https://pbs.twimg.com/media/D9sMEzzXUAAX_Yr?format=jpg&name=4096x4096

At least on my browser, this saves fine as a normal jpg, so the plugin no longer needs to do anything with image downloads (unless other people's twitter web experience is different than mine).

@CollinChaffin
Copy link
Author

Sorry I've been behind. I will absolutely work on a PR using my userscript perhaps as a guide but I also fully agree with @mtlaird in that perhaps you want to look at whether it really is even a needed feature anymore since the naming has now changed. Obviously if it's not required for naming, then that will really simplify things and as far as I can tell, remove the entire need for this permission or any of the logic.

Let me know @sindresorhus if after reviewing @mtlaird info on the current naming whether I even need to work on a PR or if you think perhaps it can just be eliminated. TIA!

@chrcoluk
Copy link

I dont know if its related, but I just had to disable this extension as it is preventing chrome from remembering download location "globally".

All downloads are sent to the configured download folder only instead of chrome remembering the last location.

@LesterCovax
Copy link

Came here to submit an issue for this, but it seems I'm not alone. It's interfering with another (actual) download manager. Going to have to keep it disabled until this is fixed. Although the idea of limiting its scope to twitter URLs is good, my first thought was to include an option to disable its download features. I'm guessing that may not work though as the permissions are baked into the extension install.

refinedTwitter

@craigeley
Copy link

Just another user who had to troubleshoot for a while to figure why a site that I regularly download SRT files from was converting them to HTML. This is the culprit! Disabled until a fix can be issued, but I agree with the above—this "feature" is no longer useful and should just be disabled.

@sindresorhus
Copy link
Owner

Closing as this extension is now deprecated: 8dd9df7

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants