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

Projects
None yet
5 participants
@efremovd
Copy link
Contributor

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

This comment has been minimized.

Copy link

commented Dec 9, 2018

CLA assistant check
All committers have signed the CLA.

@jgsogo jgsogo requested a review from lasote Dec 10, 2018

@jgsogo jgsogo added the type: bug label Dec 10, 2018

@lasote

lasote approved these changes Dec 10, 2018

Copy link
Contributor

left a comment

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

This comment has been minimized.

Copy link
Contributor Author

commented Dec 10, 2018

Hi,

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

@jgsogo jgsogo dismissed their stale review Dec 11, 2018

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

efremovd added some commits Dec 11, 2018

Merge pull request #1 from jgsogo/efremovd-ftp-download-auth
add some test cases (work on tmp_folder)
@jgsogo

jgsogo approved these changes Dec 12, 2018

Copy link
Member

left a comment

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

@jgsogo jgsogo merged commit 8413529 into conan-io:develop Dec 12, 2018

2 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
license/cla Contributor License Agreement is signed.
Details

@efremovd efremovd deleted the efremovd:ftp-download-auth branch Dec 12, 2018

grisumbras pushed a commit to grisumbras/conan that referenced this pull request Dec 27, 2018

Fixes ftp download when authentication is required (conan-io#4092)
* 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
You can’t perform that action at this time.