{"thread":{"id":"51515","subject":"[PATCH] xdiff: clamp function context indices in post-image","startedAt":"2019-07-23T19:27:07Z","lastAt":"2019-07-23T19:27:07Z","messageCount":1,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"379177","messageId":"20190723192704.GA4065@sigill.intra.peff.net","threadId":"51515","inReplyTo":null,"subject":"[PATCH] xdiff: clamp function context indices in post-image","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-07-23T19:27:05Z","receivedAt":"2019-07-23T19:27:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"After finding a function line for --function-context in the pre-image,\nxdl_emit_diff() calculates the equivalent line in the post-image.  It\nassumes that the lines between changes are the same on both sides.  If\nthe option --ignore-blank-lines was also given then this is not\nnecessarily true.\n\nClamp the calculation results for start and end of the function context\nto prevent out-of-bounds array accesses.\n\nNote that this _just_ fixes the case where our mismatch sends us off the\nbeginning of the file. There are likely other cases where our assumption\ncauses us to go to the wrong line within the file. Nobody has developed\na test case yet, and the ultimate fix is likely more complicated than\nthis patch. But this at least prevents a segfault in the meantime.\n\nCredit for finding the bug goes to \"Liu Wei of Tencent Security Xuanwu\nLab\".\n\nReported-by: 刘炜 <lw17qhdz@gmail.com>\nHelped-by: René Scharfe <l.s.r@web.de>\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t4015-diff-whitespace.sh | 22 ++++++++++++++++++++++\n xdiff/xemit.c              |  4 ++--\n 2 files changed, 24 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex ab4670d236..6b087df3dc 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -2008,4 +2008,26 @@ test_expect_success 'compare mixed whitespace delta across moved blocks' '\n \ttest_cmp expected actual\n '\n \n+# Note that the \"6\" in the expected hunk header below is funny, since we only\n+# show 5 lines (the missing one was blank and thus ignored). This is how\n+# --ignore-blank-lines behaves even without --function-context, and this test\n+# is just checking the interaction of the two features. Don't take it as an\n+# endorsement of that output.\n+test_expect_success 'combine --ignore-blank-lines with --function-context' '\n+\ttest_write_lines 1 \"\" 2 3 4 5 >a &&\n+\ttest_write_lines 1    2 3 4   >b &&\n+\ttest_must_fail git diff --no-index \\\n+\t\t--ignore-blank-lines --function-context a b >actual.raw &&\n+\tsed -n \"/@@/,\\$p\" <actual.raw >actual &&\n+\tcat <<-\\EOF >expect &&\n+\t@@ -1,6 +1,4 @@\n+\t 1\n+\t 2\n+\t 3\n+\t 4\n+\t-5\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/xdiff/xemit.c b/xdiff/xemit.c\nindex 7778dc2b19..30713ae9a9 100644\n--- a/xdiff/xemit.c\n+++ b/xdiff/xemit.c\n@@ -210,7 +210,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n \t\t\tif (fs1 < 0)\n \t\t\t\tfs1 = 0;\n \t\t\tif (fs1 < s1) {\n-\t\t\t\ts2 -= s1 - fs1;\n+\t\t\t\ts2 = XDL_MAX(s2 - (s1 - fs1), 0);\n \t\t\t\ts1 = fs1;\n \t\t\t}\n \t\t}\n@@ -232,7 +232,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n \t\t\tif (fe1 < 0)\n \t\t\t\tfe1 = xe->xdf1.nrec;\n \t\t\tif (fe1 > e1) {\n-\t\t\t\te2 += fe1 - e1;\n+\t\t\t\te2 = XDL_MIN(e2 + (fe1 - e1), xe->xdf2.nrec);\n \t\t\t\te1 = fe1;\n \t\t\t}\n \n-- \n2.22.0.993.gcc1030c86b\n"}]}