Re: Bug in canonicalize_path()

Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > I figured it would be best to leave it alone if we can't process it, but
> > if you think it is more imporant to trim in cases like ../.., go ahead.
>
> My recollection of this patch:
>
> 2004-08-09 16:20  tgl
>
>     * src/: port/exec.c, port/path.c, bin/initdb/initdb.c:
>     Path-mangling logic was failing to account for paths containing
>     mentions of '.' or '..'.  Extend canonicalize_path() to trim off
>     trailing occurrences of these things, and use it to fix up paths
>     where needed (which I think is only after places where we trim the
>     last path component, but maybe some others will turn up).  Fixes
>     Josh's complaint that './initdb' does not work.
>
> is that it's part of the API contract of canonicalize_path() that it
> will not return something with trailing "." or "..".  It's not just
> neatnik-ism to strip those, it's necessary to allow higher-level
> path-mangling routines to reason about how to do what they need.
> The entire concept of "trim the last directory" fails if you have
> to account for "." or "..".

OK, new patch which I think handles all cases.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: src/port/path.c
===================================================================
RCS file: /cvsroot/pgsql/src/port/path.c,v
retrieving revision 1.54
diff -c -c -r1.54 path.c
*** src/port/path.c    12 Aug 2005 03:07:45 -0000    1.54
--- src/port/path.c    12 Aug 2005 04:47:59 -0000
***************
*** 226,231 ****
--- 226,232 ----
  {
      char       *p, *to_p;
      bool        was_sep = false;
+     int            pending_strips = 0;

  #ifdef WIN32
      /*
***************
*** 284,306 ****

          if (len > 2 && strcmp(path + len - 2, "/.") == 0)
              trim_directory(path);
!         /*
!          *    Process only a single trailing "..", and only if ".." does
!          *    not preceed it.
!          *    So, we only deal with "/usr/local/..", not with "/usr/local/../..".
!          *    We don't handle the even more complex cases, like
!          *    "usr/local/../../..".
!          */
!         else if (len > 3 && strcmp(path + len - 3, "/..") == 0 &&
!                  (len != 5 || strcmp(path, "../..") != 0) &&
!                  (len < 6 || strcmp(path + len - 6, "/../..") != 0))
          {
              trim_directory(path);
!             trim_directory(path);    /* remove directory above */
          }
          else
              break;
      }
  }


--- 285,311 ----

          if (len > 2 && strcmp(path + len - 2, "/.") == 0)
              trim_directory(path);
!         else if (len > 3 && strcmp(path + len - 3, "/..") == 0)
          {
              trim_directory(path);
!             pending_strips++;
!         }
!         /* for absolute paths, we just keep trimming */
!         else if (*path != '\0' && pending_strips > 0)
!         {
!             trim_directory(path);
!             pending_strips--;
          }
          else
              break;
      }
+
+     if (pending_strips > 0)
+     {
+         for (; pending_strips > 0; pending_strips--)
+             strcat(path, "../");
+         trim_trailing_separator(path);
+     }
  }