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

FTP with CURLOPT_NOBODY different behavior when performing on non-existing files #5953

Closed
Togtja opened this issue Sep 11, 2020 · 4 comments
Closed
Assignees
Labels
FTP

Comments

@Togtja
Copy link

@Togtja Togtja commented Sep 11, 2020

I did this

I tried to make a function in libcurl that checks if a file exist on server:

curl_easy_setopt(curl, CURLOPT_URL, "ftp://user:pwd@host/fakefile.txt");
curl_easy_setopt(curl, CURLOPT_NOBODY, 1L);

CURLcode res = curl_easy_perform(curl);
//res is CURLE_OK

However, because I use the CURLOPT_NOBODY option, perform returns CURLE_OK (If I don't use the no body option it return CURLE_REMOTE_FILE_NOT_FOUND) . However, on SFTP calls in reruns as I expect:

curl_easy_setopt(curl, CURLOPT_URL, "sftp://user:pwd@host/fakefile.txt");
curl_easy_setopt(curl, CURLOPT_NOBODY, 1L);

CURLcode res = curl_easy_perform(curl);
//res is CURLE_REMOTE_FILE_NOT_FOUND

I expected the following

I expect them both to return CURLE_REMOTE_FILE_NOT_FOUND or at least have the same behaviour

curl/libcurl version

7.72.0

curl 7.72.0 (Linux) libcurl/7.72.0 OpenSSL/1.1.1g zlib/1.2.11 libssh2/1.9.0_DEV
Release-Date: 2020-08-19
Protocols: dict file ftp ftps gopher http https imap imaps pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS HTTPS-proxy IPv6 Largefile libz NTLM SSL UnixSockets

operating system

Linux togtja-ubuntu 5.4.0-47-generic #51~18.04.1-Ubuntu SMP Sat Sep 5 14:35:50 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

@bagder bagder added the FTP label Sep 11, 2020
@bagder
Copy link
Member

@bagder bagder commented Sep 11, 2020

I think if you also do:

  curl_easy_setopt(hnd, CURLOPT_FILETIME, 1L);

... it might actually do what you expected it. As a work-around.

bagder added a commit that referenced this issue Sep 12, 2020
and returns CURLE_FTP_COULDNT_RETR_FILE. This is primarily interesting
for cases where CURLOPT_NOBODY is set as otherwise curl would not return
an error for this case.

This is how libcurl already acts on a 550 as a MDTM response (when
CURLOPT_FILETIME is set). If CURLOPT_NOBODY is not set, the error will
happen subsequently anyway since the RETR command will also fail with
it.

Reported-by: Tomas Berger
Fixes #5953
@bagder
Copy link
Member

@bagder bagder commented Sep 12, 2020

@Togtja if you enable VERBOSE for your scenario, do you see the SIZE command get a 550 back? that's what I'm assuming and what my suggested fix in #5957 acts on.

@bagder bagder self-assigned this Sep 13, 2020
@Togtja
Copy link
Author

@Togtja Togtja commented Sep 14, 2020

setting curl_easy_setopt(curl, CURLOPT_FILETIME, 1L); returns CURLE_FTP_COULDNT_RETR_FILE

With verbose without setting CURLOPT_FILETIME, it does indeed return 550 Could not get file size

After compiling #5957 It does indeed now actually return CURLE_FTP_COULDNT_RETR_FILE for me. Is there a big difference between CURLE_FTP_COULDNT_RETR_FILE and CURLE_REMOTE_FILE_NOT_FOUND?

@bagder
Copy link
Member

@bagder bagder commented Sep 14, 2020

Hm. I think I was too quick and you're right. It should rather use CURLE_REMOTE_FILE_NOT_FOUND for this. I'll amend my PR!

bagder added a commit that referenced this issue Sep 14, 2020
This is primarily interesting for cases where CURLOPT_NOBODY is set as
otherwise curl would not return an error for this case.

MDTM getting 550 now also returns this error in order to unify return
codes for missing files across protocols and specific FTP commands.

libcurl already returns error on a 550 as a MDTM response (when
CURLOPT_FILETIME is set). If CURLOPT_NOBODY is not set, the error will
happen subsequently anyway since the RETR command will also fail with
it.

Add test 1913 and 1914 to verify. Updated several tests accordingly.

Reported-by: Tomas Berger
Fixes #5953
@bagder bagder closed this in 7ea2e1d Sep 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.