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

curl --head file:// no longer reports the headers #3083

Closed
sjackman opened this issue Oct 2, 2018 · 12 comments
Closed

curl --head file:// no longer reports the headers #3083

sjackman opened this issue Oct 2, 2018 · 12 comments

Comments

@sjackman
Copy link

sjackman commented Oct 2, 2018

With curl 7.43.0, curl --head file://… outputs headers.
With curl 7.61.1, curl --head file://… outputs the contents of the file.

I did this

curl --head file:///usr/share/dict/words

I expected the following

Content-Length: 2493109
Accept-ranges: bytes
Last-Modified: Wed, 16 Sep 2015 03:00:17 GMT

I saw the following

a
aa
aal
aalii
aam
…

curl/libcurl version

curl 7.61.1 (x86_64-apple-darwin15.6.0) libcurl/7.61.1 SecureTransport zlib/1.2.5
Release-Date: 2018-09-05
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS IPv6 Largefile NTLM NTLM_WB SSL libz UnixSockets 

operating system

macOS 10.11.6 El Capitan

❯❯❯ /usr/bin/curl --head file:///usr/share/dict/words
Content-Length: 2493109
Accept-ranges: bytes
Last-Modified: Wed, 16 Sep 2015 03:00:17 GMT
❯❯❯ curl 7.43.0 (x86_64-apple-darwin15.0.0) libcurl/7.43.0 SecureTransport zlib/1.2.5
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS IPv6 Largefile GSS-API Kerberos SPNEGO NTLM NTLM_WB SSL libz UnixSockets
@jay
Copy link
Member

jay commented Oct 2, 2018

bisected to c1c2762 which added bold header support but removed setting CURLOPT_HEADER on config->no_body.

This should fix it but I'm not sure what it would break, also tests would need to be adjusted:

diff --git a/src/tool_operate.c b/src/tool_operate.c
index 5a72b6a..1e5743c 100644
--- a/src/tool_operate.c
+++ b/src/tool_operate.c
@@ -864,8 +864,15 @@ static CURLcode operate_do(struct GlobalConfig *global,
           my_setopt(curl, CURLOPT_INFILESIZE_LARGE, uploadfilesize);
         my_setopt_str(curl, CURLOPT_URL, this_url);     /* what to fetch */
         my_setopt(curl, CURLOPT_NOPROGRESS, global->noprogress?1L:0L);
-        if(config->no_body)
+        if(config->no_body) {
           my_setopt(curl, CURLOPT_NOBODY, 1L);
+          my_setopt(curl, CURLOPT_HEADER, 1L);
+        }
+         /* If --metalink is used, we ignore --include (headers in
+            output) option because mixing headers to the body will
+            confuse XML parser and/or hash check will fail. */
+        else if(!config->use_metalink)
+          my_setopt(curl, CURLOPT_HEADER, config->show_headers?1L:0L);

         if(config->oauth_bearer)
           my_setopt_str(curl, CURLOPT_XOAUTH2_BEARER, config->oauth_bearer);

And file does not handle CURLOPT_NOBODY on its own which is why the body is being shown

curl/lib/file.c

Lines 416 to 419 in 432eb5f

/* If we have selected NOBODY and HEADER, it means that we only want file
information. Which for FILE can't be much more than the file size and
date. */
if(data->set.opt_no_body && data->set.include_header && fstated) {

which seems to me should be

  if(data->set.opt_no_body) {
    if(fstated) {
      if(data->set.include_header) {
        ...
      }
      if(!result)
        /* set the file size to make it available post transfer */
        Curl_pgrsSetDownloadSize(data, expected_size);
    }
    return result;
  }

@bagder
Copy link
Member

bagder commented Oct 3, 2018

I think the latter part is what needs to be fixed, as I believe the upper part breaks other things...

@jay
Copy link
Member

jay commented Oct 3, 2018

The latter as I wrote it should only correct the issue of the body being shown when it shouldn't be, the headers would still not be shown unless CURLOPT_HEADER is set.

@bagder
Copy link
Member

bagder commented Oct 3, 2018

curl -I localhost shows the headers so I would probably say that curl -I file:///usr/share/dict/words should also show the headers. (without --include I mean)

@jay
Copy link
Member

jay commented Oct 3, 2018

Right, that is the regression, it is documented that way. If the bold code now goes through the header callback though I may have broken something else by setting CURLOPT_HEADER, I didn't try this out yet.

@bagder
Copy link
Member

bagder commented Oct 3, 2018

Right, that's exactly what I'm thinking. The header output logic was slightly changed with that bolding...

@sjackman
Copy link
Author

sjackman commented Oct 3, 2018

I've just noticed that macOS curl 7.43.0 had a different bug. curl -I works as expected, but curl -i does not report both the headers and the content as expected.

Observed

❯❯❯ curl -I file://$PWD/hello
Content-Length: 14
Accept-ranges: bytes
Last-Modified: Wed, 03 Oct 2018 17:31:42 GMT
❯❯❯ curl -i file://$PWD/hello
Hello, world!

Expected

❯❯❯ curl -i file://$PWD/hello
Content-Length: 14
Accept-ranges: bytes
Last-Modified: Wed, 03 Oct 2018 17:31:42 GMT

Hello, world!

@jay
Copy link
Member

jay commented Oct 3, 2018

-i is not documented to work with file. there may be some undocumented behavior that varies depending on the version.

@sjackman
Copy link
Author

sjackman commented Oct 3, 2018

Feature request then: could -i and -I work with file://?

@jay
Copy link
Member

jay commented Oct 3, 2018

-I not working for file is a bug which we'll fix but -i iirc was really intended just for HTTP headers. In the short description it shows as "Include protocol response headers in the output" which could imply other protocols, so I'm on the fence on that one.

@sjackman
Copy link
Author

sjackman commented Oct 3, 2018

curl -i for file:// is not terribly important for me. It's just for consistency. I'd also be okay with documenting that it doesn't work. curl -I for file:// is important to me.

@bagder bagder self-assigned this Oct 4, 2018
bagder added a commit that referenced this issue Oct 5, 2018
Now FILE transfers send headers to the header callback like HTTP and
other protocols. Also made curl_easy_getinfo(...CURLINFO_PROTOCOL...)
work for FILE in the callbacks.

Makes "curl -i file://.." and "curl -I file://.." work like before
again. Applied the bold header logic to them too.

Regression from c1c2762 (7.61.0)

Reported-by: Shaun Jackman
Fixes #3083
bagder added a commit that referenced this issue Oct 6, 2018
Now FILE transfers send headers to the header callback like HTTP and
other protocols. Also made curl_easy_getinfo(...CURLINFO_PROTOCOL...)
work for FILE in the callbacks.

Makes "curl -i file://.." and "curl -I file://.." work like before
again. Applied the bold header logic to them too.

Regression from c1c2762 (7.61.0)

Reported-by: Shaun Jackman
Fixes #3083
Closes #3101
@bagder bagder closed this as completed in e50a200 Oct 8, 2018
@sjackman
Copy link
Author

sjackman commented Oct 8, 2018

Thanks for fixing this issue, Daniel!

@lock lock bot locked as resolved and limited conversation to collaborators Jan 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

3 participants