#863 question mark in the file name

closed-fixed
nobody
5
2014-08-19
2009-09-25
vi6nwnqrco
No

Curl from http://www.paehl.com/ , Windows XP SP2

>curl.exe -V

curl 7.19.6 (i386-pc-win32) libcurl/7.19.6 OpenSSL/1.0.0 zlib/1.2.3 libssh2/1.1
Protocols: tftp ftp telnet dict ldap http file https ftps scp sftp
Features: Largefile NTLM SSL libz

>curl.exe -L -O -k https://addons.mozilla.org/ru/firefox/downloads/latest/220/addon-220-latest.xpi?src=addondetail

% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 310k 0 0 0 0 0 0 --:--:-- 0:00:01 --:--:-- 0Warning: Failed to create the file addon-220-latest.xpi?src=addondetail
0 310k 0 1072 0 0 623 0 0:08:30 0:00:01 0:08:29 1046k
curl: (23) Failed writing body (0 != 1072)

Discussion

  • Dan Fandrich
    Dan Fandrich
    2009-09-25

    • labels: --> client module
    • milestone: --> wrong_behaviour
     
  • Dan Fandrich
    Dan Fandrich
    2009-09-25

    I posted te attached patch (now rebased to the latest source) to the mailing list last year (IIRC) to fix this problem, but without access to a Windows build system, I can't test it. If you can verify that it works, it will make it more likely to be checked in.

     
  • Dan Fandrich
    Dan Fandrich
    2009-09-25

    Don't use illegal file name characters under Windows

     
  • Thanks for reporting this issue and helping us improve curl and libcurl.

    We're awaiting feedback in this issue. Due to this, I have set the state of this issue to pending and it will automatically get closed later on unless we get further info.

    Please consider answering the outstanding questions or providing the missing info so that we can proceed to resolve this issue!

     
    • status: open --> pending
     
  • vi6nwnqrco
    vi6nwnqrco
    2009-10-14

    I dummy and was unable to compile the patched curl...

    Functions msdosify and rename_if_dos_device_name are inside "#ifdef MSDOS".

    After the change "#ifdef MSDOS" to "#if defined (MSDOS) || defined (WIN32)", deleting "&& (S_ISCHR(st_buf.st_mode))" and adding
    #ifdef WIN32
    #define PATH_MAX 512
    static const char *msdosify(const char *);
    static char *rename_if_dos_device_name(char *);
    #endif /* WIN32 */

    I managed to compile curl with one warning:
    main.c(5566) : warning C4133: 'function' : incompatible types - from 'struct stat *' to 'struct _stati64 *'

    >curl.exe -L -O -k http://addons.mozilla.org/ru/firefox/downloads/latest/220/addon-220-latest.xpi?src=addondetail

    I got a file "addon-220-latest.xpi_src=addondetail".

    Compiled binary and patched main.c: http://www.rapidspread.com/file.jsp?id=bmksu5a63v

     
  • vi6nwnqrco
    vi6nwnqrco
    2009-10-14

    • status: pending --> open
     
  • Dan Fandrich
    Dan Fandrich
    2009-10-16

    I've tweaked your patch to come up with the new one which I am attaching. Would you please give that a try?

    Also, under what conditions do you see that corrupted file name? Can you show a complete session showing what commands you ran to get that file name and where the corrupted file name is seen? Thanks.

     
  • Dan Fandrich
    Dan Fandrich
    2009-10-16

    Improved patch

     
    Attachments
  • vi6nwnqrco
    vi6nwnqrco
    2009-10-16

    Thank you. PATH_MAX in MSVC6 in LIMITS.H defined within "#ifdef _POSIX_" - i added "#define _POSIX_" before "#include <limits.h>" and compiled with warnings.

    After the first patch and my changes curl created a new file with junk in the name if the file "addon-220-latest.xpi_src=addondetail" already exist (if run twice "curl.exe -L -O -k https://addons.mozilla.org/ru/firefox/downloads/latest/220/addon-220-latest.xpi?src=addondetail").

    If the curl can't create the file name displayed in the message "failed to create ...". I gave a screenshot of file manager.

    With the second patch an existing file is overwritten.

    cl.exe /MD /O2 /DNDEBUG /I../lib /I../include /nologo /W3 /GX /DWIN32 /YX /FD /c /DCURL_STATICLIB /DUSE_SSLEAY /DCURL_STATICLIB /Fo"mainr.obj" main.c
    main.c
    main.c(419) : warning C4013: '_lseeki64' undefined; assuming extern returning int
    main.c(422) : warning C4013: '_get_osfhandle' undefined; assuming extern returning int
    main.c(2349) : warning C4013: 'setmode' undefined; assuming extern returning int
    main.c(3357) : warning C4013: 'read' undefined; assuming extern returning int
    main.c(4518) : warning C4013: 'open' undefined; assuming extern returning int
    main.c(4522) : warning C4013: 'close' undefined; assuming extern returning int
    main.c(4557) : warning C4013: 'isatty' undefined; assuming extern returning int
    main.c(5419) : warning C4013: 'access' undefined; assuming extern returning int
    main.c(5566) : warning C4133: 'function' : incompatible types - from 'struct stat *' to 'struct _stati64 *'

     
  • Yang Tse
    Yang Tse
    2009-10-16

    Hi Dan and vi6nwnqrco,

    The inlined, due to inability to attach it, patch here relative to CVS works for me here with VC6 and W2K.
    It is a slight variation on Dans's winfn.diff patch. Notice struct_stat near the end.

    Index: src/main.c

    RCS file: /cvsroot/curl/curl/src/main.c,v
    retrieving revision 1.537
    diff -u -r1.537 main.c
    --- src/main.c 15 Oct 2009 17:34:09 -0000 1.537
    +++ src/main.c 16 Oct 2009 15:01:14 -0000
    @@ -161,10 +161,6 @@
    #ifdef MSDOS
    #define USE_WATT32
    #include <dos.h>
    -
    -static const char *msdosify(const char *);
    -static char *rename_if_dos_device_name(char *);
    -
    #ifdef DJGPP
    /* we want to glob our own argv[] */
    char **__crt0_glob_function (char *arg)
    @@ -204,6 +200,9 @@
    #include <direct.h>
    #define F_OK 0
    #define mkdir(x,y) (mkdir)(x)
    +#define PATH_MAX MAX_PATH
    +#define S_ISCHR(m) (((m) & S_IFMT) == S_IFCHR)
    +#define _use_lfn(f) 1 /* long file names always available */
    #endif

    /*
    @@ -273,6 +272,11 @@
    # endif
    #endif

    +#if defined(MSDOS) || defined(WIN32)
    +static const char *msdosify(const char *);
    +static char *rename_if_dos_device_name(char *);
    +#endif
    +
    #ifdef CURL_DOES_CONVERSIONS
    #ifdef HAVE_ICONV
    iconv_t inbound_cd = (iconv_t)-1;
    @@ -4350,9 +4354,9 @@
    free(url);
    break;
    }
    -#if defined(MSDOS)
    +#if defined(MSDOS) || defined(WIN32)
    {
    - /* This is for DOS, and then we do some major replacing of
    + /* For DOS and WIN32, we do some major replacing of
    bad characters in the file name before using it */
    char file1[PATH_MAX];
    if(strlen(outfile) >= PATH_MAX)
    @@ -4366,7 +4370,7 @@
    break;
    }
    }
    -#endif /* MSDOS */
    +#endif /* MSDOS || WIN32 */
    }
    else if(urls) {
    /* fill '#1' ... '#9' terms from URL pattern */
    @@ -5411,7 +5415,7 @@
    return result; /* 0 is fine, -1 is badness */
    }

    -#ifdef MSDOS
    +#if defined(MSDOS) || defined(WIN32)

    #ifndef HAVE_BASENAME
    /* basename() returns a pointer to the last component of a pathname.
    @@ -5455,14 +5459,8 @@
    const char * const dlimit = dos_name + sizeof(dos_name) - 1;
    const char *illegal_aliens = illegal_chars_dos;
    size_t len = sizeof (illegal_chars_dos) - 1;
    - int lfn = 0;

    -#ifdef DJGPP
    - /* Support for Windows 9X VFAT systems, when available (djgpp only). */
    - if (_use_lfn (file_name))
    - lfn = 1;
    -#endif
    - if (lfn) {
    + if (_use_lfn (file_name)) {
    illegal_aliens = illegal_chars_w95;
    len -= (illegal_chars_w95 - illegal_chars_dos);
    }
    @@ -5541,7 +5539,7 @@
    * retrieve such a file would fail at best and wedge us at worst. We need
    * to rename such files. */
    char *base;
    - struct stat st_buf;
    + struct_stat st_buf;
    char fname[PATH_MAX];

    strncpy(fname, file_name, PATH_MAX-1);
    @@ -5562,4 +5560,4 @@
    }
    return file_name;
    }
    -#endif /* MSDOS */
    +#endif /* MSDOS || WIN32 */

     
  • Dan Fandrich
    Dan Fandrich
    2009-10-16

    The latest patch doesn't set _use_lfn or S_ISCHR in the non-DJPP MS-DOS case. Also, is struct_stat really valid in all of Windows, DJPP and other MS-DOS cases? I've never seen it before.

     
  • Yang Tse
    Yang Tse
    2009-10-17

    Dan, relative to your concern about last patch not setting _use_lfn or S_ISCHR in the non-DJPP MS-DOS case, yes it is actually a perfectly valid concern. Updated and inlined now patch tracks more closely yours and additionally also takes in consideration that the _use_lfn() function is not available on DJGPP versions older than 2.0.

    Relative to the struct_stat preprocessor macro, lines near #240 in unpatched main.c have the required magic available since 7.19.0.

    Placement of the PATH_MAX definition for MSVC is fine, daily builds will show if it is also appropriate for other compilers.

    vi6nwnqrco, could you verify if this works for you?

    Updated patch follows...

    Index: src/main.c

    RCS file: /cvsroot/curl/curl/src/main.c,v
    retrieving revision 1.537
    diff -u -r1.537 main.c
    --- src/main.c 15 Oct 2009 17:34:09 -0000 1.537
    +++ src/main.c 17 Oct 2009 02:26:11 -0000
    @@ -158,13 +158,30 @@
    #define O_BINARY 0
    #endif

    +#if defined(MSDOS) || defined(WIN32)
    +static const char *msdosify(const char *);
    +static char *rename_if_dos_device_name(char *);
    +
    +#ifndef S_ISCHR
    +# ifdef S_IFCHR
    +# define S_ISCHR(m) (((m) & S_IFMT) == S_IFCHR)
    +# else
    +# define S_ISCHR(m) (0) /* cannot tell if file is a device */
    +# endif
    +#endif
    +
    +#ifdef WIN32
    +# define _use_lfn(f) (1) /* long file names always available */
    +#elif !defined(__DJGPP__) || (__DJGPP__ < 2) /* DJGPP 2.0 has _use_lfn() */
    +# define _use_lfn(f) (0) /* long file names never available */
    +#endif
    +
    +#endif /* MSDOS || WIN32 */
    +
    #ifdef MSDOS
    #define USE_WATT32
    #include <dos.h>

    -static const char *msdosify(const char *);
    -static char *rename_if_dos_device_name(char *);
    -
    #ifdef DJGPP
    /* we want to glob our own argv[] */
    char **__crt0_glob_function (char *arg)
    @@ -204,6 +221,7 @@
    #include <direct.h>
    #define F_OK 0
    #define mkdir(x,y) (mkdir)(x)
    +#define PATH_MAX MAX_PATH
    #endif

    /*
    @@ -4350,9 +4368,9 @@
    free(url);
    break;
    }
    -#if defined(MSDOS)
    +#if defined(MSDOS) || defined(WIN32)
    {
    - /* This is for DOS, and then we do some major replacing of
    + /* For DOS and WIN32, we do some major replacing of
    bad characters in the file name before using it */
    char file1[PATH_MAX];
    if(strlen(outfile) >= PATH_MAX)
    @@ -4366,7 +4384,7 @@
    break;
    }
    }
    -#endif /* MSDOS */
    +#endif /* MSDOS || WIN32 */
    }
    else if(urls) {
    /* fill '#1' ... '#9' terms from URL pattern */
    @@ -5411,7 +5429,7 @@
    return result; /* 0 is fine, -1 is badness */
    }

    -#ifdef MSDOS
    +#if defined(MSDOS) || defined(WIN32)

    #ifndef HAVE_BASENAME
    /* basename() returns a pointer to the last component of a pathname.
    @@ -5455,14 +5473,9 @@
    const char * const dlimit = dos_name + sizeof(dos_name) - 1;
    const char *illegal_aliens = illegal_chars_dos;
    size_t len = sizeof (illegal_chars_dos) - 1;
    - int lfn = 0;

    -#ifdef DJGPP
    - /* Support for Windows 9X VFAT systems, when available (djgpp only). */
    - if (_use_lfn (file_name))
    - lfn = 1;
    -#endif
    - if (lfn) {
    + /* Support for Windows 9X VFAT systems, when available. */
    + if (_use_lfn (file_name)) {
    illegal_aliens = illegal_chars_w95;
    len -= (illegal_chars_w95 - illegal_chars_dos);
    }
    @@ -5541,7 +5554,7 @@
    * retrieve such a file would fail at best and wedge us at worst. We need
    * to rename such files. */
    char *base;
    - struct stat st_buf;
    + struct_stat st_buf;
    char fname[PATH_MAX];

    strncpy(fname, file_name, PATH_MAX-1);
    @@ -5562,4 +5575,4 @@
    }
    return file_name;
    }
    -#endif /* MSDOS */
    +#endif /* MSDOS || WIN32 */

     
  • vi6nwnqrco
    vi6nwnqrco
    2009-10-17

    Compiled with warnings but works.
    But does not work resume downloading. 7.19.6 also not resume. Another bug?

    cl.exe /MD /O2 /DNDEBUG /I../lib /I../include /nologo /W3 /GX /DWIN32 /YX /FD /c /DCURL_STATICLIB /DUSE_SSLEAY /DCURL_STATICLIB /Fo"mainr.obj" main.c
    main.c
    main.c(224) : warning C4005: 'PATH_MAX' : macro redefinition
    D:\MSVC6lite\VC98\INCLUDE\limits.h(110) : see previous definition of 'PATH_MAX'
    main.c(416) : warning C4013: '_lseeki64' undefined; assuming extern returning int
    main.c(419) : warning C4013: '_get_osfhandle' undefined; assuming extern returning int
    main.c(2346) : warning C4013: 'setmode' undefined; assuming extern returning int
    main.c(3354) : warning C4013: 'read' undefined; assuming extern returning int
    main.c(4515) : warning C4013: 'open' undefined; assuming extern returning int
    main.c(4519) : warning C4013: 'close' undefined; assuming extern returning int
    main.c(4554) : warning C4013: 'isatty' undefined; assuming extern returning int
    main.c(5416) : warning C4013: 'access' undefined; assuming extern returning int

    >curl.exe -C - -L -O -k https://addons.mozilla.org/ru/firefox/downloads/latest/220/addon-220-latest.xpi?src=addondetail
    % Total % Received % Xferd Average Speed Time Time Time Current
    Dload Upload Total Spent Left Speed
    11 315k 11 37280 0 0 6379 0 0:00:50 0:00:05 0:00:45 10416^C

    >curl.exe -C - -L -O -k https://addons.mozilla.org/ru/firefox/downloads/latest/220/addon-220-latest.xpi?src=addondetail
    ** Resuming transfer from byte position 39424
    % Total % Received % Xferd Average Speed Time Time Time Current
    Dload Upload Total Spent Left Speed
    0 4095M 0 0 0 0 0 0 --:--:-- 0:00:11 --:--:-- 0
    curl: (18) transfer closed with 4294927872 bytes remaining to read

     
  • Yang Tse
    Yang Tse
    2009-10-17

    vi6nwnqrco ,

    I've tested it also on WinXP and resume works properly. I'm committing the patch to CVS right away. If you give us your name, credit will be given in change log.

    I fear that all the problems you are having with compiler warnings and binaries not working properly has more to do with your build environment than with the code itself.

    From the following warning I am able to conclude that you have not installed "Windows Server 2003 PSDK" or that you have not configured your compiler properly to use it.

    "main.c(224) : warning C4005: 'PATH_MAX' : macro redefinition
    D:\MSVC6lite\VC98\INCLUDE\limits.h(110) : see previous definition
    of 'PATH_MAX'

    You can download "Windows Server 2003 PSDK" from http://www.microsoft.com/msdownload/platformsdk/sdkupdate/psdk-full.htm

    _Additionally_ verify that you are building alll related libraries and executable with the same C runtime.

     
  • vi6nwnqrco
    vi6nwnqrco
    2009-10-17

    Sometimes the server gives incorrect headers. I ran curl with the --dump-header and --trace-ascii: http://www.massmirror.com/3a54b2114a1673e83d86e4f7acc77a52.html

    Content-Length: 4294893733
    Content-Range: bytes 73563-4294967295/0

    > If you give us your name

    Anonymous :-)

    > You can download "Windows Server 2003 PSDK"

    Thanks, but I'll wait for the new release on the download page.

     
  • Thanks for the report, this problem is now fixed in CVS!

     
    • status: open --> closed-fixed