parse_arg(): really test that argument is properly terminated

The old parse_arg(), when fed an argument "refs/heads/a"master parsed 'refs/heads/a' off of the front of the argument and considered itself successful. It was only when parse_next_arg() tried to parse the *next* argument that a problem was noticed. But in fact, the definition of the input format requires arguments to be terminated by SP or NUL, so *this* argument is already erroneous and parse_arg() should diagnose the problem. So teach parse_arg() to verify that C-quoted arguments are terminated correctly. If not, emit a more specific error message. There is no corresponding error case of a non-C-quoted argument that is not terminated correctly, because the end of a non-quoted argument is *by definition* a space or NUL, so there is no way to insert other junk between the "end" of the argument and the argument terminator. Adjust the tests to expect the new error message. Add a docstring to the function, incorporating the comments that were formerly within the function plus some added information. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Michael Haggerty committed Apr 7, 2014 at 15:47 UTC 697a41519b0d41c1b2c5714b5558f68814d78885
2 files changed +17 -7
builtin/update-ref.c
+15 -5
@@ -62,16 +62,26 @@ static void update_store_old_sha1(struct ref_update *update,
62 update->have_old = *oldvalue || line_termination;
63 }
64
65 +/*
66 + * Parse one whitespace- or NUL-terminated, possibly C-quoted argument
67 + * and append the result to arg. Return a pointer to the terminator.
68 + * Die if there is an error in how the argument is C-quoted. This
69 + * function is only used if not -z.
70 + */
71 static const char *parse_arg(const char *next, struct strbuf *arg)
72 {
67 - /* Parse SP-terminated, possibly C-quoted argument */
68 - if (*next != '"')
73 + if (*next == '"') {
74 + const char *orig = next;
75 +
76 + if (unquote_c_style(arg, next, &next))
77 + die("badly quoted argument: %s", orig);
78 + if (*next && !isspace(*next))
79 + die("unexpected character after quoted argument: %s", orig);
80 + } else {
81 while (*next && !isspace(*next))
82 strbuf_addch(arg, *next++);
71 - else if (unquote_c_style(arg, next, &next))
72 - die("badly quoted argument: %s", next);
83 + }
84
74 - /* Return position after the argument */
85 return next;
86 }
87
t/t1400-update-ref.sh
+2 -2
@@ -356,10 +356,10 @@ test_expect_success 'stdin fails on badly quoted input' '
356 grep "fatal: badly quoted argument: \\\"master" err
357 '
358
359 -test_expect_success 'stdin fails on arguments not separated by space' '
359 +test_expect_success 'stdin fails on junk after quoted argument' '
360 echo "create \"$a\"master" >stdin &&
361 test_must_fail git update-ref --stdin <stdin 2>err &&
362 - grep "fatal: expected SP but got: master" err
362 + grep "fatal: unexpected character after quoted argument: \\\"$a\\\"master" err
363 '
364
365 test_expect_success 'stdin fails create with no ref' '