Re: [PATCH 0/4] diff: reject negative context values

[Junio C Hamano <gitster@pobox.com>]

"Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:

> 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- @@

It may not matter in the cover letter, but why do you need ~60
whitespace characters at the end of the command line, and many other
lines in the message?



>
>
> 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.
> PARSE_OPT_NONEG was added but only prevents the --no-* boolean form,
> not negative numeric arguments.
>
> 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 PARSE_OPT_NONEG does not reject                                                    
>      negative numbers                                                                                          
>      Documentation fix.                                                                                        
>
>
> The config variables diff.context and diff.interHunkContext have
> always rejected negative values. This series brings the CLI options in line.
>
> 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 PARSE_OPT_NONEG does not reject negative
>     numbers
>
>  diff.c                             | 25 ++++++++++++++-----------
>  diff.h                             |  4 ++--
>  parse-options.h                    |  5 ++++-
>  t/t4032-diff-inter-hunk-context.sh |  6 ++++++
>  t/t4055-diff-context.sh            |  5 +++++
>  xdiff/xemit.c                      | 16 ++++++++++++----
>  6 files changed, 43 insertions(+), 18 deletions(-)
>
>
> base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2105%2Fmmontalbo%2Fmm%2Freject-negative-interhunk-context-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2105/mmontalbo/mm/reject-negative-interhunk-context-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2105