link_alt_odb_entry: handle normalize_path errors

When we add a new alternate to the list, we try to normalize
out any redundant "..", etc. However, we do not look at the
return value of normalize_path_copy(), and will happily
continue with a path that could not be normalized. Worse,
the normalizing process is done in-place, so we are left
with whatever half-finished working state the normalizing
function was in.

Fortunately, this cannot cause us to read past the end of
our buffer, as that working state will always leave the
NUL from the original path in place. And we do tend to
notice problems when we check is_directory() on the path.
But you can see the nonsense that we feed to is_directory
with an entry like:

this/../../is/../../way/../../too/../../deep/../../to/../../resolve

in your objects/info/alternates, which yields:

error: object directory
/to/e/deep/too/way//ects/this/../../is/../../way/../../too/../../deep/../../to/../../resolve
does not exist; check .git/objects/info/alternates.

We can easily fix this just by checking the return value.
But that makes it hard to generate a good error message,
since we're normalizing in-place and our input value has
been overwritten by cruft.

Instead, let's provide a strbuf helper that does an in-place
normalize, but restores the original contents on error. This
uses a second buffer under the hood, which is slightly less
efficient, but this is not a performance-critical code path.

The strbuf helper can also properly set the "len" parameter
of the strbuf before returning. Just doing:

normalize_path_copy(buf.buf, buf.buf);

will shorten the string, but leave buf.len at the original
length. That may be confusing to later code which uses the
strbuf.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sha1_file.c b/sha1_file.c
index 94daf31ec6b73f99c4e07ed3b6f9235c2f1baec8..057262d46ef31f4e15347442c853a164fce29889 100644 (file)
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -263,7 +263,12 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
        }
        strbuf_addstr(&pathbuf, entry);
 
        }
        strbuf_addstr(&pathbuf, entry);
 

-       normalize_path_copy(pathbuf.buf, pathbuf.buf);
+       if (strbuf_normalize_path(&pathbuf) < 0) {
+               error("unable to normalize alternate object path: %s",
+                     pathbuf.buf);
+               strbuf_release(&pathbuf);
+               return -1;
+       }

 
        pfxlen = strlen(pathbuf.buf);
 
 
        pfxlen = strlen(pathbuf.buf);
 


@@ -335,7 +340,9 @@ static void link_alt_odb_entries(const char *alt, int len, int sep,
        }
 
        strbuf_add_absolute_path(&objdirbuf, get_object_directory());
        }
 
        strbuf_add_absolute_path(&objdirbuf, get_object_directory());

-       normalize_path_copy(objdirbuf.buf, objdirbuf.buf);
+       if (strbuf_normalize_path(&objdirbuf) < 0)
+               die("unable to normalize object directory: %s",
+                   objdirbuf.buf);

 
        alt_copy = xmemdupz(alt, len);
        string_list_split_in_place(&entries, alt_copy, sep, -1);
 
        alt_copy = xmemdupz(alt, len);
        string_list_split_in_place(&entries, alt_copy, sep, -1);

diff --git a/strbuf.c b/strbuf.c
index b839be491b74a034cf848d2dd043aba7d75b7b92..8fec6579f70cae4bc6b330e4e0ca3e7cc595f366 100644 (file)
--- a/strbuf.c
+++ b/strbuf.c
@@ -870,3 +870,23 @@ void strbuf_stripspace(struct strbuf *sb, int skip_comments)
 
        strbuf_setlen(sb, j);
 }
 
        strbuf_setlen(sb, j);
 }

+
+int strbuf_normalize_path(struct strbuf *src)
+{
+       struct strbuf dst = STRBUF_INIT;
+
+       strbuf_grow(&dst, src->len);
+       if (normalize_path_copy(dst.buf, src->buf) < 0) {
+               strbuf_release(&dst);
+               return -1;
+       }
+
+       /*
+        * normalize_path does not tell us the new length, so we have to
+        * compute it by looking for the new NUL it placed
+        */
+       strbuf_setlen(&dst, strlen(dst.buf));
+       strbuf_swap(src, &dst);
+       strbuf_release(&dst);
+       return 0;
+}

diff --git a/strbuf.h b/strbuf.h
index ba8d5f1d465e5b06274028aeb096e325e73e5de4..2262b12683b5d91d536baa0b71094098872200d1 100644 (file)
--- a/strbuf.h
+++ b/strbuf.h
@@ -443,6 +443,14 @@ extern int strbuf_getcwd(struct strbuf *sb);
  */
 extern void strbuf_add_absolute_path(struct strbuf *sb, const char *path);
 
  */
 extern void strbuf_add_absolute_path(struct strbuf *sb, const char *path);
 

+
+/**
+ * Normalize in-place the path contained in the strbuf. See
+ * normalize_path_copy() for details. If an error occurs, the contents of "sb"
+ * are left untouched, and -1 is returned.
+ */
+extern int strbuf_normalize_path(struct strbuf *sb);
+

 /**
  * Strip whitespace from a buffer. The second parameter controls if
  * comments are considered contents to be removed or not.
 /**
  * Strip whitespace from a buffer. The second parameter controls if
  * comments are considered contents to be removed or not.