diff: reject negative context values

Negative values for -U and --inter-hunk-context are silently accepted and produce structurally invalid diff output.

Malformed hunk headers:

$ wc -l GIT-VERSION-GEN 106 $ 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.

Overlapping hunks that cannot be applied:

$ git log -1 -p -U3 --inter-hunk-context=100 791aeddfa2
-- git-compat-util.h | git apply --check --reverse (success)

$ git log -1 -p -U3 --inter-hunk-context=-100 791aeddfa2
-- git-compat-util.h | git apply --check --reverse error: patch failed:
git-compat-util.h:118 error: git-compat-util.h: patch does not apply

Both options were originally parsed via opt_arg() which gated on isdigit(), making negative values impossible. When they were converted to OPT_INTEGER_F / OPT_CALLBACK in d473e2e0e8 (diff.c: convert -U|--unified, 2019-01-27) and 16ed6c97cc (diff-parseopt: convert --inter-hunk-context, 2019-03-24), the implicit rejection was lost.

This series restores the original invariant with stronger guarantees:

1/4 diff: reject negative values for --inter-hunk-context Change type to unsigned int, switch to OPT_UNSIGNED.

2/4 diff: reject negative values for -U/--unified Change type to unsigned int, add range check in callback.

3/4 xdiff: guard against negative context lengths BUG() in xdl_get_hunk() as defense in depth.

4/4 parse-options: clarify what "negated" means for PARSE_OPT_NONEG.

The config variables diff.context and diff.interHunkContext have always rejected negative values. This series brings the CLI options in line.

Changes since v1:

Patch 1 and 4: Rewrote commit message to not imply NONEG was related to the bug.

Patch 4: Trimmed to just clarify what "negated" means, without documenting what PARSE_OPT_NONEG does not do.

Michael Montalbo (4):
diff: reject negative values for --inter-hunk-context
diff: reject negative values for -U/--unified
xdiff: guard against negative context lengths
parse-options: clarify what "negated" means for PARSE_OPT_NONEG

 diff.c                             | 25 ++++++++++++++-----------
 diff.h                             |  4 ++--
 parse-options.h                    |  1 +
 t/t4032-diff-inter-hunk-context.sh |  6 ++++++
 t/t4055-diff-context.sh            |  5 +++++
 xdiff/xemit.c                      | 16 ++++++++++++----
 6 files changed, 40 insertions(+), 17 deletions(-)

base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2105%2Fmmontalbo%2Fmm%2Freject-negative-interhunk-context-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2105/mmontalbo/mm/reject-negative-interhunk-context-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2105

Range-diff vs v1:

1: cca75eca0e ! 1: f2ebb3a72b diff: reject negative values for --inter-hunk-context
@@ Commit message
starts at 116 (overlaps both). The resulting patch cannot be applied.

The config variable diff.interHunkContext already rejects negative
- values, but the command line option does not. The option currently
- uses OPT_INTEGER_F with PARSE_OPT_NONEG, but PARSE_OPT_NONEG only
- prevents the "--no-inter-hunk-context" boolean negation form. It does
- not reject negative numeric arguments like "--inter-hunk-context=-1".
+ values, but the command line option does not.

Change the type of diff_options.interhunkcontext and its static default from int to unsigned int, and switch the option parser from

 2:  f0478d434c = 2:  fc3d2bc31e diff: reject negative values for -U/--unified
 3:  f9cfa0c55d = 3:  020ca774c0 xdiff: guard against negative context lengths
 4:  05ff821e6f ! 4:  3a656f8c0f parse-options: clarify PARSE_OPT_NONEG does not reject negative numbers

@@ Metadata
Author: Michael Montalbo <[email protected]>

## Commit message ##
- parse-options: clarify PARSE_OPT_NONEG does not reject negative numbers
+ parse-options: clarify what "negated" means for PARSE_OPT_NONEG

- The name "NONEG" can be misread as "no negative [values]" when it
- actually means "no [boolean] negation" (the --no-* form).
-
- When --inter-hunk-context and -U/--unified were converted from a
- custom parser to OPT_INTEGER_F with PARSE_OPT_NONEG in d473e2e0e8
- and 16ed6c97cc, the implicit rejection of negative values (via
- isdigit() in the old opt_arg() parser) was silently lost. The
- previous commits in this series fix the resulting bugs.
-
- Add a clarifying note to the flag documentation.
+ The documentation says the flag prevents an option from being
+ "negated" without specifying what that means. Add a parenthetical
+ to clarify that it rejects the "--no-<option>" form.

## parse-options.h ##
@@ parse-options.h: typedef int parse_opt_subcommand_fn(int argc, const char **argv,
- * mask of parse_opt_option_flags.
* PARSE_OPT_OPTARG: says that the argument is optional (not for BOOLEANs)
* PARSE_OPT_NOARG: says that this option does not take an argument
-- * PARSE_OPT_NONEG: says that this option cannot be negated
-+ * PARSE_OPT_NONEG: says that this option cannot be negated (i.e.
-+ * prevents --no-<option> boolean form). Does not reject
-+ * negative numeric values like --option=-1. Use
-+ * OPT_UNSIGNED for options that must be non-negative.
+ * PARSE_OPT_NONEG: says that this option cannot be negated
++ * (i.e. rejects "--no-<option>")
* PARSE_OPT_HIDDEN: this option is skipped in the default usage, and
* shown only in the full usage.
* PARSE_OPT_LASTARG_DEFAULT: says that this option will take the default

gitgitgadget