diff: reject negative values for -U/--unified

Passing a negative value to -U is silently accepted and produces
corrupt unified diff output with malformed hunk headers:

    $ git log -1 -p -U-500 -- GIT-VERSION-GEN | grep '^@@'
    @@ -503,999- +503,999- @@

Line 503 of a 106-line file, count "999-" is not a valid integer.

The config variable diff.context already rejects negative values, but
the command line callback diff_opt_unified() uses strtol() with no
range check.

Change the type of diff_options.context and its static default from
int to unsigned int, matching the change to interhunkcontext in the
previous commit. The type change requires reworking the callback and
config parsing to validate in a local variable before assigning to
the now-unsigned field.

Unlike --inter-hunk-context which could be converted to OPT_UNSIGNED,
-U needs OPT_CALLBACK_F for PARSE_OPT_OPTARG (bare -U with no value
enables patch output). Add a range check in the callback instead.

Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diff.c b/diff.c
index 5df28e49c5057a5b1ea5ff29f0b8d27552c691d5..1771b2c444c005be458534d0eaeea20c148ca9ee 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -60,7 +60,7 @@ static int diff_suppress_blank_empty;
 static enum git_colorbool diff_use_color_default = GIT_COLOR_UNKNOWN;
 static int diff_color_moved_default;
 static int diff_color_moved_ws_default;
-static int diff_context_default = 3;
+static unsigned int diff_context_default = 3;
 static unsigned int diff_interhunk_context_default;
 static char *diff_word_regex_cfg;
 static struct external_diff external_diff_cfg;
@@ -382,9 +382,10 @@ int git_diff_ui_config(const char *var, const char *value,
                return 0;
        }
        if (!strcmp(var, "diff.context")) {
-               diff_context_default = git_config_int(var, value, ctx->kvi);
-               if (diff_context_default < 0)
+               int val = git_config_int(var, value, ctx->kvi);
+               if (val < 0)
                        return -1;
+               diff_context_default = val;
                return 0;
        }
        if (!strcmp(var, "diff.interhunkcontext")) {
@@ -5924,9 +5925,12 @@ static int diff_opt_unified(const struct option *opt,
        BUG_ON_OPT_NEG(unset);
 
        if (arg) {
-               options->context = strtol(arg, &s, 10);
+               long val = strtol(arg, &s, 10);
                if (*s)
                        return error(_("%s expects a numerical value"), "--unified");
+               if (val < 0)
+                       return error(_("%s expects a non-negative integer"), "--unified");
+               options->context = val;
        }
        enable_patch_output(&options->output_format);
 

diff --git a/diff.h b/diff.h
index 033d633db4112963afc5183d7333fc7a3d12242c..bb5cddaf3499e9a59c76382fa52a69adb5b7dbd6 100644 (file)
--- a/diff.h
+++ b/diff.h
@@ -294,7 +294,7 @@ struct diff_options {
        enum git_colorbool use_color;
 
        /* Number of context lines to generate in patch output. */
-       int context;
+       unsigned int context;
 
        unsigned int interhunkcontext;
 

diff --git a/t/t4055-diff-context.sh b/t/t4055-diff-context.sh
index 1384a8195705e3a838b3e4c442d19315917945f4..b26f6eea7c8332233e5b1eeba4d04b9541f234e6 100755 (executable)
--- a/t/t4055-diff-context.sh
+++ b/t/t4055-diff-context.sh
@@ -82,6 +82,11 @@ test_expect_success 'negative integer config parsing' '
        test_grep "bad config variable" output
 '
 
+test_expect_success '-U-1 is rejected' '
+       test_must_fail git diff -U-1 2>err &&
+       test_grep "expects a non-negative integer" err
+'
+
 test_expect_success '-U0 is valid, so is diff.context=0' '
        test_config diff.context 0 &&
        git diff >output &&