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

[Chris Bohn]

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 primarily specific to Windows).

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

Log entry:
2004-03-30        Chris Bohn  <cbohn@rrinc.com>

  * 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.

Note that my diffs may show different line numbers from the diffed revision.
That is usually because I had other code changes also (causing lines to be
offset), but I'm only including the relevant pieces for the fixes specified 
above.  This is especially true of sanity.sh because I had a lot of issues 
running it under FreeBSD so I had a lot of changes for debugging and to get it 
to work (some are documented in issue 171).

[D:\temp\ccvs\src]cvs diff -r cvs1-12-6 client.h
Index: client.h
===================================================================
RCS file: /cvs/ccvs/src/client.h,v
retrieving revision 1.47
diff -r1.47 client.h
85a86,89
> /* Calculate and send max-dotdot to the server */
> void
> send_max_dotdot(int argc, char **argv);
>


[D:\temp\ccvs\src]cvs diff -r cvs1-12-6 client.c
Index: client.c
===================================================================
RCS file: /cvs/ccvs/src/client.c,v
retrieving revision 1.356
diff -r1.356 client.c
4213a4214,4252
> /* 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 send_max_dotdot */
4221,4222d4259
<     int level;
<     int max_level;
4229,4256c4266
<     /* 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);
4263,4264c4273
<   char *line = xmalloc (1);
<   *line = '\0';
---
>   char *line = NULL;
4410a4420,4425
>
>   /* 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);
4412c4427
<     /*
---
>   /*


[D:\temp\ccvs\src]cvs diff -r cvs1-12-6 cvs.h
Index: cvs.h
===================================================================
RCS file: /cvs/ccvs/src/cvs.h,v
retrieving revision 1.282
diff -r1.282 cvs.h
465a466
> int path_to_UNIX(char *path);


[D:\temp\ccvs\src]cvs diff -r cvs1-12-6 subr.c
Index: subr.c
===================================================================
RCS file: /cvs/ccvs/src/subr.c,v
retrieving revision 1.103
diff -r1.103 subr.c
71a74,95
> /* 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++)
>   {
72a97,120
>     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 */
90a139,144
>   /* 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);
>


[D:\temp\ccvs\src]cvs diff -r cvs1-12-6 sanity.sh
4041c4016
< "Checking in \./sdir/\.\./sdir/ssdir/\.\.///ssdir/\.file;
---
> "Checking in \./sdir/\.\./sdir/ssdir/\.\./ssdir/\.file;
4046c4021
<       dotest_fail files-14 \
---
>       dotest files-14 \
4048c4023,4026
< "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"
4251c4229,4280
<     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 ../../..