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

Use 1xx as expected response when requesting a data connection #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

suddjian
Copy link

@suddjian suddjian commented Jan 18, 2019

I have an issue where when I call StoreFile, the server responds with 125 Data connection already open; Transfer starting., which this library treats as an unexpected code and incorrectly aborts the transfer.

I don't have control over this server, and I'm not sure why it is responding this way (I am only opening one connection in a simple script to store one file, and I always close the connection afterwards). But regardless, 125 is not actually an error code, and if we treat it as a success code the file still gets uploaded just fine.

So this commit changes the expected response to 1xx, which should match any positive response, as described here.

@marcobeierer
Copy link
Owner

Hi @ChemicalRocketeer

Thank you for your pull requests. I had a look at the issue and you are absolutely right, we shouldn't abort on status code 125.

Unfortunately I think your fix introduces new issues. When I understand it correctly, we cannot process normally when we receive status code 110 or 120. 120 for example means Service ready in nnn minutes. and it is necessary to handle this situation if we accept all 1xx status codes... Also 110 seems to need special handling.

If I look at the code it seems that accepting just the status codes 125 and 150 isn't a quick fix as it needs some refactoring...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants