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.

Sign up for GitHub

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

Unexpected / unportable --output-dir behaviour #7218

Closed
piru opened this issue Jun 9, 2021 · 4 comments
Closed

Unexpected / unportable --output-dir behaviour #7218

piru opened this issue Jun 9, 2021 · 4 comments
Labels
cmdline tool

Comments

@piru
Copy link

piru commented Jun 9, 2021

I did this

curl --output-dir "" -O https://curl.se/index.html

Curl attempts to write file to "/index.html" instead of "index.html"

In fact, when merging the output-dir and the actual file name curl explicitly uses / as a path separator. This isn't universally portable, some platforms may use different path separator character (however / is used in most).

I expected the following

More consistent and platform agnostic behaviour. Perhaps it would be more portable to actually chdir to the --output-dir rather than trying to construct the path manually.

curl/libcurl version

curl 7.77.1-DEV (x86_64-pc-linux-gnu) libcurl/7.77.1-DEV OpenSSL/1.1.1k zlib/1.2.11 brotli/1.0.9 libidn2/2.3.0 libpsl/0.21.0 (+libidn2/2.3.0) libssh2/1.9.0 nghttp2/1.43.0 librtmp/2.3 OpenLDAP/2.4.57
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtmp rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS brotli HSTS HTTP2 HTTPS-proxy IDN IPv6 Largefile libz NTLM NTLM_WB PSL SSL TLS-SRP UnixSockets

operating system

Linux hostname 5.10.0-6-amd64 #1 SMP Debian 5.10.28-1 (2021-04-09) x86_64 GNU/Linux
@piru
Copy link
Author

piru commented Jun 9, 2021 •
edited

The issue with chdir is that then you'd need to restore the working directory afterwards. You can use getcwd to get the current working directory.

Another (more modern) option would be to use open on the directory and then openat(dirfd, filename, ...) + fdopen but I believe this is less portable than getcwd +chdir + open + chdir.

bagder added a commit that referenced this issue Jun 10, 2021
@bagder
single_transfer: ignore blank --output-dir
706b21e 
... as otherwise it creates a rather unexpected target directory with a
leading slash.

Reported-by: Harry Sintonen
Fixes #7218
@bagder
Copy link
Member

bagder commented Jun 10, 2021

Can you name a current platform for which this is a portability problem?

@bagder bagder mentioned this issue Jun 10, 2021
Closed
@piru
Copy link
Author

piru commented Jun 10, 2021

Can you name a current platform for which this is a portability problem?

VMS seems to be affected, at least. They use quite "special" paths: https://wiki.vmssoftware.com/File_specification

AmigaOS and compatibles could also be problematic in some cases as "foo/bar" is "foo/bar" but "foo//bar" is "foo/../bar" ... this normally is no issue except when something assumes that multiple // are "ignored". This can become problematic if code makes assumptions about this. Technically this specific use case isn't affected by this, I think.

@bagder
Copy link
Member

bagder commented Jun 10, 2021

Thanks. I think I'll stick to fixing the empty dir issue with my PR and leave the portability issue for later/someone else...

@bagder bagder closed this as completed in 2784a58 Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
cmdline tool
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

single_transfer: ignore blank --output-dir curl/curl
2 participants
@bagder @piru