Re: [PATCH v2] grep: avoid setting UTF mode when dangerous with PCRE
From: Hamza Mahfooz <hidden>
Date: 2021-11-18 21:21:38
Subsystem:
the rest · Maintainer:
Linus Torvalds
Building on to your patch we can disable the feature if an older version of PCRE2 is used, like so:
diff --git a/pretty.c b/pretty.c
index 1af5b093ae..9bd3f61b30 100644
--- a/pretty.c
+++ b/pretty.c@@ -451,8 +451,11 @@ static void append_line_with_color(struct strbuf *sb, struct grep_opt *opt, buf = line; eol = buf + linelen; - +#ifdef GIT_PCRE2_VERSION_10_34_OR_HIGHER if (!opt || !want_color(color) || opt->invert) +#else + if (!opt || !want_color(color) || opt->invert || opt->pcre2) +#endif goto end; line_color = opt->colors[GREP_COLOR_SELECTED]; On Wed, Nov 17 2021 at 02:23:29 AM -0800, Carlo Marcelo Arenas Belón [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Since ae39ba431a (grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 data, 2021-10-15), PCRE2_UTF mode is enabled for cases where it will trigger UTF-8 validation errors (as reported) or can result in undefined behaviour. Our use of PCRE2 only allows searching through non UTF-8 validated data safely through the use of the PCRE2_MATCH_INVALID_UTF flag, that is only available after 10.34, so restrict the change to newer versions of PCRE and revert to the old logic for older releases, which will still allow for matching not using UTF-8 for likely most usecases (as shown in the tests). Fix one test that was using an expression that wouldn't fail without the new code so it can be forced to fail if it is missing and restrict it to run only for newer PCRE releases; while at it do some minor refactoring to cleanup the fallout for when that test might be skipped or might succeed under the new conditions. Keeping the overly complex and unnecessary logic for now, to reduce risk but with the hope that it will be cleaned up later. Helped-by: René Scharfe [off-list ref] Reported-by: Andreas Schwab <redacted> Signed-off-by: Carlo Marcelo Arenas Belón <redacted> --- v2: * restrict the code at compile time instead of reverting * "fix" test to document the behaviour under both PCRE code versions * update commit message to better explain the issue grep.c | 7 ++++++- t/t7812-grep-icase-non-ascii.sh | 32 ++++++++++++++++++-------------- 2 files changed, 24 insertions(+), 15 deletions(-)diff --git a/grep.c b/grep.c index f6e113e9f0..0126aa3db4 100644 --- a/grep.c +++ b/grep.c@@ -382,12 +382,17 @@ static void compile_pcre2_pattern(structgrep_pat *p, const struct grep_opt *opt } options |= PCRE2_CASELESS; } +#ifdef GIT_PCRE2_VERSION_10_34_OR_HIGHER if ((!opt->ignore_locale && !has_non_ascii(p->pattern)) || (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) && !(!opt->ignore_case && (p->fixed || p->is_fixed)))) options |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF); - +#else + if (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) && + !(!opt->ignore_case && (p->fixed || p->is_fixed))) + options |= PCRE2_UTF; +#endif #ifdef GIT_PCRE2_VERSION_10_36_OR_HIGHER /* Work around https://bugs.exim.org/show_bug.cgi?id=2642 fixed in 10.36 */ if (PCRE2_MATCH_INVALID_UTF && options & (PCRE2_UTF | PCRE2_CASELESS))diff --git a/t/t7812-grep-icase-non-ascii.shb/t/t7812-grep-icase-non-ascii.sh index 22487d90fd..3bfe1ee728 100755--- a/t/t7812-grep-icase-non-ascii.sh +++ b/t/t7812-grep-icase-non-ascii.sh@@ -53,14 +53,27 @@ test_expect_success REGEX_LOCALE 'pickaxe -i onnon-ascii' ' test_cmp expected actual ' -test_expect_success GETTEXT_LOCALE,PCRE 'log --author with an ascii pattern on UTF-8 data' ' - cat >expected <<-\EOF && - Author: <BOLD;RED>À Ú Thor<RESET> [off-list ref] - EOF +test_expect_success GETTEXT_LOCALE,PCRE 'setup ascii pattern on UTF-8 data' ' test_write_lines "forth" >file4 && git add file4 && git commit --author="À Ú Thor [off-list ref]" -m sécond && - git log -1 --color=always --perl-regexp --author=".*Thor" >log && + test_write_lines "fifth" >file5 && + git add file5 && + GIT_COMMITTER_NAME="Ç O Mîtter" && + GIT_COMMITTER_EMAIL="committer@example.com" && + git -c i18n.commitEncoding=latin1 commit -m thïrd +' + +test_lazy_prereq PCRE2_MATCH_INVALID_UTF ' + test-tool pcre2-config has-PCRE2_MATCH_INVALID_UTF +' + +test_expect_success GETTEXT_LOCALE,PCRE,PCRE2_MATCH_INVALID_UTF 'log --author with an ascii pattern on UTF-8 data' ' + cat >expected <<-\EOF && + Author: <BOLD;RED>A U Thor<RESET> [off-list ref] + Author: <BOLD;RED>À Ú Thor<RESET> [off-list ref] + EOF + git log --color=always --perl-regexp --author=". . Thor" >log && grep Author log >actual.raw && test_decode_color <actual.raw >actual && test_cmp expected actual@@ -70,11 +83,6 @@ test_expect_success GETTEXT_LOCALE,PCRE 'log --committer with an ascii pattern o cat >expected <<-\EOF && Commit: Ç<BOLD;RED> O Mîtter <committer@example.com><RESET> EOF - test_write_lines "fifth" >file5 && - git add file5 && - GIT_COMMITTER_NAME="Ç O Mîtter" && - GIT_COMMITTER_EMAIL="committer@example.com" && - git -c i18n.commitEncoding=latin1 commit -m thïrd && git -c i18n.logOutputEncoding=latin1 log -1 --pretty=fuller --color=always --perl-regexp --committer=" O.*" >log && grep Commit: log >actual.raw && test_decode_color <actual.raw >actual &&@@ -141,10 +149,6 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2'PCRE v2: grep non-ASCII from invali test_cmp invalid-0xe5 actual ' -test_lazy_prereq PCRE2_MATCH_INVALID_UTF ' - test-tool pcre2-config has-PCRE2_MATCH_INVALID_UTF -' - test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' ' test_might_fail git grep -hi "Æ" invalid-0x80 >actual && test_might_fail git grep -hi "(*NO_JIT)Æ" invalid-0x80 >actual -- 2.34.0.352.g07dee3c5e1