{"thread":{"id":"28025","subject":"[PATCH] fast-export: quote paths in output","startedAt":"2011-08-05T10:55:27Z","lastAt":"2011-08-06T02:36:04Z","messageCount":5,"participants":["Jeff King","Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"172967","messageId":"20110805105526.GA22480@sigill.intra.peff.net","threadId":"28025","inReplyTo":null,"subject":"[PATCH] fast-export: quote paths in output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-08-05T10:55:27Z","receivedAt":"2011-08-05T10:55:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Many pathnames in a fast-import stream need to be quoted. In\nparticular:\n\n  1. Pathnames at the end of an \"M\" or \"D\" line need quoting\n     if they contain a LF or start with double-quote.\n\n  2. Pathnames on a \"C\" or \"R\" line need quoting as above,\n     but also if they contain spaces.\n\nFor (1), we weren't quoting at all. For (2), we put\ndouble-quotes around the paths to handle spaces, but ignored\nthe possibility that they would need further quoting.\n\nThis patch checks whether each pathname needs c-style\nquoting, and uses it. This is slightly overkill for (1),\nwhich doesn't actually need to quote many characters that\nvanilla c-style quoting does. However, it shouldn't hurt, as\nany implementation needs to be ready to handle quoted\nstrings anyway.\n\nIn addition to adding a test, we have to tweak a test which\nblindly assumed that case (2) would always use\ndouble-quotes, whether it needed to or not.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/fast-export.c  |   31 ++++++++++++++++++++++++-------\n t/t9350-fast-export.sh |   26 +++++++++++++++++++++++++-\n 2 files changed, 49 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex becef85..9836e6b 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -16,6 +16,7 @@\n #include \"string-list.h\"\n #include \"utf8.h\"\n #include \"parse-options.h\"\n+#include \"quote.h\"\n \n static const char *fast_export_usage[] = {\n \t\"git fast-export [rev-list-opts]\",\n@@ -179,6 +180,15 @@ static int depth_first(const void *a_, const void *b_)\n \treturn (a->status == 'R') - (b->status == 'R');\n }\n \n+static void print_path(const char *path)\n+{\n+\tint need_quote = quote_c_style(path, NULL, NULL, 0);\n+\tif (need_quote)\n+\t\tquote_c_style(path, NULL, stdout, 0);\n+\telse\n+\t\tprintf(\"%s\", path);\n+}\n+\n static void show_filemodify(struct diff_queue_struct *q,\n \t\t\t    struct diff_options *options, void *data)\n {\n@@ -196,13 +206,18 @@ static void show_filemodify(struct diff_queue_struct *q,\n \n \t\tswitch (q->queue[i]->status) {\n \t\tcase DIFF_STATUS_DELETED:\n-\t\t\tprintf(\"D %s\\n\", spec->path);\n+\t\t\tprintf(\"D \");\n+\t\t\tprint_path(spec->path);\n+\t\t\tputchar('\\n');\n \t\t\tbreak;\n \n \t\tcase DIFF_STATUS_COPIED:\n \t\tcase DIFF_STATUS_RENAMED:\n-\t\t\tprintf(\"%c \\\"%s\\\" \\\"%s\\\"\\n\", q->queue[i]->status,\n-\t\t\t       ospec->path, spec->path);\n+\t\t\tprintf(\"%c \", q->queue[i]->status);\n+\t\t\tprint_path(ospec->path);\n+\t\t\tputchar(' ');\n+\t\t\tprint_path(spec->path);\n+\t\t\tputchar('\\n');\n \n \t\t\tif (!hashcmp(ospec->sha1, spec->sha1) &&\n \t\t\t    ospec->mode == spec->mode)\n@@ -217,13 +232,15 @@ static void show_filemodify(struct diff_queue_struct *q,\n \t\t\t * output the SHA-1 verbatim.\n \t\t\t */\n \t\t\tif (no_data || S_ISGITLINK(spec->mode))\n-\t\t\t\tprintf(\"M %06o %s %s\\n\", spec->mode,\n-\t\t\t\t       sha1_to_hex(spec->sha1), spec->path);\n+\t\t\t\tprintf(\"M %06o %s \", spec->mode,\n+\t\t\t\t       sha1_to_hex(spec->sha1));\n \t\t\telse {\n \t\t\t\tstruct object *object = lookup_object(spec->sha1);\n-\t\t\t\tprintf(\"M %06o :%d %s\\n\", spec->mode,\n-\t\t\t\t       get_object_mark(object), spec->path);\n+\t\t\t\tprintf(\"M %06o :%d \", spec->mode,\n+\t\t\t\t       get_object_mark(object));\n \t\t\t}\n+\t\t\tprint_path(spec->path);\n+\t\t\tputchar('\\n');\n \t\t\tbreak;\n \n \t\tdefault:\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex f823c05..4673ac0 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -228,7 +228,7 @@ test_expect_success 'fast-export -C -C | fast-import' '\n \tmkdir new &&\n \tgit --git-dir=new/.git init &&\n \tgit fast-export -C -C --signed-tags=strip --all > output &&\n-\tgrep \"^C \\\"file6\\\" \\\"file7\\\"\\$\" output &&\n+\tgrep \"^C file6 file7\\$\" output &&\n \tcat output |\n \t(cd new &&\n \t git fast-import &&\n@@ -414,4 +414,28 @@ test_expect_success SYMLINKS 'directory becomes symlink'        '\n \t(cd result && git show master:foo)\n '\n \n+test_expect_success 'fast-export quotes pathnames' '\n+\tgit init crazy-paths &&\n+\t(cd crazy-paths &&\n+\t >\"$(printf \"path with\\\\nnewline\")\" &&\n+\t >\"path with \\\"quote\\\"\" &&\n+\t >\"path with \\\\backslash\" &&\n+\t >\"path with space\" &&\n+\t git add . &&\n+\t git commit -m addition &&\n+\t mkdir subdir &&\n+\t git mv path* subdir &&\n+\t git commit -m rename &&\n+\t git rm -r subdir &&\n+\t git commit -m deletion &&\n+\t git fast-export HEAD >export.out &&\n+\t git rev-list HEAD >expect &&\n+\t git init result &&\n+\t cd result &&\n+\t git fast-import <../export.out &&\n+\t git rev-list HEAD >actual &&\n+\t test_cmp ../expect actual\n+\t)\n+'\n+\n test_done\n-- \n1.7.6.rc0.36.gd385b.dirty\n"},{"id":"172969","messageId":"4E3BD006.6010005@viscovery.net","threadId":"28025","inReplyTo":"20110805105526.GA22480@sigill.intra.peff.net","subject":"Re: [PATCH] fast-export: quote paths in output","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-08-05T11:12:06Z","receivedAt":"2011-08-05T11:12:06Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 8/5/2011 12:55, schrieb Jeff King:\n> +test_expect_success 'fast-export quotes pathnames' '\n> +\tgit init crazy-paths &&\n> +\t(cd crazy-paths &&\n> +\t >\"$(printf \"path with\\\\nnewline\")\" &&\n> +\t >\"path with \\\"quote\\\"\" &&\n> +\t >\"path with \\\\backslash\" &&\n> +\t >\"path with space\" &&\n\nPlease construct the tree object using git-update-index's --cacheinfo or\n--index-info rather than using actual files and git add so that the test\ncan pass on filesystems that do not allow newlines, quotes, or\nbackslashes(!) in filenames.\n\n-- Hannes\n-- \n\"Atomic objects are neither active nor radioactive.\" --\nProgramming Languages -- C++, Final Committee Draft (Doc.N3092)\n"},{"id":"172996","messageId":"20110805223622.GA5808@sigill.intra.peff.net","threadId":"28025","inReplyTo":"4E3BD006.6010005@viscovery.net","subject":"Re: [PATCH] fast-export: quote paths in output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-08-05T22:36:22Z","receivedAt":"2011-08-05T22:36:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 05, 2011 at 01:12:06PM +0200, Johannes Sixt wrote:\n\n> Am 8/5/2011 12:55, schrieb Jeff King:\n> > +test_expect_success 'fast-export quotes pathnames' '\n> > +\tgit init crazy-paths &&\n> > +\t(cd crazy-paths &&\n> > +\t >\"$(printf \"path with\\\\nnewline\")\" &&\n> > +\t >\"path with \\\"quote\\\"\" &&\n> > +\t >\"path with \\\\backslash\" &&\n> > +\t >\"path with space\" &&\n> \n> Please construct the tree object using git-update-index's --cacheinfo or\n> --index-info rather than using actual files and git add so that the test\n> can pass on filesystems that do not allow newlines, quotes, or\n> backslashes(!) in filenames.\n\nSorry, I should have thought of that myself. Here's an updated version.\n\n-- >8 --\nSubject: [PATCH] fast-export: quote paths in output\n\nMany pathnames in a fast-import stream need to be quoted. In\nparticular:\n\n  1. Pathnames at the end of an \"M\" or \"D\" line need quoting\n     if they contain a LF or start with double-quote.\n\n  2. Pathnames on a \"C\" or \"R\" line need quoting as above,\n     but also if they contain spaces.\n\nFor (1), we weren't quoting at all. For (2), we put\ndouble-quotes around the paths to handle spaces, but ignored\nthe possibility that they would need further quoting.\n\nThis patch checks whether each pathname needs c-style\nquoting, and uses it. This is slightly overkill for (1),\nwhich doesn't actually need to quote many characters that\nvanilla c-style quoting does. However, it shouldn't hurt, as\nany implementation needs to be ready to handle quoted\nstrings anyway.\n\nIn addition to adding a test, we have to tweak a test which\nblindly assumed that case (2) would always use\ndouble-quotes, whether it needed to or not.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/fast-export.c  |   31 ++++++++++++++++++++++++-------\n t/t9350-fast-export.sh |   28 +++++++++++++++++++++++++++-\n 2 files changed, 51 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex becef85..9836e6b 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -16,6 +16,7 @@\n #include \"string-list.h\"\n #include \"utf8.h\"\n #include \"parse-options.h\"\n+#include \"quote.h\"\n \n static const char *fast_export_usage[] = {\n \t\"git fast-export [rev-list-opts]\",\n@@ -179,6 +180,15 @@ static int depth_first(const void *a_, const void *b_)\n \treturn (a->status == 'R') - (b->status == 'R');\n }\n \n+static void print_path(const char *path)\n+{\n+\tint need_quote = quote_c_style(path, NULL, NULL, 0);\n+\tif (need_quote)\n+\t\tquote_c_style(path, NULL, stdout, 0);\n+\telse\n+\t\tprintf(\"%s\", path);\n+}\n+\n static void show_filemodify(struct diff_queue_struct *q,\n \t\t\t    struct diff_options *options, void *data)\n {\n@@ -196,13 +206,18 @@ static void show_filemodify(struct diff_queue_struct *q,\n \n \t\tswitch (q->queue[i]->status) {\n \t\tcase DIFF_STATUS_DELETED:\n-\t\t\tprintf(\"D %s\\n\", spec->path);\n+\t\t\tprintf(\"D \");\n+\t\t\tprint_path(spec->path);\n+\t\t\tputchar('\\n');\n \t\t\tbreak;\n \n \t\tcase DIFF_STATUS_COPIED:\n \t\tcase DIFF_STATUS_RENAMED:\n-\t\t\tprintf(\"%c \\\"%s\\\" \\\"%s\\\"\\n\", q->queue[i]->status,\n-\t\t\t       ospec->path, spec->path);\n+\t\t\tprintf(\"%c \", q->queue[i]->status);\n+\t\t\tprint_path(ospec->path);\n+\t\t\tputchar(' ');\n+\t\t\tprint_path(spec->path);\n+\t\t\tputchar('\\n');\n \n \t\t\tif (!hashcmp(ospec->sha1, spec->sha1) &&\n \t\t\t    ospec->mode == spec->mode)\n@@ -217,13 +232,15 @@ static void show_filemodify(struct diff_queue_struct *q,\n \t\t\t * output the SHA-1 verbatim.\n \t\t\t */\n \t\t\tif (no_data || S_ISGITLINK(spec->mode))\n-\t\t\t\tprintf(\"M %06o %s %s\\n\", spec->mode,\n-\t\t\t\t       sha1_to_hex(spec->sha1), spec->path);\n+\t\t\t\tprintf(\"M %06o %s \", spec->mode,\n+\t\t\t\t       sha1_to_hex(spec->sha1));\n \t\t\telse {\n \t\t\t\tstruct object *object = lookup_object(spec->sha1);\n-\t\t\t\tprintf(\"M %06o :%d %s\\n\", spec->mode,\n-\t\t\t\t       get_object_mark(object), spec->path);\n+\t\t\t\tprintf(\"M %06o :%d \", spec->mode,\n+\t\t\t\t       get_object_mark(object));\n \t\t\t}\n+\t\t\tprint_path(spec->path);\n+\t\t\tputchar('\\n');\n \t\t\tbreak;\n \n \t\tdefault:\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex f823c05..950d0ff 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -228,7 +228,7 @@ test_expect_success 'fast-export -C -C | fast-import' '\n \tmkdir new &&\n \tgit --git-dir=new/.git init &&\n \tgit fast-export -C -C --signed-tags=strip --all > output &&\n-\tgrep \"^C \\\"file6\\\" \\\"file7\\\"\\$\" output &&\n+\tgrep \"^C file6 file7\\$\" output &&\n \tcat output |\n \t(cd new &&\n \t git fast-import &&\n@@ -414,4 +414,30 @@ test_expect_success SYMLINKS 'directory becomes symlink'        '\n \t(cd result && git show master:foo)\n '\n \n+test_expect_success 'fast-export quotes pathnames' '\n+\tgit init crazy-paths &&\n+\t(cd crazy-paths &&\n+\t blob=`echo foo | git hash-object -w --stdin` &&\n+\t git update-index --add \\\n+\t\t--cacheinfo 100644 $blob \"$(printf \"path with\\\\nnewline\")\" \\\n+\t\t--cacheinfo 100644 $blob \"path with \\\"quote\\\"\" \\\n+\t\t--cacheinfo 100644 $blob \"path with \\\\backslash\" \\\n+\t\t--cacheinfo 100644 $blob \"path with space\" &&\n+\t git commit -m addition &&\n+\t git ls-files -z -s | perl -0pe \"s{\\\\t}{$&subdir/}\" >index &&\n+\t git read-tree --empty &&\n+\t git update-index -z --index-info <index &&\n+\t git commit -m rename &&\n+\t git read-tree --empty &&\n+\t git commit -m deletion &&\n+\t git fast-export HEAD >export.out &&\n+\t git rev-list HEAD >expect &&\n+\t git init result &&\n+\t cd result &&\n+\t git fast-import <../export.out &&\n+\t git rev-list HEAD >actual &&\n+\t test_cmp ../expect actual\n+\t)\n+'\n+\n test_done\n-- \n1.7.6.rc0.36.gd385b.dirty\n"},{"id":"172997","messageId":"7v4o1v31uw.fsf@alter.siamese.dyndns.org","threadId":"28025","inReplyTo":"20110805223622.GA5808@sigill.intra.peff.net","subject":"Re: [PATCH] fast-export: quote paths in output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-05T22:55:19Z","receivedAt":"2011-08-05T22:55:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> +\t git read-tree --empty &&\n\nHmmmm, this adds more work to the backporting of this fix.\n"},{"id":"173000","messageId":"20110806023604.GA7013@sigill.intra.peff.net","threadId":"28025","inReplyTo":"7v4o1v31uw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] fast-export: quote paths in output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-08-06T02:36:04Z","receivedAt":"2011-08-06T02:36:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 05, 2011 at 03:55:19PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > +\t git read-tree --empty &&\n> \n> Hmmmm, this adds more work to the backporting of this fix.\n\nWe can easily replace it with \"rm -f .git/index\". Or we can be fancy and\nactually tell update-index to drop the entries individually, but they\nare somewhat of a pain to manipulate, given that they have magic\ncharacters.\n\n-Peff\n"}]}