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>

diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 1292cfea11d87e8c98ba64ffae9d6a1aeffaca9f..02b5f950e34a3122f55f8aba35f3fb116cc57e06 100644 (file)
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -62,16 +62,26 @@ static void update_store_old_sha1(struct ref_update *update,
        update->have_old = *oldvalue || line_termination;
 }
 
+/*
+ * Parse one whitespace- or NUL-terminated, possibly C-quoted argument
+ * and append the result to arg.  Return a pointer to the terminator.
+ * Die if there is an error in how the argument is C-quoted.  This
+ * function is only used if not -z.
+ */
 static const char *parse_arg(const char *next, struct strbuf *arg)
 {
-       /* Parse SP-terminated, possibly C-quoted argument */
-       if (*next != '"')
+       if (*next == '"') {
+               const char *orig = next;
+
+               if (unquote_c_style(arg, next, &next))
+                       die("badly quoted argument: %s", orig);
+               if (*next && !isspace(*next))
+                       die("unexpected character after quoted argument: %s", orig);
+       } else {
                while (*next && !isspace(*next))
                        strbuf_addch(arg, *next++);
-       else if (unquote_c_style(arg, next, &next))
-               die("badly quoted argument: %s", next);
+       }
 
-       /* Return position after the argument */
        return next;
 }
 

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 29391c67fc9aaccba9ce9af5327acfcbba5f3bf0..774f8c5bf16b21642165e819283d7768487fd89d 100755 (executable)
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -356,10 +356,10 @@ test_expect_success 'stdin fails on badly quoted input' '
        grep "fatal: badly quoted argument: \\\"master" err
 '
 
-test_expect_success 'stdin fails on arguments not separated by space' '
+test_expect_success 'stdin fails on junk after quoted argument' '
        echo "create \"$a\"master" >stdin &&
        test_must_fail git update-ref --stdin <stdin 2>err &&
-       grep "fatal: expected SP but got: master" err
+       grep "fatal: unexpected character after quoted argument: \\\"$a\\\"master" err
 '
 
 test_expect_success 'stdin fails create with no ref' '