View Ticket
Not logged in
Ticket UUID: 40df1ced7ebd6a8b9e419ff1a788ce05e33a82e1
Title: Sync fail/crash with file: URL on Win32
Status: Fixed Type: Code_Defect
Severity: Severe Priority:
Subsystem: Resolution: Fixed
Last Modified: 2009-06-25 12:54:02
Version Found In: 767ae79c3d
Description & Comments:
On Windows (XP SP3) it is possible to clone a repository using a file: URL, but attempting a pull, push or sync fails (and occasionally crashes).

This occurs whether you are in an open repository or specifying the repository via -R. Also no difference between absolute and relative file: URLs.

To reproduce:

C:> mkdir repo1 && cd repo1
C:> fossil new repo1.fossil
C:> cd ..
C:> mkdir repo2 && cd repo2
C:> fossil clone file://../repo1/repo1.fossil repo2.fossil
C:> fossil sync file://../repo1/repo1.fossil -R repo2.fossil
                Bytes      Cards  Artifacts     Deltas
Send:             263          3          0          0
'fossil" http "C:' is not recognized as an internal or external command,
operable program or batch file.
C:> fossil open repo2.fossil
C:> fossil sync file://../repo1/repo1.fossil
                Bytes      Cards  Artifacts     Deltas
Send:             263          3          0          0
'fossil" http "C:' is not recognized as an internal or external command,
operable program or batch file.

anonymous claiming to be Twylite added on 2009-06-23 16:41:36:
Some more information:

transport_flip() in http_transport.c is using system() to execute "fossil http" with some parameters. By adding a strategic 'echo' to the front of the command I can tell that it's trying to do this ...

cmd /c "fossil" http "C:/Temp/test/repo1/repo1.fossil" "C:/Temp/test/repo2/repo2.fossil-17469728767716370470-out.http" "C:/Temp/test/repo2/repo2.fossil-17469728767716370470-in.http" 127.0.0.1

... which executed on a command line gives the same error that I was seeing, i.e. 'fossil" http "C:' is not recognized ....

The help for 'cmd' says the following:

1. If all of the following conditions are met (SNIP - they aren't)

2. Otherwise, old behavior is to see if the first character is a quote character and if so, strip the leading character and remove the last quote character on the command line, preserving any text after the last quote character.

So on Windows XP we need to add additional double-quotes around the entire command to make it work correctly (and that should also be backwards compatible to 2000, NT, etc.).

I assume that /bin/sh on *nix will be less happy about such a change.

The crash is not directly related to this -- it would seem that waiting for "fossil http" causes a memory access violation (the crash happens after a similar delay if I execute a different/successful command in the system() call).


anonymous claiming to be Twylite added on 2009-06-23 20:26:51:
Possible fix: use an #ifdef to change quoting on WIN32.

The following patch appears to fix the problem (tested on WinXP SP3 under cmd shell and msys). It should leave the behaviour on *nix systems untouched.

--- src/http_transport.c
+++ src/http_transport.c
@@ -140,14 +140,21 @@
 /*
 ** This routine is called when the outbound message is complete and
 ** it is time to being recieving a reply.
 */
 void transport_flip(void){
+#ifdef __MINGW32__
+  /* If a command begins with a quote, cmd.exe will strip the leading char
+   * and the last quote char on the line.  See 'cmd /?' */
+  char* cmdformat = "\"\"%s\" http \"%s\" \"%s\" \"%s\" 127.0.0.1\"";
+#else
+  char* cmdformat = "\"%s\" http \"%s\" \"%s\" \"%s\" 127.0.0.1";
+#endif
   if( g.urlIsFile ){
     char *zCmd;
     fclose(transport.pFile);
-    zCmd = mprintf("\"%s\" http \"%s\" \"%s\" \"%s\" 127.0.0.1",
+    zCmd = mprintf(cmdformat,
        g.argv[bad-link: 0]0, g.urlName, transport.zOutFile, transport.zInFile
     );
     system(zCmd);
     free(zCmd);
     transport.pFile = fopen(transport.zInFile, "rb");

anonymous claiming to be Twylite added on 2009-06-24 12:53:43:
I think I posted that patch as 'pre' instead of 'verbatim'. To safe a lot of frustration if you didn't notice the [bad-link:0] in there, here's a better copy:

--- src/http_transport.c
+++ src/http_transport.c
@@ -140,14 +140,21 @@
 /*
 ** This routine is called when the outbound message is complete and
 ** it is time to being recieving a reply.
 */
 void transport_flip(void){
+#ifdef __MINGW32__
+  /* If a command begins with a quote, cmd.exe will strip the leading char
+   * and the last quote char on the line.  See 'cmd /?' */
+  char* cmdformat = "\"\"%s\" http \"%s\" \"%s\" \"%s\" 127.0.0.1\"";
+#else
+  char* cmdformat = "\"%s\" http \"%s\" \"%s\" \"%s\" 127.0.0.1";
+#endif
   if( g.urlIsFile ){
     char *zCmd;
     fclose(transport.pFile);
-    zCmd = mprintf("\"%s\" http \"%s\" \"%s\" \"%s\" 127.0.0.1",
+    zCmd = mprintf(cmdformat,
        g.argv[0], g.urlName, transport.zOutFile, transport.zInFile
     );
     system(zCmd);
     free(zCmd);
     transport.pFile = fopen(transport.zInFile, "rb");


drh added on 2009-06-25 12:54:02:
Fixed by check-in 2554ba460dd8b56