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

Jump to bottom

FTP download fails with "No such file or directory" when CURLOPT_FILETIME is enabled #9357

Closed
mhei opened this issue Aug 24, 2022 · 5 comments
Closed

FTP download fails with "No such file or directory" when CURLOPT_FILETIME is enabled #9357

mhei opened this issue Aug 24, 2022 · 5 comments
Labels
FTP

Comments

@mhei
Copy link
Contributor

mhei commented Aug 24, 2022 •
edited

I did this

I want to download a file from an FTP server and enabled CURLOPT_FILETIME by default in my curl context.
However, some FTP servers are configured more strictly so that the MDTM command is rejected with 550 error "Permission denied".
curl translates the 550 to "File not found", however, the file is actually present and when I don't set CURLOPT_FILETIME, then the download works fine. See the following trace:

*   Trying 1.1.1.1...
* TCP_NODELAY set
* Connected to ftp.example.com (1.1.1.1) port 21 (#0)
< 220 ProFTPD Server (Ftp Server) [1.1.1.1]
 [1.1.1.1]
> AUTH SSL
< 500 AUTH not understood
> AUTH TLS
< 500 AUTH not understood
> USER myuser
< 331 Password required for myuser
> PASS foobarpassword
< 230 User myuser logged in
> PWD
< 550 PWD: Permission denied
> MDTM 1_0_7.image
* ftp_perform ends with SECONDARY: 0
< 550 1_0_7.image: Operation not permitted
* Given file does not exist
* Remembering we are in dir ""
* Connection #0 to host ftp.example.com left intact

I expected the following

I would expect that the download still works with CURLOPT_FILETIME set - personally, I consider it more as optional thing - if it works then it's fine - if not then it's still ok. In function ftp_state_mdtm_resp for example an unsupported reply is also handled in such a way, that no error is thrown.
However, I know that the FTP server responses are some kind of implementation specific: for some servers 550 is really "File not found", some servers (as above) use it for permission things.
In my eyes, there are three options to handle it:

  1. don't fail on every 550 anymore in ftp_state_mdtm_resp, just ignore it -> this would defer the error in case the file really does not exists
  2. introduce something like CURLOPT_FILETIME_OPTIONAL which deals with this special situation and allows the library user to fine-control the behavior -> it would only fail hard if the user does not give this flag (keep current behavior) but ignores the 550 in case this flag is set
  3. not only look at the numeric error code but use a heuristic on the plain text description, e.g. look for "found" or "permitted" in the assumption that the FTP server always respon in English and decide upon the found string whether the 550 can be ignored or not
  4. any other idea :-)

I want to discuss here, whether you also consider it as issue and which solution should be worked on.

curl/libcurl version

curl 7.58.0 (x86_64-pc-linux-gnu) libcurl/7.58.0 OpenSSL/1.1.1 zlib/1.2.11 libidn2/2.0.4 libpsl/0.19.1 (+libidn2/2.0.4) nghttp2/1.30.0 librtmp/2.3
Release-Date: 2018-01-24
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtmp rtsp smb smbs smtp smtps telnet tftp
Features: AsynchDNS IDN IPv6 Largefile GSS-API Kerberos SPNEGO NTLM NTLM_WB SSL libz TLS-SRP HTTP2 UnixSockets HTTPS-proxy PSL

But this is still present in latest Github branch.

operating system

I'm still on Ubuntu 18.04 on my dev host, but I also tested with newer libcurl on an embedded Linux system (Yocto based).
@bagder bagder added the FTP label Aug 24, 2022
@bagder
Copy link
Member

bagder commented Aug 24, 2022

The main problem here is of course that 550 is overused as a return code so it is the same for "missing file" as for "not permitted".

We cannot "use a heuristic on the plain text". The text can be anything and we cannot depend on that.

But I just checked the docs: we can safely convert this one become a non-fatal error and instead have CURLINFO_FILETIME return -1 for this case, as it is already a documented behavior.

Do you want to work on a fix for this?

mhei added a commit to mhei/curl that referenced this issue Aug 24, 2022
@mhei
ftp: ignore a 550 response to MDTM
8e6ad60 
The 550 is overused as a return code for multiple error case, e.g.
file not found and/or insufficient permissions to access the file.

So we cannot fail hard in this case.

Adjust test 511 since we now fail later.

Reported-by: Michael Heimpold
Fixes curl#9357
@mhei
Copy link
Contributor Author

mhei commented Aug 24, 2022

Do you want to work on a fix for this?

I'd try my best 😄 It is just as simple as in my proposal? I guess I missed something in the big picture... awaiting your feedback.

@bagder
Copy link
Member

bagder commented Aug 25, 2022

I think so. It would of course also be good to do a test for the specific case where MDTM fails but the file is still present and gets downloaded but there's no FILETIME provided. Maybe adjust test 1914, as I believe your change will break that test anyway.

@mhei
Copy link
Contributor Author

mhei commented Aug 27, 2022

Yes, I'll check/add...

@mhei
Copy link
Contributor Author

mhei commented Aug 29, 2022

Tests related/affected according:

test 511 - FTP with FILETIME and NOBODY but missing file
-- uses curl binary, this test now fails later since the failing MDTM is ignored

test 1913 - FTP with NOBODY set, getting a missing file
-- this test is unchanged since FILETIME is not set and thus no MDTM is issued

test 1914 - FTP with NOBODY and FILETIME set, getting a missing file
-- uses libcurl, this test now fails later with same error code since the failing MDTM is now ignored (failed with same error previously)

I'll add a new test as you suggested:

test 3027 - Get a file via FTP but 550 after MDTM command
-- uses libcurl, ensures that filetime == -1 is returned

mhei added a commit to mhei/curl that referenced this issue Aug 29, 2022
@mhei
ftp: ignore a 550 response to MDTM
3f2866f 
The 550 is overused as a return code for multiple error case, e.g.
file not found and/or insufficient permissions to access the file.

So we cannot fail hard in this case.

Adjust test 511 since we now fail later.
Add new test 3027 which check that when MDTM failed, but the file could
actually be retrieved, that in this case no filetime is provided.

Reported-by: Michael Heimpold
Fixes curl#9357
@mhei mhei mentioned this issue Aug 29, 2022
Closed
mhei added a commit to mhei/curl that referenced this issue Aug 29, 2022
@mhei
ftp: ignore a 550 response to MDTM
f419270 
The 550 is overused as a return code for multiple error case, e.g.
file not found and/or insufficient permissions to access the file.

So we cannot fail hard in this case.

Adjust test 511 since we now fail later.
Add new test 3027 which check that when MDTM failed, but the file could
actually be retrieved, that in this case no filetime is provided.

Reported-by: Michael Heimpold
Fixes curl#9357
Closes curl#9387
mhei added a commit to mhei/curl that referenced this issue Aug 30, 2022
@mhei
ftp: ignore a 550 response to MDTM
9fdcbc7 
The 550 is overused as a return code for multiple error case, e.g.
file not found and/or insufficient permissions to access the file.

So we cannot fail hard in this case.

Adjust test 511 since we now fail later.
Add new test 3027 which check that when MDTM failed, but the file could
actually be retrieved, that in this case no filetime is provided.

Reported-by: Michael Heimpold
Fixes curl#9357
Closes curl#9387
mhei added a commit to mhei/curl that referenced this issue Aug 30, 2022
@mhei
ftp: ignore a 550 response to MDTM
da81517 
The 550 is overused as a return code for multiple error case, e.g.
file not found and/or insufficient permissions to access the file.

So we cannot fail hard in this case.

Adjust test 511 since we now fail later.
Add new test 3027 which check that when MDTM failed, but the file could
actually be retrieved, that in this case no filetime is provided.

Reported-by: Michael Heimpold
Fixes curl#9357
Closes curl#9387
mhei added a commit to mhei/curl that referenced this issue Sep 6, 2022
@mhei
ftp: ignore a 550 response to MDTM
d3d14fd 
The 550 is overused as a return code for multiple error case, e.g.
file not found and/or insufficient permissions to access the file.

So we cannot fail hard in this case.

Adjust test 511 since we now fail later.
Add new test 3027 which check that when MDTM failed, but the file could
actually be retrieved, that in this case no filetime is provided.

Reported-by: Michael Heimpold
Fixes curl#9357
Closes curl#9387
@bagder bagder closed this as completed in d668685 Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FTP
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ftp: ignore a 550 response to MDTM mhei/curl
2 participants
@bagder @mhei