Hi folks,

I think I have found a bug in wget where it fails to change the working directory when retrying a failed ftp transaction. This is wget 1.10.2 on FreeBSD-6.0/amd64.

I was trying to use wget to get files from a broken ftp server which occasionally sends garbled responses, causing wget to get confused, eventually timeout, and retry the transfer. (The failure mode which makes it most obvious is sending a response to PASV which lacks the initial numeric response code, so that wget can't recognize it.) This is fine. However, when wget reconnects, it mistakenly thinks it is already in the appropriate directory, and it doesn't change it, reporting "CWD not required". This results in it trying to fetch the file from the root directory instead of the correct path.

Unfortunately I can't give you access to the server in question. I can sanitize the output of a wget session if you want. However, I think the bug is obvious from inspection. At ftp.c:1197 in ftp_loop_internal() we have

      err = getftp (u, &len, restval, con);

      if (con->csock != -1)
        con->st &= ~DONE_CWD;
        con->st |= DONE_CWD;

This test seems clearly to be backwards. If con->csock is -1 (i.e. the connection has been closed) then we must clear the DONE_CWD flag. Otherwise CWD has been done and we can set the flag.

Reversing the test fixes the problem. It also causes the CWD optimization to actually work when it's applicable, instead of only when it isn't :)

It might be worthwhile at other spots in the code to put in an assert() to ensure that we have (DO_CWD || !DO_LOGIN). Perhaps after those flags are set, e.g. ftp.c:1161 in ftp_loop_internal() and ftp.c:1409 in ftp_retrieve_list(). Also the existence of both DONE_CWD and DO_CWD may cause confusion and could probably cleaned up.

Thanks for working on wget!  It's a great tool.

Nate Eldredge

Reply via email to