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

Fixes ftp download when authentication is required #4092

Merged
merged 5 commits into from
Dec 12, 2018

Conversation

efremovd
Copy link
Contributor

@efremovd efremovd commented Dec 9, 2018

Changelog: Bugfix: In ftp_download function there is extra call to ftp.login() with empty args. This causes ftp lib to login again with empty credentials and throwing exception because authentication is required by server.

Docs: omit

It's a small fix so I did not create any issue about it.

@CLAassistant
Copy link

CLAassistant commented Dec 9, 2018

CLA assistant check
All committers have signed the CLA.

jgsogo
jgsogo previously approved these changes Dec 10, 2018
Copy link
Contributor

@lasote lasote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right:

class ftplib.FTP([host[, user[, passwd[, acct[, timeout]]]]])
Return a new instance of the FTP class. When host is given, the method call connect(host) is made. 

When user is given, additionally the method call login(user, passwd, acct) is made ....

@jgsogo jgsogo added this to the 1.11 milestone Dec 10, 2018
@efremovd
Copy link
Contributor Author

Hi,

Thanks for pointing out. I've update tests and code to support anonymous ftp too.

@jgsogo jgsogo dismissed their stale review December 11, 2018 09:50

At least there is an use-case that is wrong (see my PR to efremovd branch)

Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Now I think that all the use cases are covered 👍

@jgsogo jgsogo merged commit 8413529 into conan-io:develop Dec 12, 2018
@efremovd efremovd deleted the ftp-download-auth branch December 12, 2018 17:29
grisumbras pushed a commit to grisumbras/conan that referenced this pull request Dec 27, 2018
* Fixes ftp download when authentication is required

* Fixes back anonymous ftp download

* add some test cases (work on tmp_folder)

* Fixes ftp download of file that  does not exist
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants