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

-T /dev/stdin/-T /dev/fd/$n may upload with an incorrect content length on BSDs #12177

Open
emanuele6 opened this issue Oct 21, 2023 · 0 comments · May be fixed by #12178
Open

-T /dev/stdin/-T /dev/fd/$n may upload with an incorrect content length on BSDs #12177

emanuele6 opened this issue Oct 21, 2023 · 0 comments · May be fixed by #12178

Comments

@emanuele6
Copy link
Contributor

emanuele6 commented Oct 21, 2023

I did this

-T path will stat path to figure out its size in bytes to use it as Content-Length if it is a regular file.

The problem with that is that, on BSDs and some other UNIXes (not Linux), open(path) may not give you a file descriptor with a 0 offset from the start of the file.

On BSDs, if the path given to open is to a /dev/fd/* file or /dev/stdin, /dev/stdout, /dev/stderr, that will trigger a special behaviour for open: open on BSD, instead of reopening the file related to that file descriptor with different open modes like in Linux, will actually duplicate the file descriptor specified by that path if its open modes are compatible with the ones requested by open, and otherwise fail.

Since -T /dev/fd/4 for example, will effectively just be a dup(4), when curl will open the path it thinks is for a regular file (because fd 4 is indeed a file descriptor for a regular file, myfile), it will not find itself at byte offset 0 into the file, and will send less data than Content-Length.

printf 'NAME\tJOB\nTom\tCarpenter\nAlice\tMechanic\n' > myfile
{
   read -r skip_firstline <&4 &&
   curl -T /dev/fd/4 localhost:43000
} 4< myfile
$ nc -l localhost:43000
PUT /4 HTTP/1.1
Host: localhost:43000
User-Agent: curl/8.2.1
Accept: */*
Content-Length: 38

Tom     Carpenter
Alice   Mechanic

I expected the following

I expected:

$ nc -l 43000
PUT /4 HTTP/1.1
Host: localhost:43000
User-Agent: curl/8.2.1
Accept: */*
Content-Length: 29

Tom     Carpenter
Alice   Mechanic

curl should checked the current offset into the regular file and use its size - offset as content length instead of just using its size.


Other possible alternatives:

  • check if the offset into the file is not 0 after operning it, and perform a chunked upload in that case
$ nc -l 43000
PUT /4 HTTP/1.1
Host: localhost:43000
User-Agent: curl/8.2.1
Accept: */*
Transfer-Encoding: chunked
Expect: 100-continue

1d
Tom     Carpenter
Alice   Mechanic

0
[...]
  • seek to offset 0 after opening the regular file and send the entire file:
$ nc -l 43000
PUT /4 HTTP/1.1
Host: localhost:43000
User-Agent: curl/8.2.1
Accept: */*
Content-Length: 38

NAME    JOB
Tom     Carpenter
Alice   Mechanic

Those solutions are all fairly easy to implement. I, personally, would prefer either the first or the second approach, I don't really like the idea of curl seeking back to 0 after opening.

curl/libcurl version

curl 8.2.1 (I am using my https://sdf.org/ shell account to test this)

curl 8.2.1 (x86_64--netbsd) libcurl/8.2.1 OpenSSL/1.1.1u zlib/1.2.13 libidn2/2.3.4 nghttp2/1.52.0
Release-Date: 2023-07-26
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS GSS-API HSTS HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile libz NTLM NTLM_WB SPNEGO SSL threadsafe TLS-SRP UnixSocket

operating system

NetBSD sdf 9.3 NetBSD 9.3 (GENERIC) #0: Thu Aug  4 15:30:37 UTC 2022  [email protected]:/usr/src/sys/arch/amd64/compile/GENERIC amd64
emanuele6 added a commit to emanuele6/curl that referenced this issue Oct 21, 2023
curl will now also compute the content-length of the transfer if stdin
is the file to upload and stdin is a regular file, using its file size.

Since, while being a regular file, stdin could not have its offset at
the start of the file, curl will now also get the current offset into
the upload file's file descriptor and use (filesize - offset) as
content-length for transfer instead of just using the full filesize.
This also fixes a bug on BSDs where open("/dev/fd/N") behaves like
dup(N), so, if N is a file descriptor to a regular file, the file offset
of the file descriptor returned by open() may not have been at the start
of the file despite curl's previous assumption.

Since I don't know anything about VMS systems, I left the behaviour for
VMS unchanged; on VMS, curl will still perform a chunked transfer if the
upload file is stdin.

Fixes curl#12171
Fixes curl#12177
emanuele6 added a commit to emanuele6/curl that referenced this issue Oct 21, 2023
curl will now also compute the content-length of the transfer if stdin
is the file to upload and stdin is a regular file, using its file size.

Since, while being a regular file, stdin could not have its offset at
the start of the file, curl will now also get the current offset into
the upload file's file descriptor and use (filesize - offset) as
content-length for transfer instead of just using the full filesize.
This also fixes a bug on BSDs where open("/dev/fd/N") behaves like
dup(N), so, if N is a file descriptor to a regular file, the file offset
of the file descriptor returned by open() may not have been at the start
of the file despite curl's previous assumption.

Since I don't know anything about VMS systems, I left the behaviour for
VMS unchanged; on VMS, curl will still perform a chunked transfer if the
upload file is stdin.

Fixes curl#12171
Fixes curl#12177
emanuele6 added a commit to emanuele6/curl that referenced this issue Oct 24, 2023
curl will now also compute the content-length of the transfer if stdin
is the file to upload and stdin is a regular file, using its file size.

Since, while being a regular file, stdin could not have its offset at
the start of the file, curl will now also get the current offset into
the upload file's file descriptor and use (filesize - offset) as
content-length for transfer instead of just using the full filesize.
This also fixes a bug on BSDs where open("/dev/fd/N") behaves like
dup(N), so, if N is a file descriptor to a regular file, the file offset
of the file descriptor returned by open() may not have been at the start
of the file despite curl's previous assumption.

Since I don't know anything about VMS systems, I left the behaviour for
VMS unchanged; on VMS, curl will still perform a chunked transfer if the
upload file is stdin.

I updated tests that were testing chunked transfers for stdin to use
<stdin pipe="yes"> since now curl only performs a chunked transfer by
default if the upload file (stdin or not) is not a regular file.

Fixes curl#12171
Fixes curl#12177
emanuele6 added a commit to emanuele6/curl that referenced this issue Oct 24, 2023
curl will now also compute the content-length of the transfer if stdin
is the file to upload and stdin is a regular file, using its file size.

Since, while being a regular file, stdin could not have its offset at
the start of the file, curl will now also get the current offset into
the upload file's file descriptor and use (filesize - offset) as
content-length for transfer instead of just using the full filesize.
This also fixes a bug on BSDs where open("/dev/fd/N") behaves like
dup(N), so, if N is a file descriptor to a regular file, the file offset
of the file descriptor returned by open() may not have been at the start
of the file despite curl's previous assumption.

Since I don't know anything about VMS systems, I left the behaviour for
VMS unchanged; on VMS, curl will still perform a chunked transfer if the
upload file is stdin.

I updated tests that were testing chunked transfers for stdin to use
<stdin pipe="yes"> since now curl only performs a chunked transfer by
default if the upload file (stdin or not) is not a regular file.

Fixes curl#12171
Fixes curl#12177
emanuele6 added a commit to emanuele6/curl that referenced this issue Oct 24, 2023
curl will now also compute the content-length of the transfer if stdin
is the file to upload and stdin is a regular file, using its file size.

Since, while being a regular file, stdin could not have its offset at
the start of the file, curl will now also get the current offset into
the upload file's file descriptor and use (filesize - offset) as
content-length for transfer instead of just using the full filesize.
This also fixes a bug on BSDs where open("/dev/fd/N") behaves like
dup(N), so, if N is a file descriptor to a regular file, the file offset
of the file descriptor returned by open() may not have been at the start
of the file despite curl's previous assumption.

Since I don't know anything about VMS systems, I left the behaviour for
VMS unchanged; on VMS, curl will still perform a chunked transfer if the
upload file is stdin.

I updated tests that were testing chunked transfers for stdin to use
<stdin pipe="yes"> since now curl only performs a chunked transfer by
default if the upload file (stdin or not) is not a regular file.

Fixes curl#12171
Fixes curl#12177
emanuele6 added a commit to emanuele6/curl that referenced this issue Oct 24, 2023
curl will now also compute the content-length of the transfer if stdin
is the file to upload and stdin is a regular file, using its file size.

Since, while being a regular file, stdin could not have its offset at
the start of the file, curl will now also get the current offset into
the upload file's file descriptor and use (filesize - offset) as
content-length for transfer instead of just using the full filesize.
This also fixes a bug on BSDs where open("/dev/fd/N") behaves like
dup(N), so, if N is a file descriptor to a regular file, the file offset
of the file descriptor returned by open() may not have been at the start
of the file despite curl's previous assumption.

Since I don't know anything about VMS systems, I left the behaviour for
VMS unchanged; on VMS, curl will still perform a chunked transfer if the
upload file is stdin.

I updated tests that were testing chunked transfers for stdin to use
<stdin pipe="yes"> since now curl only performs a chunked transfer by
default if the upload file (stdin or not) is not a regular file.
And I added a new test (258) that verifies that stdin is uploaded with
content-length (not transfer-encoding:chunked) if it is a regular file.

Fixes curl#12171
Fixes curl#12177
emanuele6 added a commit to emanuele6/curl that referenced this issue Oct 24, 2023
curl will now also compute the content-length of the transfer if stdin
is the file to upload and stdin is a regular file, using its file size.

Since, while being a regular file, stdin could not have its offset at
the start of the file, curl will now also get the current offset into
the upload file's file descriptor and use (filesize - offset) as
content-length for transfer instead of just using the full filesize.
This also fixes a bug on BSDs where open("/dev/fd/N") behaves like
dup(N), so, if N is a file descriptor to a regular file, the file offset
of the file descriptor returned by open() may not have been at the start
of the file despite curl's previous assumption.

Since I don't know anything about VMS systems, I left the behaviour for
VMS unchanged; on VMS, curl will still perform a chunked transfer if the
upload file is stdin.

I updated tests that were testing chunked transfers for stdin to use
<stdin pipe="yes"> since now curl only performs a chunked transfer by
default if the upload file (stdin or not) is not a regular file.
And I added a new test (268) that verifies that stdin is uploaded with
content-length (not transfer-encoding:chunked) if it is a regular file.

Fixes curl#12171
Fixes curl#12177
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants