search

atinux / node-ftps

FTP, FTPS and SFTP client for node.js, mainly a lftp wrapper.
MIT License
203 stars 57 forks source link

Empty username forces anonymous access #37

Closed jeromew closed 7 years ago

jeromew commented 7 years ago

I changed to test on password allow for a simple

{ host: host, protocol: protocol }

configuration for anonymous access. Otherwise, password was required.

jeromew commented 7 years ago

note: I went too fast and did not see that you had tests on this so this will probably break tests. You can either accept it and change the tests or I will look at it early next week. Tell me what you think and if the diff is ok with you

atinux commented 7 years ago

You can update the tests and I will merge the pull request.

Thank you for your collaboration :)

jeromew commented 7 years ago

@Atlinux I updated my PR with tests on the feature. I added a prepareLFTPOptions method so that the lftp options can be tested without launching lftp (basically this breaks exec in 2 : 'prepare' then 'spawn') I hope you will agree on this.

This showed a hidden bug on protocol saying ".toLowerCase" cannot be called on undefined in the cases where protocol was not defined in the options. I added a default value for protocol ('ftp'). This lead to some changes because of the way host is modified with a 'ftp://' prefix. Here again I hope you will agree with my modification.

Keep me posted.

atinux commented 7 years ago

Great job @jeromew