On Thu, 18 May 2006, Junio C Hamano wrote:
>
> Ouch, things are worse than I thought...
>
> $ mkdir foo
> $ date >bar
> $ git-add foo/../bar
> $ git ls-files
> foo/../bar
>
> Huh?
"git-update-index" does a "verify_path()" which I missed, so the new
builtin add doesn't do it.
We could do it either in the "add_cache_entry()" function (which would be
safest, because then by _definition_ you couldn't make this mistake
again), or we could do it in the caller.
This patch does it in the caller, just to match the old "git add" (which
would just say "Ignoring path ..." and not do anything).
But if people are ok with changing it from a "print a warning and ignore"
into an _error_, we could just move it into "add_cache_entry()".
The real _meat_ of this patch is really just that first hunk to
"builtin-add.c" - all the rest is just making that "verify_path()"
function available in the git library.
Linus
---
diff --git a/builtin-add.c b/builtin-add.c
index 7083820..0346bb5 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -172,6 +172,11 @@ static int add_file_to_index(const char
if (lstat(path, &st))
die("%s: unable to stat (%s)", path, strerror(errno));
+ if (!verify_path(path)) {
+ fprintf(stderr, "Ignoring path %s\n", path);
+ return -1;
+ }
+
if (!S_ISREG(st.st_mode) && !S_ISLNK(st.st_mode))
die("%s: can only add regular files or symbolic links", path);
diff --git a/cache.h b/cache.h
index b1300cd..f9b7049 100644
--- a/cache.h
+++ b/cache.h
@@ -143,6 +143,7 @@ #define alloc_nr(x) (((x)+16)*3/2)
/* Initialize and use the cache information */
extern int read_cache(void);
extern int write_cache(int newfd, struct cache_entry **cache, int entries);
+extern int verify_path(const char *path);
extern int cache_name_pos(const char *name, int namelen);
#define ADD_CACHE_OK_TO_ADD 1 /* Ok to add */
#define ADD_CACHE_OK_TO_REPLACE 2 /* Ok to replace file/directory */
diff --git a/read-cache.c b/read-cache.c
index ed0da38..6b323dd 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -347,6 +347,70 @@ int ce_path_match(const struct cache_ent
}
/*
+ * We fundamentally don't like some paths: we don't want
+ * dot or dot-dot anywhere, and for obvious reasons don't
+ * want to recurse into ".git" either.
+ *
+ * Also, we don't want double slashes or slashes at the
+ * end that can make pathnames ambiguous.
+ */
+static int verify_dotfile(const char *rest)
+{
+ /*
+ * The first character was '.', but that
+ * has already been discarded, we now test
+ * the rest.
+ */
+ switch (*rest) {
+ /* "." is not allowed */
+ case '\0': case '/':
+ return 0;
+
+ /*
+ * ".git" followed by NUL or slash is bad. This
+ * shares the path end test with the ".." case.
+ */
+ case 'g':
+ if (rest[1] != 'i')
+ break;
+ if (rest[2] != 't')
+ break;
+ rest += 2;
+ /* fallthrough */
+ case '.':
+ if (rest[1] == '\0' || rest[1] == '/')
+ return 0;
+ }
+ return 1;
+}
+
+int verify_path(const char *path)
+{
+ char c;
+
+ goto inside;
+ for (;;) {
+ if (!c)
+ return 1;
+ if (c == '/') {
+inside:
+ c = *path++;
+ switch (c) {
+ default:
+ continue;
+ case '/': case '\0':
+ break;
+ case '.':
+ if (verify_dotfile(path))
+ continue;
+ }
+ return 0;
+ }
+ c = *path++;
+ }
+}
+
+/*
* Do we have another file that has the beginning components being a
* proper superset of the name we're trying to add?
*/
diff --git a/update-index.c b/update-index.c
index f6b09a4..69b9a71 100644
--- a/update-index.c
+++ b/update-index.c
@@ -8,6 +8,7 @@ #include "strbuf.h"
#include "quote.h"
#include "cache-tree.h"
#include "tree-walk.h"
+#include "dir.h"
/*
* Default to not allowing changes to the list of files. The
@@ -245,70 +246,6 @@ static int refresh_cache(int really)
return has_errors;
}
-/*
- * We fundamentally don't like some paths: we don't want
- * dot or dot-dot anywhere, and for obvious reasons don't
- * want to recurse into ".git" either.
- *
- * Also, we don't want double slashes or slashes at the
- * end that can make pathnames ambiguous.
- */
-static int verify_dotfile(const char *rest)
-{
- /*
- * The first character was '.', but that
- * has already been discarded, we now test
- * the rest.
- */
- switch (*rest) {
- /* "." is not allowed */
- case '\0': case '/':
- return 0;
-
- /*
- * ".git" followed by NUL or slash is bad. This
- * shares the path end test with the ".." case.
- */
- case 'g':
- if (rest[1] != 'i')
- break;
- if (rest[2] != 't')
- break;
- rest += 2;
- /* fallthrough */
- case '.':
- if (rest[1] == '\0' || rest[1] == '/')
- return 0;
- }
- return 1;
-}
-
-static int verify_path(const char *path)
-{
- char c;
-
- goto inside;
- for (;;) {
- if (!c)
- return 1;
- if (c == '/') {
-inside:
- c = *path++;
- switch (c) {
- default:
- continue;
- case '/': case '\0':
- break;
- case '.':
- if (verify_dotfile(path))
- continue;
- }
- return 0;
- }
- c = *path++;
- }
-}
-
static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,
const char *path, int stage)
{
-
: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html