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 <mmontalbo@gmail.com>
## 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.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
## 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