veliovgroup / Meteor-Files

🚀 Upload files via DDP or HTTP to ☄️ Meteor server FS, AWS, GridFS, DropBox or Google Drive. Fast, secure and robust.
https://packosphere.com/ostrio/files
BSD 3-Clause "New" or "Revised" License
1.11k stars 166 forks source link

Use content-dispositon package #820

Closed Prinzhorn closed 2 years ago

Prinzhorn commented 3 years ago

https://www.npmjs.com/package/content-disposition

These three occurrences would benefit from using a proper package. Because the format is not as trivial as they make it look.

https://github.com/veliovgroup/Meteor-Files/blob/7f67a0885e120ad77085b191e3399add575a2d89/server.js#L1762 https://github.com/veliovgroup/Meteor-Files/blob/master/docs/gridfs-bucket-integration.md#4-create-download-handler https://github.com/veliovgroup/Meteor-Files/blob/7f67a0885e120ad77085b191e3399add575a2d89/docs/gridfs-streaming.md

E.g. this will break easily:

http.response.setHeader('Content-Disposition', `inline; filename="${file.name}"`);
dr-dimitru commented 2 years ago

@Prinzhorn is it broken now? Or you just suggesting a package?

Prinzhorn commented 2 years ago

I didn't set up an actual PoC, but every edge case the package handles should be broken. E.g. if file.name has a double quote. This example doesn't do any special handling https://github.com/veliovgroup/Meteor-Files/blob/master/docs/gridfs-bucket-integration.md#4-create-download-handler which I think sparked this issue. The other places seem to handle edge cases, I was just advocating re-using the same code across the repo.

dr-dimitru commented 2 years ago

@Prinzhorn thank you for the prompt reply. I believe we should update documentation in that case. Package itself does everything necessary to meet RFC 6266

Prinzhorn commented 2 years ago

Sounds good, I think the documentation is the only part that needs to be adjusted then!

dr-dimitru commented 2 years ago

@Prinzhorn I see that you're the last one to edit this file, would you be open to send a PR?