# [PATCH] fast-export: quote paths in output 5 messages from 2011-08-05 to 2011-08-06. Participants: Jeff King, Johannes Sixt, Junio C Hamano. Thread: https://gitlist.dev/t/28025 ## Jeff King, 2011-08-05 10:55 Subject: [PATCH] fast-export: quote paths in output Message-ID: <20110805105526.GA22480@sigill.intra.peff.net> URL: https://gitlist.dev/e/20110805105526.GA22480%40sigill.intra.peff.net ``` Many pathnames in a fast-import stream need to be quoted. In particular: 1. Pathnames at the end of an "M" or "D" line need quoting if they contain a LF or start with double-quote. 2. Pathnames on a "C" or "R" line need quoting as above, but also if they contain spaces. For (1), we weren't quoting at all. For (2), we put double-quotes around the paths to handle spaces, but ignored the possibility that they would need further quoting. This patch checks whether each pathname needs c-style quoting, and uses it. This is slightly overkill for (1), which doesn't actually need to quote many characters that vanilla c-style quoting does. However, it shouldn't hurt, as any implementation needs to be ready to handle quoted strings anyway. In addition to adding a test, we have to tweak a test which blindly assumed that case (2) would always use double-quotes, whether it needed to or not. Signed-off-by: Jeff King --- builtin/fast-export.c | 31 ++++++++++++++++++++++++------- t/t9350-fast-export.sh | 26 +++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/builtin/fast-export.c b/builtin/fast-export.c index becef85..9836e6b 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -16,6 +16,7 @@ #include "string-list.h" #include "utf8.h" #include "parse-options.h" +#include "quote.h" static const char *fast_export_usage[] = { "git fast-export [rev-list-opts]", @@ -179,6 +180,15 @@ static int depth_first(const void *a_, const void *b_) return (a->status == 'R') - (b->status == 'R'); } +static void print_path(const char *path) +{ + int need_quote = quote_c_style(path, NULL, NULL, 0); + if (need_quote) + quote_c_style(path, NULL, stdout, 0); + else + printf("%s", path); +} + static void show_filemodify(struct diff_queue_struct *q, struct diff_options *options, void *data) { @@ -196,13 +206,18 @@ static void show_filemodify(struct diff_queue_struct *q, switch (q->queue[i]->status) { case DIFF_STATUS_DELETED: - printf("D %s\n", spec->path); + printf("D "); + print_path(spec->path); + putchar('\n'); break; case DIFF_STATUS_COPIED: case DIFF_STATUS_RENAMED: - printf("%c \"%s\" \"%s\"\n", q->queue[i]->status, - ospec->path, spec->path); + printf("%c ", q->queue[i]->status); + print_path(ospec->path); + putchar(' '); + print_path(spec->path); + putchar('\n'); if (!hashcmp(ospec->sha1, spec->sha1) && ospec->mode == spec->mode) @@ -217,13 +232,15 @@ static void show_filemodify(struct diff_queue_struct *q, * output the SHA-1 verbatim. */ if (no_data || S_ISGITLINK(spec->mode)) - printf("M %06o %s %s\n", spec->mode, - sha1_to_hex(spec->sha1), spec->path); + printf("M %06o %s ", spec->mode, + sha1_to_hex(spec->sha1)); else { struct object *object = lookup_object(spec->sha1); - printf("M %06o :%d %s\n", spec->mode, - get_object_mark(object), spec->path); + printf("M %06o :%d ", spec->mode, + get_object_mark(object)); } + print_path(spec->path); + putchar('\n'); break; default: diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh index f823c05..4673ac0 100755 --- a/t/t9350-fast-export.sh +++ b/t/t9350-fast-export.sh @@ -228,7 +228,7 @@ test_expect_success 'fast-export -C -C | fast-import' ' mkdir new && git --git-dir=new/.git init && git fast-export -C -C --signed-tags=strip --all > output && - grep "^C \"file6\" \"file7\"\$" output && + grep "^C file6 file7\$" output && cat output | (cd new && git fast-import && @@ -414,4 +414,28 @@ test_expect_success SYMLINKS 'directory becomes symlink' ' (cd result && git show master:foo) ' +test_expect_success 'fast-export quotes pathnames' ' + git init crazy-paths && + (cd crazy-paths && + >"$(printf "path with\\nnewline")" && + >"path with \"quote\"" && + >"path with \\backslash" && + >"path with space" && + git add . && + git commit -m addition && + mkdir subdir && + git mv path* subdir && + git commit -m rename && + git rm -r subdir && + git commit -m deletion && + git fast-export HEAD >export.out && + git rev-list HEAD >expect && + git init result && + cd result && + git fast-import <../export.out && + git rev-list HEAD >actual && + test_cmp ../expect actual + ) +' + test_done -- 1.7.6.rc0.36.gd385b.dirty ``` ## Johannes Sixt, 2011-08-05 11:12 Subject: Re: [PATCH] fast-export: quote paths in output Message-ID: <4E3BD006.6010005@viscovery.net> URL: https://gitlist.dev/e/4E3BD006.6010005%40viscovery.net In-Reply-To: <20110805105526.GA22480@sigill.intra.peff.net> ``` Am 8/5/2011 12:55, schrieb Jeff King: > +test_expect_success 'fast-export quotes pathnames' ' > + git init crazy-paths && > + (cd crazy-paths && > + >"$(printf "path with\\nnewline")" && > + >"path with \"quote\"" && > + >"path with \\backslash" && > + >"path with space" && Please construct the tree object using git-update-index's --cacheinfo or --index-info rather than using actual files and git add so that the test can pass on filesystems that do not allow newlines, quotes, or backslashes(!) in filenames. -- Hannes -- "Atomic objects are neither active nor radioactive." -- Programming Languages -- C++, Final Committee Draft (Doc.N3092) ``` ## Jeff King, 2011-08-05 22:36 Subject: Re: [PATCH] fast-export: quote paths in output Message-ID: <20110805223622.GA5808@sigill.intra.peff.net> URL: https://gitlist.dev/e/20110805223622.GA5808%40sigill.intra.peff.net In-Reply-To: <4E3BD006.6010005@viscovery.net> ``` On Fri, Aug 05, 2011 at 01:12:06PM +0200, Johannes Sixt wrote: > Am 8/5/2011 12:55, schrieb Jeff King: > > +test_expect_success 'fast-export quotes pathnames' ' > > + git init crazy-paths && > > + (cd crazy-paths && > > + >"$(printf "path with\\nnewline")" && > > + >"path with \"quote\"" && > > + >"path with \\backslash" && > > + >"path with space" && > > Please construct the tree object using git-update-index's --cacheinfo or > --index-info rather than using actual files and git add so that the test > can pass on filesystems that do not allow newlines, quotes, or > backslashes(!) in filenames. Sorry, I should have thought of that myself. Here's an updated version. -- >8 -- Subject: [PATCH] fast-export: quote paths in output Many pathnames in a fast-import stream need to be quoted. In particular: 1. Pathnames at the end of an "M" or "D" line need quoting if they contain a LF or start with double-quote. 2. Pathnames on a "C" or "R" line need quoting as above, but also if they contain spaces. For (1), we weren't quoting at all. For (2), we put double-quotes around the paths to handle spaces, but ignored the possibility that they would need further quoting. This patch checks whether each pathname needs c-style quoting, and uses it. This is slightly overkill for (1), which doesn't actually need to quote many characters that vanilla c-style quoting does. However, it shouldn't hurt, as any implementation needs to be ready to handle quoted strings anyway. In addition to adding a test, we have to tweak a test which blindly assumed that case (2) would always use double-quotes, whether it needed to or not. Signed-off-by: Jeff King --- builtin/fast-export.c | 31 ++++++++++++++++++++++++------- t/t9350-fast-export.sh | 28 +++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/builtin/fast-export.c b/builtin/fast-export.c index becef85..9836e6b 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -16,6 +16,7 @@ #include "string-list.h" #include "utf8.h" #include "parse-options.h" +#include "quote.h" static const char *fast_export_usage[] = { "git fast-export [rev-list-opts]", @@ -179,6 +180,15 @@ static int depth_first(const void *a_, const void *b_) return (a->status == 'R') - (b->status == 'R'); } +static void print_path(const char *path) +{ + int need_quote = quote_c_style(path, NULL, NULL, 0); + if (need_quote) + quote_c_style(path, NULL, stdout, 0); + else + printf("%s", path); +} + static void show_filemodify(struct diff_queue_struct *q, struct diff_options *options, void *data) { @@ -196,13 +206,18 @@ static void show_filemodify(struct diff_queue_struct *q, switch (q->queue[i]->status) { case DIFF_STATUS_DELETED: - printf("D %s\n", spec->path); + printf("D "); + print_path(spec->path); + putchar('\n'); break; case DIFF_STATUS_COPIED: case DIFF_STATUS_RENAMED: - printf("%c \"%s\" \"%s\"\n", q->queue[i]->status, - ospec->path, spec->path); + printf("%c ", q->queue[i]->status); + print_path(ospec->path); + putchar(' '); + print_path(spec->path); + putchar('\n'); if (!hashcmp(ospec->sha1, spec->sha1) && ospec->mode == spec->mode) @@ -217,13 +232,15 @@ static void show_filemodify(struct diff_queue_struct *q, * output the SHA-1 verbatim. */ if (no_data || S_ISGITLINK(spec->mode)) - printf("M %06o %s %s\n", spec->mode, - sha1_to_hex(spec->sha1), spec->path); + printf("M %06o %s ", spec->mode, + sha1_to_hex(spec->sha1)); else { struct object *object = lookup_object(spec->sha1); - printf("M %06o :%d %s\n", spec->mode, - get_object_mark(object), spec->path); + printf("M %06o :%d ", spec->mode, + get_object_mark(object)); } + print_path(spec->path); + putchar('\n'); break; default: diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh index f823c05..950d0ff 100755 --- a/t/t9350-fast-export.sh +++ b/t/t9350-fast-export.sh @@ -228,7 +228,7 @@ test_expect_success 'fast-export -C -C | fast-import' ' mkdir new && git --git-dir=new/.git init && git fast-export -C -C --signed-tags=strip --all > output && - grep "^C \"file6\" \"file7\"\$" output && + grep "^C file6 file7\$" output && cat output | (cd new && git fast-import && @@ -414,4 +414,30 @@ test_expect_success SYMLINKS 'directory becomes symlink' ' (cd result && git show master:foo) ' +test_expect_success 'fast-export quotes pathnames' ' + git init crazy-paths && + (cd crazy-paths && + blob=`echo foo | git hash-object -w --stdin` && + git update-index --add \ + --cacheinfo 100644 $blob "$(printf "path with\\nnewline")" \ + --cacheinfo 100644 $blob "path with \"quote\"" \ + --cacheinfo 100644 $blob "path with \\backslash" \ + --cacheinfo 100644 $blob "path with space" && + git commit -m addition && + git ls-files -z -s | perl -0pe "s{\\t}{$&subdir/}" >index && + git read-tree --empty && + git update-index -z --index-info export.out && + git rev-list HEAD >expect && + git init result && + cd result && + git fast-import <../export.out && + git rev-list HEAD >actual && + test_cmp ../expect actual + ) +' + test_done -- 1.7.6.rc0.36.gd385b.dirty ``` ## Junio C Hamano, 2011-08-05 22:55 Subject: Re: [PATCH] fast-export: quote paths in output Message-ID: <7v4o1v31uw.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v4o1v31uw.fsf%40alter.siamese.dyndns.org In-Reply-To: <20110805223622.GA5808@sigill.intra.peff.net> ``` Jeff King writes: > + git read-tree --empty && Hmmmm, this adds more work to the backporting of this fix. ``` ## Jeff King, 2011-08-06 02:36 Subject: Re: [PATCH] fast-export: quote paths in output Message-ID: <20110806023604.GA7013@sigill.intra.peff.net> URL: https://gitlist.dev/e/20110806023604.GA7013%40sigill.intra.peff.net In-Reply-To: <7v4o1v31uw.fsf@alter.siamese.dyndns.org> ``` On Fri, Aug 05, 2011 at 03:55:19PM -0700, Junio C Hamano wrote: > Jeff King writes: > > > + git read-tree --empty && > > Hmmmm, this adds more work to the backporting of this fix. We can easily replace it with "rm -f .git/index". Or we can be fancy and actually tell update-index to drop the entries individually, but they are somewhat of a pain to manipulate, given that they have magic characters. -Peff ```