search

hgouveia / node-downloader-helper

A simple http file downloader for node.js
MIT License
247 stars 54 forks source link

Fix leading dots being removed #59

Closed Chaphasilor closed 2 years ago

Chaphasilor commented 3 years ago

Switched to a simple RegExp-based approach that works well, with a single or multiple trailing dots.
Added a test to make sure it works.

I also thought about the option for forcing the provided name without any processing (e.g. forceFilename: true), but I think because this can lead to insecure filenames (e.g. trailing dots causing corrupted files on Windows), this shouldn't be so easy to do.
And if someone really needs this, passing filename: (forcedName) => forcedName solves the problem.

If you want me to, I could also mention the above in the readme so people know how to deal with it :)

Chaphasilor commented 3 years ago

Oh and I notice that there are several hundreds of security vulnerabilities that can be automatically fixed by npm, I can fix them and either add it to this PR or to a separate one...

hgouveia commented 3 years ago

Oh and I notice that there are several hundreds of security vulnerabilities that can be automatically fixed by npm, I can fix them and either add it to this PR or to a separate one...

Hello @Chaphasilor if this vulnerabilities were introduced because of the use of the Regexp, please include the fix in this PR, but if the vulnerabilities were there before, yes, please create a new PR for this

Chaphasilor commented 3 years ago

@hgouveia it seems like the problems were already fixed but I had to reinstall my node_modules first :)
All is well.

hgouveia commented 3 years ago

@Chaphasilor cool, go ahead and add in the readme as you mentioned, how to force the name, i think the callback is the best option for these cases, after that i will merge it, thanks for the great support and job!

Chaphasilor commented 3 years ago

@hgouveia okay so I took another thorough look at the code and noticed that the way I did it back in #41 meant that the name would always be modified, even when using a callback, like you suggested in #58. Sorry about that, I fixed it now 😅

This also means that there's no need to change the readme any more, because the filename will only be modified (e.g. remove trailing dots), if the user didn't set the fileName option and we are using the Content-Disposition from the server :)

Chaphasilor commented 2 years ago

@hgouveia any updates on this? :)

hgouveia commented 2 years ago

Hello @Chaphasilor, merge to dev branch, i will try to create a new version this week with your changes! thanks for the contribution!

hgouveia commented 2 years ago

@Chaphasilor published to npm v1.0.19