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
Unhandled exception (TypeError) with ftplib in function retrbinary/retrlines causes inoperable behavior without crashing #70121
Comments
Hello, I believe that I found a bug in ftplib.py in version 3.3.6. Whenever a user opens a file for writing in nonbinary mode and then proceeds to call the retrbinary function, or opens the file in binary mode and proceeds to call the retrlines fuction, then attempts to write to the file using the callback function, an unhandled TypeError exception is raised that does not cause the program to crash, but rather cause the last message from the server to not be "read" by the readline() function, thus causing cascading errors until the function readline() is called on its own or the connection with the server is reset. Code to replicate: Unhandled exception raised: File "/usr/local/lib/python3.3/ftplib.py", line 434, in retrbinary Likewise, if ftp.retrlines('retr randomfilehere', fileTest.write) is called when fileTest is opened for writing with binary option the following error is raised: File "/usr/local/lib/python3.3/ftplib.py", line 464, in retrlines Calling ftp.getline() clears the error and resumes normal operation Possible Solution: add exception handling in lines 434/464 for TypeError |
Can you test this against 3.5? I guessing it hasn't been fixed, but we did do some bytes/string fixes since 3.3, so it might have been. |
I don't have access to 3.5 where I am now. I can try later on, but it appears after a quick glance that the code for this function between 3.3 and 3.5 is the same for calling the callback function. |
With the default branch I get regular TypeError exception, without any extras. To me it seems that its responsibility of the caller to provide the correct callback for the type of data being retrieved. So, I'm not exactly sure that this is a bug at all. |
Silent: The issue that i see is how the error is handled. I can trap the TypeError easily, however, if I keep the socket open, the behavior of ftplib will not be as intended. For example: fileTest = open('filename1', 'wb')
ftp.retrlines('RETR README.html', fileTest.write) This will give a TypeError, as intended However, what happens next is, I believe, not intended -- If i send a command after this trapped error, say, ftp.sendcmd('NOOP') The result will not be but, instead, 226 Transfer Complete As the previous status was not read from the file created by the socket, so if i want to try and do anything for the user after, the message received, and subsequently parsed by the getresp function, will not be the message that is expected. For a noop command this is ok, but for a transfer command, it will set the mode to either A or I and return a 200 status when a different status was expected. This will raise an error. The only way to fix this is to either run the internal function getline() or reset the connection, thus clearing all messages from the file. I have attached a file that has a fix, Im not sure if this is helpful or not. I modified the retrbinary function on lines: I also modified the retrlines function on lines: |
OK, here is the patch with the test that I think is exercising the issue. |
It looks to me that forcing self.voidresp() in case the callback raises an exception is a bad idea: it may easily end up hanging if the server is not supposed to send a response at that point (see: the transfer is not finished). As for the issue per se: I think there's nothing wrong with the current behavior except perhaps we could provide a better error message. But in order to do that we should check file mode before hand and relying on the file mode is also not a good idea IMO because the file object can be anything. |
Closing based on the comments above. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: