bug-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

update: patch: relative path bugs in cvs (client/server mode only)


From: Chris Bohn
Subject: update: patch: relative path bugs in cvs (client/server mode only)
Date: Wed, 31 Mar 2004 15:10:22 -0500
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113

Larry Jones pointed out my diff line numbers absolutely needed to be correct (my note indicated they may not be in all cases) to be useful, so everything below is the same except I made the diffs using the unified (-u) option.

----------------------------------

This only affects client/server modes of running, and it affects all versions of
cvs since the introduction of multiple repository support.

This is also tracked in issue 81:
http://ccvs.cvshome.org/issues/show_bug.cgi?id=81

I wanted to submit the patch to this list as well because that is the way
suggested in HACKING.  See the issue for full details, but the patch is also
pasted below.  I really think this should be in the next release since
client/server modes can't handle relative paths correctly without it.

Chris

Note that while I tested these changes under Windows, there is currently no
regression testing framework for it, but the changes are regression tested
under UNIX (though one change is specific to Windows).

NEWS entry:
* using relative paths now works under client/server mode

Log entry:
2004-03-30        Chris Bohn  <address@hidden>

  * client.c, client.h (send_files, send_file_names, send_max_dotdot):
  I pulled out the code for calculating and sending max-dotdot to the
  server into a new function that is now called from both send_files
  and send_file_names.  Max-dotdot was previously only sent from
  send_file_names.  That was fine because send_file_names was called
  before send_files for a while, but when multiple repository support
  was added, the send_files and send_file_names calls were swapped, which
  led to Max-dotdot not getting sent at the right time.  That led to
  relative path operations typically failing.  This change leaves
  send_file_names sending Max-dotdot and adds it being sent with send_files.
  This fixes the relative path problems described in issue 81.  It may be ok
  to remove sending Max-dotdot from send_file_names, but since I'm sure all
  possible variations likely aren't covered by the regression tests, I thought
  it would be safer to leave it, and it causes no harm (other than a little
  extra traffic).

  *subr.c, cvs.h (pathname_levels, path_to_UNIX):  pathname_levels only looks
  for "/" characters for delimiting paths.  That's no good under Windows, so
  I modified it to call a new function, path_to_UNIX, before doing its work.
  The new function basically changes "\" to "/".  The FIXME in pathname_levels
  is still a better solution, but it is also more complicated.  This fixes the
  immediate problem of relative paths not working under Windows (part of issue
  81).  Note that a side-effect of the path_to_UNIX call is that extra "/"
  characters get stripped from relative paths (as I found in the regression
  run of sanity.sh), so a commit of ..///HACKING will result in cvs reporting
  the file ../HACKING was checked-in as opposed to ..///HACKING.  I think the
  new behavior is better, so I left it.  If people think it isn't better or is
  risky since it changes current behavior, we could modify the path_to_UNIX
  function to leave multiple "/" characters intact (or only run it if built
  under Windows).

  *sanity.sh (files, status): modified files-13 to remove extra "/" characters
  that are now stripped in relative paths, modified files-14 to be a successful
  test instead of a failure now that relative paths are supported.  Added a few
  extra tests to status to test more relative path command lines.

[D:\temp\ccvs\src]cvs diff -u -r cvs1-12-6 client.h
Index: client.h
===================================================================
RCS file: /cvs/ccvs/src/client.h,v
retrieving revision 1.47
diff -u -r1.47 client.h
--- client.h  30 Sep 2003 20:17:06 -0000  1.47
+++ client.h  31 Mar 2004 18:42:14 -0000
@@ -83,6 +83,10 @@
 void
 start_server (void);

+/* Calculate and send max-dotdot to the server */
+void
+send_max_dotdot(int argc, char **argv);
+
 /* Send the names of all the argument files to the server.  */
 void
 send_file_names (int argc, char **argv, unsigned int flags);


[D:\temp\ccvs\src]cvs diff -u -r cvs1-12-6 client.c
Index: client.c
===================================================================
RCS file: /cvs/ccvs/src/client.c,v
retrieving revision 1.356
diff -u -r1.356 client.c
--- client.c  25 Feb 2004 07:21:54 -0000  1.356
+++ client.c  31 Mar 2004 18:42:40 -0000
@@ -4211,6 +4211,45 @@
     free (copy);
 }

+/* Calculate and send max-dotdot to the server */
+void
+send_max_dotdot(int argc, char **argv)
+{
+    int i;
+ int level = 0;
+    int max_level = 0;
+
+    /* Send Max-dotdot if needed.  */
+    for (i = 0; i < argc; ++i)
+    {
+   level = pathname_levels (argv[i]);
+   if (level > max_level)
+     max_level = level;
+    }
+
+    if (max_level > 0)
+    {
+   if (supported_request ("Max-dotdot"))
+   {
+     char buf[10];
+     sprintf (buf, "%d", max_level);
+
+     send_to_server ("Max-dotdot ", 0);
+     send_to_server (buf, 0);
+     send_to_server ("\012", 1);
+   }
+   else
+   {
+     /*
+      * "leading .." is not strictly correct, as this also includes
+      * cases like "foo/../..".  But trying to explain that in the
+      * error message would probably just confuse users.
+      */
+     error (1, 0,
+        "leading .. not supported by old (pre-Max-dotdot) servers");
+   }
+    }
+} /* end calculate */

 /* Send the names of all the argument files to the server.  */

@@ -4218,50 +4257,20 @@
 send_file_names( int argc, char **argv, unsigned int flags )
 {
     int i;
-    int level;
-    int max_level;

     /* The fact that we do this here as well as start_recursion is a bit
        of a performance hit.  Perhaps worth cleaning up someday.  */
     if (flags & SEND_EXPAND_WILD)
  expand_wild (argc, argv, &argc, &argv);

-    /* Send Max-dotdot if needed.  */
-    max_level = 0;
-    for (i = 0; i < argc; ++i)
-    {
- level = pathname_levels (argv[i]);
- if (level > max_level)
-     max_level = level;
-    }
-    if (max_level > 0)
-    {
- if (supported_request ("Max-dotdot"))
- {
-            char buf[10];
-            sprintf (buf, "%d", max_level);
-
-     send_to_server ("Max-dotdot ", 0);
-     send_to_server (buf, 0);
-     send_to_server ("\012", 1);
- }
- else
-     /*
-      * "leading .." is not strictly correct, as this also includes
-      * cases like "foo/../..".  But trying to explain that in the
-      * error message would probably just confuse users.
-      */
-     error (1, 0,
-      "leading .. not supported by old (pre-Max-dotdot) servers");
-    }
+ send_max_dotdot(argc, argv);

     for (i = 0; i < argc; ++i)
     {
  char buf[1];
  char *p;
 #ifdef FILENAMES_CASE_INSENSITIVE
@@ -4408,8 +4417,14 @@
 {
     struct send_data args;
     int err;
+
+ /* send max-dotdot here too (also in send_file_names) to avoid relative
+  * path issues, like those described in issue 81
+  * http://ccvs.cvshome.org/issues/show_bug.cgi?id=81
+  */
+ send_max_dotdot(argc, argv);


[D:\temp\ccvs\src]cvs diff -u -r cvs1-12-6 cvs.h
Index: cvs.h
===================================================================
RCS file: /cvs/ccvs/src/cvs.h,v
retrieving revision 1.282
diff -u -r1.282 cvs.h
--- cvs.h 20 Feb 2004 23:16:54 -0000  1.282
+++ cvs.h 31 Mar 2004 18:42:00 -0000
@@ -463,6 +463,7 @@
 char *xstrdup (const char *str)
  __attribute__ ((__malloc__));
 int strip_trailing_newlines (char *str);
+int path_to_UNIX(char *path);
 int pathname_levels (char *path);

 typedef  int (*CALLPROC) (char *_repository, char *_value, void *_closure);


[D:\temp\ccvs\src]cvs diff -u -r cvs1-12-6 subr.c
Index: subr.c
===================================================================
RCS file: /cvs/ccvs/src/subr.c,v
retrieving revision 1.103
diff -u -r1.103 subr.c
--- subr.c  9 Mar 2004 17:12:08 -0000 1.103
+++ subr.c  31 Mar 2004 18:43:16 -0000
@@ -69,7 +71,53 @@
     return index != origlen;
 }

+/* Convert a path to UNIX style, replacing back
+   slashes with forward slashes when appropriate. */
+int
+path_to_UNIX(char *path)
+{
+ char *p;
+ char *dup;
+ int len;
+ int slash;
+ int i;
+
+ if (NULL == path) return 0;
+ if (0 == path[0]) return 0;
+
+ len=strlen(path);
+
+ dup=path;
+
+ p=dup;
+ slash=0;
+ for(i=0;i<len;i++)
+ {

+   if ((path[i] != '/') && (path[i] != '\\') )
+   {
+     *p++=path[i];
+     slash=0;
+   }
+
+   /* convert \ to / except when there are multiple / */
+   /* except allow starting // or \\ */
+   if ( ((path[i]=='/') || (path[i]=='\\')) && (!slash || (i==1)))
+   {
+     *p++='/';
+     slash=1;
+   }
+
+ }
+
+ /* null terminate the string, removing trailing slash action */
+ if (p[-1] == '/')
+   p[-1]=0;
+ else
+   *p=0;
+
+ return 0;
+} /* end path_to_UNIX */

 /* Return the number of levels that path ascends above where it starts.
    For example:
@@ -88,6 +136,12 @@
     int level;
     int max_level;

+ /* Convert the path to unix style first (\ --> /) so the function
+  * works under Windows also.  This is another temp fix before the
+  * better solution described in FIXME is implemented.
+  */
+ path_to_UNIX(path);
+
     max_level = 0;
     p = path;
     level = 0;
@@ -109,6 +163,7 @@
      ++level;
  p = q;
     } while (p != NULL);
+
     return max_level;
 }



[D:\temp\ccvs\src]cvs diff -u -r cvs1-12-6 sanity.sh
Index: sanity.sh
===================================================================
RCS file: /cvs/ccvs/src/sanity.sh,v
retrieving revision 1.891
diff -u -r1.891 sanity.sh
--- sanity.sh 4 Mar 2004 03:07:32 -0000 1.891
+++ sanity.sh 31 Mar 2004 18:43:48 -0000
@@ -4038,14 +4013,17 @@
    fi
    dotest files-13 \
 "${testcvs} commit -fmtest ./sdir/../sdir/ssdir/..///ssdir/.file" \
-"Checking in \./sdir/\.\./sdir/ssdir/\.\.///ssdir/\.file;
+"Checking in \./sdir/\.\./sdir/ssdir/\.\./ssdir/\.file;
 ${CVSROOT_DIRNAME}/first-dir/dir/sdir/ssdir/Attic/\.file,v  <--  \.file
 new revision: 1\.1\.2\.4; previous revision: 1\.1\.2\.3
 done"
    if $remote; then
-     dotest_fail files-14 \
+     dotest files-14 \
 "${testcvs} commit -fmtest ../../first-dir/dir/.file" \
-"protocol error: .\.\./\.\./first-dir/dir' has too many \.\."
+"Checking in \.\./\.\./first-dir/dir/\.file;
+/zip/tmp/cbohn/tmpcvs/cvsroot/first-dir/dir/Attic/\.file,v  <--  .file
+new revision: 1\.1\.2\.4; previous revision: 1\.1\.2\.3
+done"
    else
      dotest files-14 \
 "${testcvs} commit -fmtest ../../first-dir/dir/.file" \
@@ -4248,7 +4226,58 @@
      exit 0
    fi

-   cd ../..
+   cd ..
+   mkdir fourth-dir
+   dotest status-init-8 "${testcvs} add fourth-dir" \
+"Directory ${CVSROOT_DIRNAME}/fourth-dir added to the repository"
+   cd fourth-dir
+   echo yet another line >t3file
+   dotest status-init-9 "${testcvs} add t3file" \
+"${SPROG} add: scheduling file .t3file. for addition
+${SPROG} add: use .${SPROG} commit. to add this file permanently"
+   dotest status-init-10 "${testcvs} -q ci -m add" \
+"RCS file: ${CVSROOT_DIRNAME}/fourth-dir/t3file,v
+done
+Checking in t3file;
+${CVSROOT_DIRNAME}/fourth-dir/t3file,v  <--  t3file
+initial revision: 1\.1
+done"
+   cd ../first-dir
+   mkdir third-dir
+   dotest status-init-11 "${testcvs} add third-dir" \
+"Directory ${CVSROOT_DIRNAME}/first-dir/third-dir added to the repository"
+   cd third-dir
+   echo another line >t2file
+   dotest status-init-12 "${testcvs} add t2file" \
+"${SPROG} add: scheduling file .t2file. for addition
+${SPROG} add: use .${SPROG} commit. to add this file permanently"
+   dotest status-init-13 "${testcvs} -q ci -m add" \
+"RCS file: ${CVSROOT_DIRNAME}/first-dir/third-dir/t2file,v
+done
+Checking in t2file;
+${CVSROOT_DIRNAME}/first-dir/third-dir/t2file,v  <--  t2file
+initial revision: 1\.1
+done"
+   dotest status-5 "${testcvs} status ../tfile" \
+"===================================================================
+File: tfile              Status: Locally Modified
+
+   Working revision: 1\.2.*
+   Repository revision:  1\.2  ${CVSROOT_DIRNAME}/first-dir/tfile,v
+   Sticky Tag:   (none)
+   Sticky Date:    (none)
+   Sticky Options: (none)"
+   dotest status-6 "${testcvs} status ../../fourth-dir/t3file" \
+"===================================================================
+File: t3file             Status: Up-to-date
+
+   Working revision: 1\.1.*
+   Repository revision:  1\.1  ${CVSROOT_DIRNAME}/fourth-dir/t3file,v
+   Sticky Tag:   (none)
+   Sticky Date:    (none)
+   Sticky Options: (none)"
+
+   cd ../../..
    rm -rf status
    rm -rf ${CVSROOT_DIRNAME}/first-dir
    ;;






reply via email to

[Prev in Thread] Current Thread [Next in Thread]