{"thread":{"id":"32221","subject":"[PATCH] fsck: warn about '.' and '..' in trees","startedAt":"2012-11-28T02:27:37Z","lastAt":"2012-11-28T04:34:33Z","messageCount":4,"participants":["Jeff King","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"204105","messageId":"20121128022736.GA3739@sigill.intra.peff.net","threadId":"32221","inReplyTo":null,"subject":"[PATCH] fsck: warn about '.' and '..' in trees","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-28T02:27:37Z","receivedAt":"2012-11-28T02:27:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"A tree with meta-paths like '.' or '..' does not work well\nwith git; the index will refuse to load it or check it out\nto the filesystem (and even if we did not have that safety,\nit would look like we were overwriting an untracked\ndirectory). For the same reason, it is difficult to create\nsuch a tree with regular git.\n\nLet's warn about these dubious entries during fsck, just in\ncase somebody has created a bogus tree (and this also lets\nus prevent them from propagating when transfer.fsckObjects\nis set).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI don't think this is happening in the wild, but I did see somebody\nplaying around with libgit2 make such a tree (and it is easy to do with\ngit-mktree, of course).\n\nTechnically one could use git with such a tree as long as you never ever\nchecked out the result, but I think it is sufficiently crazy that we\nshould probably detect it, just in case.\n\nShould we also detect \".git\" in the same way? It is similarly treated\nspecially by verify_path(), and its presence is likely to cause weird\nerrors.\n\n fsck.c          | 10 ++++++++++\n t/t1450-fsck.sh | 16 ++++++++++++++++\n 2 files changed, 26 insertions(+)\n\ndiff --git a/fsck.c b/fsck.c\nindex 7395ef6..31c9a51 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -142,6 +142,8 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)\n \tint has_null_sha1 = 0;\n \tint has_full_path = 0;\n \tint has_empty_name = 0;\n+\tint has_dot = 0;\n+\tint has_dotdot = 0;\n \tint has_zero_pad = 0;\n \tint has_bad_modes = 0;\n \tint has_dup_entries = 0;\n@@ -168,6 +170,10 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)\n \t\t\thas_full_path = 1;\n \t\tif (!*name)\n \t\t\thas_empty_name = 1;\n+\t\tif (!strcmp(name, \".\"))\n+\t\t\thas_dot = 1;\n+\t\tif (!strcmp(name, \"..\"))\n+\t\t\thas_dotdot = 1;\n \t\thas_zero_pad |= *(char *)desc.buffer == '0';\n \t\tupdate_tree_entry(&desc);\n \n@@ -217,6 +223,10 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)\n \t\tretval += error_func(&item->object, FSCK_WARN, \"contains full pathnames\");\n \tif (has_empty_name)\n \t\tretval += error_func(&item->object, FSCK_WARN, \"contains empty pathname\");\n+\tif (has_dot)\n+\t\tretval += error_func(&item->object, FSCK_WARN, \"contains '.'\");\n+\tif (has_dotdot)\n+\t\tretval += error_func(&item->object, FSCK_WARN, \"contains '..'\");\n \tif (has_zero_pad)\n \t\tretval += error_func(&item->object, FSCK_WARN, \"contains zero-padded file modes\");\n \tif (has_bad_modes)\ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex 08aa24c..0b5c30b 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -237,4 +237,20 @@ test_expect_success 'fsck notices submodule entry pointing to null sha1' '\n \t)\n '\n \n+test_expect_success 'fsck notices \".\" and \"..\" in trees' '\n+\t(\n+\t\tgit init dots &&\n+\t\tcd dots &&\n+\t\tblob=$(echo foo | git hash-object -w --stdin) &&\n+\t\ttab=$(printf \"\\\\t\") &&\n+\t\tgit mktree <<-EOF &&\n+\t\t100644 blob $blob$tab.\n+\t\t100644 blob $blob$tab..\n+\t\tEOF\n+\t\tgit fsck 2>out &&\n+\t\tcat out &&\n+\t\tgrep \"warning.*\\\\.\" out\n+\t)\n+'\n+\n test_done\n-- \n1.8.0.207.gdf2154c\n"},{"id":"204127","messageId":"CACsJy8DQCo9UzDadHJ2dF-eK20tFDTVn_ScwV+T7z-qLDJMytw@mail.gmail.com","threadId":"32221","inReplyTo":"20121128022736.GA3739@sigill.intra.peff.net","subject":"Re: [PATCH] fsck: warn about '.' and '..' in trees","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-11-28T04:22:20Z","receivedAt":"2012-11-28T04:22:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Nov 28, 2012 at 9:27 AM, Jeff King <peff@peff.net> wrote:\n> A tree with meta-paths like '.' or '..' does not work well\n> with git; the index will refuse to load it or check it out\n> to the filesystem (and even if we did not have that safety,\n> it would look like we were overwriting an untracked\n> directory). For the same reason, it is difficult to create\n> such a tree with regular git.\n>\n> Let's warn about these dubious entries during fsck, just in\n> case somebody has created a bogus tree (and this also lets\n> us prevent them from propagating when transfer.fsckObjects\n> is set).\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> I don't think this is happening in the wild, but I did see somebody\n> playing around with libgit2 make such a tree (and it is easy to do with\n> git-mktree, of course).\n>\n> Technically one could use git with such a tree as long as you never ever\n> checked out the result, but I think it is sufficiently crazy that we\n> should probably detect it, just in case.\n\nCan we declare \".\" and \"..\" illegal? There's no room for extension in\ntree objects and I'm thinking of using maybe \".\" entry as an extension\nindicator. Not sure if it works, old gits may attempt to checkout \".\"\nentries and fail...\n-- \nDuy\n"},{"id":"204129","messageId":"20121128043215.GA5362@sigill.intra.peff.net","threadId":"32221","inReplyTo":"CACsJy8DQCo9UzDadHJ2dF-eK20tFDTVn_ScwV+T7z-qLDJMytw@mail.gmail.com","subject":"Re: [PATCH] fsck: warn about '.' and '..' in trees","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-28T04:32:16Z","receivedAt":"2012-11-28T04:32:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 28, 2012 at 11:22:20AM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> > I don't think this is happening in the wild, but I did see somebody\n> > playing around with libgit2 make such a tree (and it is easy to do with\n> > git-mktree, of course).\n> >\n> > Technically one could use git with such a tree as long as you never ever\n> > checked out the result, but I think it is sufficiently crazy that we\n> > should probably detect it, just in case.\n> \n> Can we declare \".\" and \"..\" illegal? There's no room for extension in\n> tree objects and I'm thinking of using maybe \".\" entry as an extension\n> indicator. Not sure if it works, old gits may attempt to checkout \".\"\n> entries and fail...\n\nYeah, current git fails pretty hard. Try this:\n\n  check() {\n    git init -q \"$1\" &&\n    (cd \"$1\" &&\n     blob=$(echo foo | git hash-object -w --stdin) &&\n     tree=$(printf '100644 blob %s\\t%s' $blob \"$2\" | git mktree) &&\n     commit=$(echo foo | git commit-tree $tree) &&\n     git update-ref HEAD $commit &&\n     git clone -q . clone\n    )\n  }\n\n  $ check dot .\n  error: Invalid path '.'\n\n  $ check dotdot ..\n  error: Updating '..' would lose untracked files in it\n\n  $ check dotgit .git\n  error: Updating '.git' would lose untracked files in it\n\nInteresting that we detect the first one while reading into the cache,\nbut apparently try much harder to checkout on the latter two. Not sure I\nwant to try \"git checkout -f\". :)\n\n-Peff\n"},{"id":"204130","messageId":"20121128043433.GB5362@sigill.intra.peff.net","threadId":"32221","inReplyTo":"20121128043215.GA5362@sigill.intra.peff.net","subject":"Re: [PATCH] fsck: warn about '.' and '..' in trees","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-11-28T04:34:33Z","receivedAt":"2012-11-28T04:34:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 27, 2012 at 11:32:16PM -0500, Jeff King wrote:\n\n>   $ check dot .\n>   error: Invalid path '.'\n> \n>   $ check dotdot ..\n>   error: Updating '..' would lose untracked files in it\n> \n>   $ check dotgit .git\n>   error: Updating '.git' would lose untracked files in it\n> \n> Interesting that we detect the first one while reading into the cache,\n> but apparently try much harder to checkout on the latter two. Not sure I\n> want to try \"git checkout -f\". :)\n\nActually, I take it back. The \"untracked files\" check comes _first_, and\nif you do \"-f\", we end up in the verify_path check when trying to pull\nthe tree into the index. So I think the way git handles it makes sense.\n\n-Peff\n"}]}