fast-import: tighten parsing of datarefs

The syntax for the use of mark references in fast-import demands either a SP (space) or LF (end-of-line) after a mark reference. Fast-import does not complain when garbage appears after a mark reference in some cases. Factor out parsing of mark references and complain if errant characters are found. Also be a little more careful when parsing "inline" and SHA1s, complaining if extra characters appear or if the form of the dataref is unrecognized. Buggy input can cause fast-import to produce the wrong output, silently, without error. This makes it difficult to track down buggy generators of fast-import streams. An example is seen in the last line of this commit command: commit refs/heads/S2 committer Name <name@example.com> 1112912893 -0400 data <<COMMIT commit message COMMIT from :1M 100644 :103 hello.c It is missing a newline and should be: [...] from :1 M 100644 :103 hello.c What fast-import does is to produce a commit with the same contents for hello.c as in refs/heads/S2^. What the buggy program was expecting was the contents of blob :103. While the resulting commit graph looked correct, the contents in some commits were wrong. Signed-off-by: Pete Wyckoff <pw@padd.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Pete Wyckoff committed Apr 7, 2012 at 18:59 UTC 06454cb9a3bd2a34299779b147e388ff0f31c9f7
2 files changed +364 -33
fast-import.c
+77 -33
@@ -2207,6 +2207,59 @@ static uintmax_t change_note_fanout(struct tree_entry *root,
2207 return do_change_note_fanout(root, root, hex_sha1, 0, path, 0, fanout);
2208 }
2209
2210 +/*
2211 + * Given a pointer into a string, parse a mark reference:
2212 + *
2213 + * idnum ::= ':' bigint;
2214 + *
2215 + * Return the first character after the value in *endptr.
2216 + *
2217 + * Complain if the following character is not what is expected,
2218 + * either a space or end of the string.
2219 + */
2220 +static uintmax_t parse_mark_ref(const char *p, char **endptr)
2221 +{
2222 + uintmax_t mark;
2223 +
2224 + assert(*p == ':');
2225 + p++;
2226 + mark = strtoumax(p, endptr, 10);
2227 + if (*endptr == p)
2228 + die("No value after ':' in mark: %s", command_buf.buf);
2229 + return mark;
2230 +}
2231 +
2232 +/*
2233 + * Parse the mark reference, and complain if this is not the end of
2234 + * the string.
2235 + */
2236 +static uintmax_t parse_mark_ref_eol(const char *p)
2237 +{
2238 + char *end;
2239 + uintmax_t mark;
2240 +
2241 + mark = parse_mark_ref(p, &end);
2242 + if (*end != '\0')
2243 + die("Garbage after mark: %s", command_buf.buf);
2244 + return mark;
2245 +}
2246 +
2247 +/*
2248 + * Parse the mark reference, demanding a trailing space. Return a
2249 + * pointer to the space.
2250 + */
2251 +static uintmax_t parse_mark_ref_space(const char **p)
2252 +{
2253 + uintmax_t mark;
2254 + char *end;
2255 +
2256 + mark = parse_mark_ref(*p, &end);
2257 + if (*end != ' ')
2258 + die("Missing space after mark: %s", command_buf.buf);
2259 + *p = end;
2260 + return mark;
2261 +}
2262 +
2263 static void file_change_m(struct branch *b)
2264 {
2265 const char *p = command_buf.buf + 2;
@@ -2235,21 +2288,21 @@ static void file_change_m(struct branch *b)
2288 }
2289
2290 if (*p == ':') {
2238 - char *x;
2239 - oe = find_mark(strtoumax(p + 1, &x, 10));
2291 + oe = find_mark(parse_mark_ref_space(&p));
2292 hashcpy(sha1, oe->idx.sha1);
2241 - p = x;
2242 - } else if (!prefixcmp(p, "inline")) {
2293 + } else if (!prefixcmp(p, "inline ")) {
2294 inline_data = 1;
2244 - p += 6;
2295 + p += strlen("inline"); /* advance to space */
2296 } else {
2297 if (get_sha1_hex(p, sha1))
2247 - die("Invalid SHA1: %s", command_buf.buf);
2298 + die("Invalid dataref: %s", command_buf.buf);
2299 oe = find_object(sha1);
2300 p += 40;
2301 + if (*p != ' ')
2302 + die("Missing space after SHA1: %s", command_buf.buf);
2303 }
2251 - if (*p++ != ' ')
2252 - die("Missing space after SHA1: %s", command_buf.buf);
2304 + assert(*p == ' ');
2305 + p++; /* skip space */
2306
2307 strbuf_reset(&uq);
2308 if (!unquote_c_style(&uq, p, &endp)) {
@@ -2407,21 +2460,21 @@ static void note_change_n(struct branch *b, unsigned char *old_fanout)
2460 /* Now parse the notemodify command. */
2461 /* <dataref> or 'inline' */
2462 if (*p == ':') {
2410 - char *x;
2411 - oe = find_mark(strtoumax(p + 1, &x, 10));
2463 + oe = find_mark(parse_mark_ref_space(&p));
2464 hashcpy(sha1, oe->idx.sha1);
2413 - p = x;
2414 - } else if (!prefixcmp(p, "inline")) {
2465 + } else if (!prefixcmp(p, "inline ")) {
2466 inline_data = 1;
2416 - p += 6;
2467 + p += strlen("inline"); /* advance to space */
2468 } else {
2469 if (get_sha1_hex(p, sha1))
2419 - die("Invalid SHA1: %s", command_buf.buf);
2470 + die("Invalid dataref: %s", command_buf.buf);
2471 oe = find_object(sha1);
2472 p += 40;
2473 + if (*p != ' ')
2474 + die("Missing space after SHA1: %s", command_buf.buf);
2475 }
2423 - if (*p++ != ' ')
2424 - die("Missing space after SHA1: %s", command_buf.buf);
2476 + assert(*p == ' ');
2477 + p++; /* skip space */
2478
2479 /* <committish> */
2480 s = lookup_branch(p);
@@ -2430,7 +2483,7 @@ static void note_change_n(struct branch *b, unsigned char *old_fanout)
2483 die("Can't add a note on empty branch.");
2484 hashcpy(commit_sha1, s->sha1);
2485 } else if (*p == ':') {
2433 - uintmax_t commit_mark = strtoumax(p + 1, NULL, 10);
2486 + uintmax_t commit_mark = parse_mark_ref_eol(p);
2487 struct object_entry *commit_oe = find_mark(commit_mark);
2488 if (commit_oe->type != OBJ_COMMIT)
2489 die("Mark :%" PRIuMAX " not a commit", commit_mark);
@@ -2537,7 +2590,7 @@ static int parse_from(struct branch *b)
2590 hashcpy(b->branch_tree.versions[0].sha1, t);
2591 hashcpy(b->branch_tree.versions[1].sha1, t);
2592 } else if (*from == ':') {
2540 - uintmax_t idnum = strtoumax(from + 1, NULL, 10);
2593 + uintmax_t idnum = parse_mark_ref_eol(from);
2594 struct object_entry *oe = find_mark(idnum);
2595 if (oe->type != OBJ_COMMIT)
2596 die("Mark :%" PRIuMAX " not a commit", idnum);
@@ -2572,7 +2625,7 @@ static struct hash_list *parse_merge(unsigned int *count)
2625 if (s)
2626 hashcpy(n->sha1, s->sha1);
2627 else if (*from == ':') {
2575 - uintmax_t idnum = strtoumax(from + 1, NULL, 10);
2628 + uintmax_t idnum = parse_mark_ref_eol(from);
2629 struct object_entry *oe = find_mark(idnum);
2630 if (oe->type != OBJ_COMMIT)
2631 die("Mark :%" PRIuMAX " not a commit", idnum);
@@ -2735,7 +2788,7 @@ static void parse_new_tag(void)
2788 type = OBJ_COMMIT;
2789 } else if (*from == ':') {
2790 struct object_entry *oe;
2738 - from_mark = strtoumax(from + 1, NULL, 10);
2791 + from_mark = parse_mark_ref_eol(from);
2792 oe = find_mark(from_mark);
2793 type = oe->type;
2794 hashcpy(sha1, oe->idx.sha1);
@@ -2867,18 +2920,13 @@ static void parse_cat_blob(void)
2920 /* cat-blob SP <object> LF */
2921 p = command_buf.buf + strlen("cat-blob ");
2922 if (*p == ':') {
2870 - char *x;
2871 - oe = find_mark(strtoumax(p + 1, &x, 10));
2872 - if (x == p + 1)
2873 - die("Invalid mark: %s", command_buf.buf);
2923 + oe = find_mark(parse_mark_ref_eol(p));
2924 if (!oe)
2925 die("Unknown mark: %s", command_buf.buf);
2876 - if (*x)
2877 - die("Garbage after mark: %s", command_buf.buf);
2926 hashcpy(sha1, oe->idx.sha1);
2927 } else {
2928 if (get_sha1_hex(p, sha1))
2881 - die("Invalid SHA1: %s", command_buf.buf);
2929 + die("Invalid dataref: %s", command_buf.buf);
2930 if (p[40])
2931 die("Garbage after SHA1: %s", command_buf.buf);
2932 oe = find_object(sha1);
@@ -2944,17 +2992,13 @@ static struct object_entry *parse_treeish_dataref(const char **p)
2992 struct object_entry *e;
2993
2994 if (**p == ':') { /* <mark> */
2947 - char *endptr;
2948 - e = find_mark(strtoumax(*p + 1, &endptr, 10));
2949 - if (endptr == *p + 1)
2950 - die("Invalid mark: %s", command_buf.buf);
2995 + e = find_mark(parse_mark_ref_space(p));
2996 if (!e)
2997 die("Unknown mark: %s", command_buf.buf);
2953 - *p = endptr;
2998 hashcpy(sha1, e->idx.sha1);
2999 } else { /* <sha1> */
3000 if (get_sha1_hex(*p, sha1))
2957 - die("Invalid SHA1: %s", command_buf.buf);
3001 + die("Invalid dataref: %s", command_buf.buf);
3002 e = find_object(sha1);
3003 *p += 40;
3004 }
t/t9300-fast-import.sh
+287
@@ -2635,4 +2635,291 @@ test_expect_success \
2635 'n=$(grep $a verify | wc -l) &&
2636 test 1 = $n'
2637
2638 +###
2639 +### series S
2640 +###
2641 +#
2642 +# Make sure missing spaces and EOLs after mark references
2643 +# cause errors.
2644 +#
2645 +# Setup:
2646 +#
2647 +# 1--2--4
2648 +# \ /
2649 +# -3-
2650 +#
2651 +# commit marks: 301, 302, 303, 304
2652 +# blob marks: 403, 404, resp.
2653 +# note mark: 202
2654 +#
2655 +# The error message when a space is missing not at the
2656 +# end of the line is:
2657 +#
2658 +# Missing space after ..
2659 +#
2660 +# or when extra characters come after the mark at the end
2661 +# of the line:
2662 +#
2663 +# Garbage after ..
2664 +#
2665 +# or when the dataref is neither "inline " or a known SHA1,
2666 +#
2667 +# Invalid dataref ..
2668 +#
2669 +test_tick
2670 +
2671 +cat >input <<INPUT_END
2672 +commit refs/heads/S
2673 +mark :301
2674 +committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
2675 +data <<COMMIT
2676 +commit 1
2677 +COMMIT
2678 +M 100644 inline hello.c
2679 +data <<BLOB
2680 +blob 1
2681 +BLOB
2682 +
2683 +commit refs/heads/S
2684 +mark :302
2685 +committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
2686 +data <<COMMIT
2687 +commit 2
2688 +COMMIT
2689 +from :301
2690 +M 100644 inline hello.c
2691 +data <<BLOB
2692 +blob 2
2693 +BLOB
2694 +
2695 +blob
2696 +mark :403
2697 +data <<BLOB
2698 +blob 3
2699 +BLOB
2700 +
2701 +blob
2702 +mark :202
2703 +data <<BLOB
2704 +note 2
2705 +BLOB
2706 +INPUT_END
2707 +
2708 +test_expect_success 'S: initialize for S tests' '
2709 + git fast-import --export-marks=marks <input
2710 +'
2711 +
2712 +#
2713 +# filemodify, three datarefs
2714 +#
2715 +test_expect_success 'S: filemodify with garbage after mark must fail' '
2716 + test_must_fail git fast-import --import-marks=marks <<-EOF 2>err &&
2717 + commit refs/heads/S
2718 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
2719 + data <<COMMIT
2720 + commit N
2721 + COMMIT
2722 + M 100644 :403x hello.c
2723 + EOF
2724 + cat err &&
2725 + test_i18ngrep "space after mark" err
2726 +'
2727 +
2728 +# inline is misspelled; fast-import thinks it is some unknown dataref
2729 +test_expect_success 'S: filemodify with garbage after inline must fail' '
2730 + test_must_fail git fast-import --import-marks=marks <<-EOF 2>err &&
2731 + commit refs/heads/S
2732 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
2733 + data <<COMMIT
2734 + commit N
2735 + COMMIT
2736 + M 100644 inlineX hello.c
2737 + data <<BLOB
2738 + inline
2739 + BLOB
2740 + EOF
2741 + cat err &&
2742 + test_i18ngrep "nvalid dataref" err
2743 +'
2744 +
2745 +test_expect_success 'S: filemodify with garbage after sha1 must fail' '
2746 + sha1=$(grep :403 marks | cut -d\ -f2) &&
2747 + test_must_fail git fast-import --import-marks=marks <<-EOF 2>err &&
2748 + commit refs/heads/S
2749 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
2750 + data <<COMMIT
2751 + commit N
2752 + COMMIT
2753 + M 100644 ${sha1}x hello.c
2754 + EOF
2755 + cat err &&
2756 + test_i18ngrep "space after SHA1" err
2757 +'
2758 +
2759 +#
2760 +# notemodify, three ways to say dataref
2761 +#
2762 +test_expect_success 'S: notemodify with garabge after mark dataref must fail' '
2763 + test_must_fail git fast-import --import-marks=marks <<-EOF 2>err &&
2764 + commit refs/heads/S
2765 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
2766 + data <<COMMIT
2767 + commit S note dataref markref
2768 + COMMIT
2769 + N :202x :302
2770 + EOF
2771 + cat err &&
2772 + test_i18ngrep "space after mark" err
2773 +'
2774 +
2775 +test_expect_success 'S: notemodify with garbage after inline dataref must fail' '
2776 + test_must_fail git fast-import --import-marks=marks <<-EOF 2>err &&
2777 + commit refs/heads/S
2778 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
2779 + data <<COMMIT
2780 + commit S note dataref inline
2781 + COMMIT
2782 + N inlineX :302
2783 + data <<BLOB
2784 + note blob
2785 + BLOB
2786 + EOF
2787 + cat err &&
2788 + test_i18ngrep "nvalid dataref" err
2789 +'
2790 +
2791 +test_expect_success 'S: notemodify with garbage after sha1 dataref must fail' '
2792 + sha1=$(grep :202 marks | cut -d\ -f2) &&
2793 + test_must_fail git fast-import --import-marks=marks <<-EOF 2>err &&
2794 + commit refs/heads/S
2795 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
2796 + data <<COMMIT
2797 + commit S note dataref sha1
2798 + COMMIT
2799 + N ${sha1}x :302
2800 + EOF
2801 + cat err &&
2802 + test_i18ngrep "space after SHA1" err
2803 +'
2804 +
2805 +#
2806 +# notemodify, mark in committish
2807 +#
2808 +test_expect_success 'S: notemodify with garbarge after mark committish must fail' '
2809 + test_must_fail git fast-import --import-marks=marks <<-EOF 2>err &&
2810 + commit refs/heads/Snotes
2811 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
2812 + data <<COMMIT
2813 + commit S note committish
2814 + COMMIT
2815 + N :202 :302x
2816 + EOF
2817 + cat err &&
2818 + test_i18ngrep "after mark" err
2819 +'
2820 +
2821 +#
2822 +# from
2823 +#
2824 +test_expect_success 'S: from with garbage after mark must fail' '
2825 + # no &&
2826 + git fast-import --import-marks=marks --export-marks=marks <<-EOF 2>err
2827 + commit refs/heads/S2
2828 + mark :303
2829 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
2830 + data <<COMMIT
2831 + commit 3
2832 + COMMIT
2833 + from :301x
2834 + M 100644 :403 hello.c
2835 + EOF
2836 +
2837 + ret=$? &&
2838 + echo returned $ret &&
2839 + test $ret -ne 0 && # failed, but it created the commit
2840 +
2841 + # go create the commit, need it for merge test
2842 + git fast-import --import-marks=marks --export-marks=marks <<-EOF &&
2843 + commit refs/heads/S2
2844 + mark :303
2845 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
2846 + data <<COMMIT
2847 + commit 3
2848 + COMMIT
2849 + from :301
2850 + M 100644 :403 hello.c
2851 + EOF
2852 +
2853 + # now evaluate the error
2854 + cat err &&
2855 + test_i18ngrep "after mark" err
2856 +'
2857 +
2858 +
2859 +#
2860 +# merge
2861 +#
2862 +test_expect_success 'S: merge with garbage after mark must fail' '
2863 + test_must_fail git fast-import --import-marks=marks <<-EOF 2>err &&
2864 + commit refs/heads/S
2865 + mark :304
2866 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
2867 + data <<COMMIT
2868 + merge 4
2869 + COMMIT
2870 + from :302
2871 + merge :303x
2872 + M 100644 :403 hello.c
2873 + EOF
2874 + cat err &&
2875 + test_i18ngrep "after mark" err
2876 +'
2877 +
2878 +#
2879 +# tag, from markref
2880 +#
2881 +test_expect_success 'S: tag with garbage after mark must fail' '
2882 + test_must_fail git fast-import --import-marks=marks <<-EOF 2>err &&
2883 + tag refs/tags/Stag
2884 + from :302x
2885 + tagger $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
2886 + data <<TAG
2887 + tag S
2888 + TAG
2889 + EOF
2890 + cat err &&
2891 + test_i18ngrep "after mark" err
2892 +'
2893 +
2894 +#
2895 +# cat-blob markref
2896 +#
2897 +test_expect_success 'S: cat-blob with garbage after mark must fail' '
2898 + test_must_fail git fast-import --import-marks=marks <<-EOF 2>err &&
2899 + cat-blob :403x
2900 + EOF
2901 + cat err &&
2902 + test_i18ngrep "after mark" err
2903 +'
2904 +
2905 +#
2906 +# ls markref
2907 +#
2908 +test_expect_success 'S: ls with garbage after mark must fail' '
2909 + test_must_fail git fast-import --import-marks=marks <<-EOF 2>err &&
2910 + ls :302x hello.c
2911 + EOF
2912 + cat err &&
2913 + test_i18ngrep "space after mark" err
2914 +'
2915 +
2916 +test_expect_success 'S: ls with garbage after sha1 must fail' '
2917 + sha1=$(grep :302 marks | cut -d\ -f2) &&
2918 + test_must_fail git fast-import --import-marks=marks <<-EOF 2>err &&
2919 + ls ${sha1}x hello.c
2920 + EOF
2921 + cat err &&
2922 + test_i18ngrep "space after tree-ish" err
2923 +'
2924 +
2925 test_done