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

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

Comments

@piru
Copy link

@piru 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 piru commented Jun 9, 2021

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
... as otherwise it creates a rather unexpected target directory with a
leading slash.

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

@bagder bagder commented Jun 10, 2021

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

@piru
Copy link
Author

@piru 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 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 in 2784a58 Jun 11, 2021
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