Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Included credentials while downloading file #717

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

agoutam27
Copy link

No description provided.

@jimmywarting
Copy link
Collaborator

really think you should be downloading a blob on your own, it isn't really the job of filesaver to download files.
it's technically bad to download the hole file into the memory and then save it.

It's much better if you can do <a href="link" download="name.txt"> instead. Cookies will be included/sent... it's possible to trigger it with a click.

also if the server can send content-disposition attachment header then that is even better

@paulftw
Copy link

paulftw commented Jan 25, 2022

I agree with your comments, @jimmywarting, however the download code is already part of FileSaver.
Since this PR is changing existing functionality and touches on security, I'd rather make withCredentials an optional setting, default to false (to preserve current behavior). That way it wouldn't affect anyone who upgrades to latest version without looking.

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

Successfully merging this pull request may close these issues.

3 participants