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

Native Windows curl doesn't create paths with forward slashes #1007

Closed
RyanGlScott opened this Issue Sep 11, 2016 · 16 comments

Comments

Projects
None yet
3 participants
@RyanGlScott

RyanGlScott commented Sep 11, 2016

(Originally reported as Alexpux/MINGW-packages#1707, re-opened here by suggestion)

I did this

I used the native Windows version of curl (in MSYS2 terms, this is mingw-w64-x86_64-curl) to download a file to a path containing forward slashes, but it failed unexpectedly:

$ /mingw64/bin/curl -L https://downloads.haskell.org/~ghc/mingw/x86_64/mingw-w64-x86_64-crt-git-5.0.0.4531.49c7046-1-any.pkg.tar.xz -o ghc-tarballs/mingw-w64/x86_64/mingw-w64-x86_64-crt-git-5.0.0.4531.49c7046-1-any.pkg.tar.xz --create-dirs -#
Warning: Failed to create the file
Warning: ghc-tarballs/mingw-w64/x86_64/mingw-w64-x86_64-crt-git-5.0.0.4531.49c7
Warning: 046-1-any.pkg.tar.xz: No such file or directory

curl: (23) Failed writing body (0 != 2759)

I expected the following

I expected it to behave like curl does on Linux, or how the MSYS2-built version of curl behaves (i.e., how curl behaves in a POSIX emulation layer):

$ /usr/bin/curl -L https://downloads.haskell.org/~ghc/mingw/x86_64/mingw-w64-x86_64-crt-git-5.0.0.4531.49c7046-1-any.pkg.tar.xz -o ghc-tarballs/mingw-w64/x86_64/mingw-w64-x86_64-crt-git-5.0.0.4531.49c7046-1-any.pkg.tar.xz --create-dirs -#
######################################################################## 100.0%

A workaround is to convert all the forward slashes (/) in that path to backslashes (\\):

$ /mingw64/bin/curl -L https://downloads.haskell.org/~ghc/mingw/x86_64/mingw-w64-x86_64-crt-git-5.0.0.4531.49c7046-1-any.pkg.tar.xz -o ghc-tarballs\\mingw-w64\\x86_64\\mingw-w64-x86_64-crt-git-5.0.0.4531.49c7046-1-any.pkg.tar.xz --create-dirs -#
######################################################################## 100.0%

curl/libcurl version

I reproduced this with versions 7.50.1 and 7.50.2.

operating system

Windows 10 (64-bit)

I installed the native Windows version and the MSYS2 version of curl using pacman -S mingw-w64-x86_64-curl and pacman -S curl, respectively, but @vszakats reports experiencing the same issue using the Windows version of curl-7.50.2 available for download from the curl website.

@RyanGlScott

This comment has been minimized.

RyanGlScott commented Sep 11, 2016

On a similar note, it appears that the MSYS2 version of curl (the one run through a POSIX emulation layer) won't work on paths with backslashes either:

$ /usr/bin/curl -L https://downloads.haskell.org/~ghc/mingw/x86_64/mingw-w64-x86_64-crt-git-5.0.0.4531.49c7046-1-any.pkg.tar.xz -o ghc-tarballs\\mingw-w64\\x86_64\\mingw-w64-x86_64-crt-git-5.0.0.4531.49c7046-1-any.pkg.tar.xz --create-dirs -#
Warning: Failed to create the file
Warning: ghc-tarballs\mingw-w64\x86_64\mingw-w64-x86_64-crt-git-5.0.0.4531.49c7
Warning: 046-1-any.pkg.tar.xz: No such file or directory

curl: (23) Failed writing body (0 != 2759)
@bagder

This comment has been minimized.

Member

bagder commented Sep 11, 2016

So this: https://github.com/curl/curl/blob/master/lib/curl_setup.h#L448 should have some #ifdef logic to detect mingw and then use a forward slash?

@bagder bagder added the cmdline tool label Sep 11, 2016

@vszakats

This comment has been minimized.

Member

vszakats commented Sep 11, 2016

Ideally the code should recognise both \ and / as directory separator on Windows (not just mingw). Perhaps by first converting user-supplied paths to an internal representation using OS-native dirseps and then continue to use existing DIR_CHAR. Or, by accepting either character where DIR_CHAR is currently accepted.

@RyanGlScott

This comment has been minimized.

RyanGlScott commented Sep 11, 2016

I don't think that'd be the fix I want, since if you require paths to be separated by forward slashes with MinGW-w64, then paths like foo\\bar\\baz would no longer be able to be parsed, right? (At least, if this comment is any indication.)

I'd want at least the MinGW-w64 version of curl to parse both types of path separators, since there are numerous scenarios where both are used on Windows. Really, I feel like any version of curl should be able to parse both, but maybe that's a taller order...

FWIW, this command:

$ /mingw64/bin/curl -L https://downloads.haskell.org/~ghc/mingw/x86_64/mingw-w64-x86_64-crt-git-5.0.0.4531.49c7046-1-any.pkg.tar.xz -o ghc-tarballs/mingw-w64/x86_64/mingw-w64-x86_64-crt-git-5.0.0.4531.49c7046-1-any.pkg.tar.xz --create-dirs -#

seems to work if the ghc-tarballs/mingw-w64/x86_64 directory already exists. It's only when curl has to create ghc-tarballs/mingw-w64/x86_64 itself (through --create-dirs) that problems arise. So it looks like the MinGW-w64 version of curl can successfully navigate paths with forward slashes, but creating them is where it chokes.

@bagder

This comment has been minimized.

Member

bagder commented Sep 11, 2016

The current command line tool logic for this uses only one slash: https://github.com/curl/curl/blob/master/src/tool_dirhie.c#L117 I suppose that function should be fixed to work with backslashes and forward slashes (on windows)? Or do we have this problem elsewhere as well?

@RyanGlScott

This comment has been minimized.

RyanGlScott commented Sep 11, 2016

I suppose that function should be fixed to work with backslashes and forward slashes (on windows)?

That sounds good to me!

@vszakats

This comment has been minimized.

Member

vszakats commented Sep 11, 2016

Similar issue may be present on MS-DOS and OS/2. Though I don't have information on how expected it is from tools to transparently accept forward slashes on these systems — probably not very.

@bagder

This comment has been minimized.

Member

bagder commented Sep 12, 2016

@vszakats I don't remember DOS and OS/2 using the forward slash very much for that, but the code should probably be written with a nice #ifdef in there somehow anyway and then the DOS and OS/2 aware people can later switch on that logic if deemed applicable. I don't think we have a very large user base on those operating systems (but yes it exists).

@RyanGlScott you feel like taking a stab at writing an improved version of that function?

@vszakats

This comment has been minimized.

Member

vszakats commented Sep 12, 2016

@bagder Agreed!

@bagder

This comment has been minimized.

Member

bagder commented Sep 13, 2016

Here's my suggested patch (updated since initial post):

From b97310641ef8063e6afc89d01bcb5ea04e0b517e Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <daniel@haxx.se>
Date: Tue, 13 Sep 2016 15:20:05 +0200
Subject: [PATCH] curl: make --create-dirs on windows grok both forward and
 backward slashes

Reported-by: Ryan Scott

Fixes #1007
---
 src/tool_dirhie.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/tool_dirhie.c b/src/tool_dirhie.c
index db810d6..23bb2cb 100644
--- a/src/tool_dirhie.c
+++ b/src/tool_dirhie.c
@@ -89,10 +89,18 @@ static void show_dir_errno(FILE *errors, const char *name)
  *  multi-GETs in file output, ie:
  *  curl "http://my.site/dir[1-5]/file[1-5].txt" -o "dir#1/file#2.txt"
  *  should create all the dir* automagically
  */

+#ifdef WIN32
+/* systems that may use either or when specifying a path */
+#define PATH_DELIMITERS "\\/"
+#else
+#define PATH_DELIMITERS DIR_CHAR
+#endif
+
+
 CURLcode create_dir_hierarchy(const char *outfile, FILE *errors)
 {
   char *tempdir;
   char *tempdir2;
   char *outdup;
@@ -112,22 +120,23 @@ CURLcode create_dir_hierarchy(const char *outfile, FILE *errors)
   }
   dirbuildup[0] = '\0';

   /* Allow strtok() here since this isn't used threaded */
   /* !checksrc! disable BANNEDFUNC 2 */
-  tempdir = strtok(outdup, DIR_CHAR);
+  tempdir = strtok(outdup, PATH_DELIMITERS);

   while(tempdir != NULL) {
-    tempdir2 = strtok(NULL, DIR_CHAR);
+    tempdir2 = strtok(NULL, PATH_DELIMITERS);
     /* since strtok returns a token for the last word even
        if not ending with DIR_CHAR, we need to prune it */
     if(tempdir2 != NULL) {
       size_t dlen = strlen(dirbuildup);
       if(dlen)
         snprintf(&dirbuildup[dlen], outlen - dlen, "%s%s", DIR_CHAR, tempdir);
       else {
-        if(0 != strncmp(outdup, DIR_CHAR, 1))
+        if(outdup == tempdir)
+          /* the output string doesn't start with a separator */
           strcpy(dirbuildup, tempdir);
         else
           snprintf(dirbuildup, outlen, "%s%s", DIR_CHAR, tempdir);
       }
       if(access(dirbuildup, F_OK) == -1) {
-- 
2.9.3
@bagder

This comment has been minimized.

Member

bagder commented Sep 13, 2016

(of course with a Reported-by: Ryan Scott in there as well)

@RyanGlScott

This comment has been minimized.

RyanGlScott commented Sep 13, 2016

Thanks, @badger! When I get a chance I'll try this patch out and see if it fixes the issues I've been having.

@bagder

This comment has been minimized.

Member

bagder commented Sep 13, 2016

(and just for the record, I'm @bagder, which the g and d switched!)

@RyanGlScott

This comment has been minimized.

RyanGlScott commented Sep 13, 2016

My apologies, @bagder! And to @badger as well, sorry for pinging you needlessly :)

@RyanGlScott

This comment has been minimized.

RyanGlScott commented Sep 14, 2016

That patch works beautifully! curl now successfully creates the path ghc-tarballs/mingw-w64/x86_64, regardless of the direction of the slashes.

@bagder bagder closed this in ffa0709 Sep 14, 2016

@bagder

This comment has been minimized.

Member

bagder commented Sep 14, 2016

thanks!

@lock lock bot locked as resolved and limited conversation to collaborators May 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.