Problem occurs when using libcurl to attempt to connect to a non-existent TFTP server, with CURLOPT_CONNECTTIMEOUT < CURLOPT_TIMEOUT.
libcurl version libcurl/7.33.0
CURLOPT_URL=tftp://192.168.0.3/somefile (server doesn't exist)
CURLOPT_CONNECTTIMEOUT=10
CURLOPT_TIMEOUT=20
When calling curl_easy_perform:
< 10 seconds later... >
curl_easy_perform returned CURLE_OPERATION_TIMEDOUT after 20 seconds.
What happens after the 10 second timeout is that tftp_multi_statemach returns CURLE_OPERATION_TIMEDOUT to tftp_doing each time it is called, but tftp_doing overwrites the result with the result from Curl_speedcheck, which returns OK.
Forcing tftp_doing to return the CURLE_OPERATION_TIMEDOUT produces the correct result, which is that curl_easy_perform returns CURLE_OPERATION_TIMEDOUT after 10 seconds. I don't know if this is the correct approach.
May be related to http://sourceforge.net/p/curl/bugs/856/ which is a similar bug with FTP.
Hm. TFTP is UDP so it has no real connection, so thus a connection timeout is a bit of a funny concept there. It will still be considered for what's done before the actual data transfer, but after the name resolving is done there's no more connection being done but it switches immediately to transfer...
I'm not sure I have a better fix for this than documenting this better. Do you?
You are right, TFTP is not connection-oriented; however, you could also say that the initial request & response constitutes a connection.
tftp_set_timeouts makes a distinction between the connection phase (TFTP_STATE_START) and the transfer phase (TFTP_STATE_RX) when setting timeouts, which is why the connection failure is correctly detected after CURLOPT_CONNECTTIMEOUT.
It should be simple to make this work as expected (e.g. tftp_doing returns CURLE_OPERATION_TIMEDOUT if it receives CURLE_OPERATION_TIMEDOUT from tftp_multi_statemach, which seems to work.)
Hm, yeah I guess that could work. Any chance you can give it a shot to write up a patch, as you're already having a "nice" test setup there etc?
This fix works for me:
Oh, nice. That's a clean and simple fix. I'll even add to it and make it even more generic:
~~~~~
--- a/lib/tftp.c
+++ b/lib/tftp.c
@@ -1256,7 +1256,7 @@ static CURLcode tftp_doing(struct connectdata conn, bool dophas
if(dophase_done) {
DEBUGF(infof(conn->data, "DO phase is complete\n"));
}
- else {
+ else if(!result) {
/ The multi code doesn't have this logic for the DOING state so we
provide it for TFTP since it may do the entire transfer in this
state. */
I pushed this change to git now as commit c4f46e97ca6c0f
Thanks, case closed!