git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] grep: avoid setting UTF mode when dangerous with PCRE

From
Hamza Mahfooz <someguy@effective-light.com>
Date
Nov 18, 2021, 21:21 UTC
Message-ID
<JZDS2R.2JB8M8ZCH6NX1@effective-light.com>
In-Reply-To
<20211117102329.95456-1-carenas@gmail.com>

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 
<carenas@gmail.com> wrote:
> 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 <l.s.r@web.de>
> Reported-by: Andreas Schwab <schwab@linux-m68k.org>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
> 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(struct 
> grep_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.sh 
> b/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 on 
> non-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> <author@example.com>
> -	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 <author@example.com>" -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> <author@example.com>
> +	Author: <BOLD;RED>À Ú Thor<RESET> <author@example.com>
> +	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
>
Previous: Ævar Arnfjörð BjarmasonNext: René Scharfe
Message 30 of 47 in “grep: refactor next_match() and match_one_pattern() for external use”
  1. 1/3 grep: refactor next_match() and match_one_pattern() for external useHamza Mahfooz, Oct 15, 2021
  2. 3/3 grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 dataHamza Mahfooz, Oct 15, 2021
  3. Junio C HamanoOct 15, 2021
  4. René ScharfeOct 16, 2021
  5. Ævar Arnfjörð BjarmasonOct 16, 2021
  6. René ScharfeOct 16, 2021
  7. Junio C HamanoOct 17, 2021
  8. René ScharfeOct 17, 2021
  9. Ævar Arnfjörð BjarmasonOct 17, 2021
  10. Andreas SchwabNov 15, 2021
  11. Ævar Arnfjörð BjarmasonNov 15, 2021
  12. Carlo ArenasNov 16, 2021
  13. Andreas SchwabNov 16, 2021
  14. Carlo ArenasNov 16, 2021
  15. Andreas SchwabNov 16, 2021
  16. Andreas SchwabNov 16, 2021
  17. Carlo ArenasNov 16, 2021
  18. Andreas SchwabNov 16, 2021
  19. grep: avoid setting UTF mode when not neededCarlo Marcelo Arenas Belón, Nov 16, 2021
  20. Ævar Arnfjörð BjarmasonNov 16, 2021
  21. Hamza MahfoozNov 16, 2021
  22. Ævar Arnfjörð BjarmasonNov 17, 2021
  23. Carlo ArenasNov 16, 2021
  24. Junio C HamanoNov 16, 2021
  25. grep: avoid setting UTF mode when dangerous with PCRECarlo Marcelo Arenas Belón, Nov 17, 2021
  26. Junio C HamanoNov 18, 2021
  27. Carlo ArenasNov 18, 2021
  28. Hamza MahfoozNov 18, 2021
  29. Ævar Arnfjörð BjarmasonNov 19, 2021
  30. Hamza MahfoozNov 18, 2021
  31. René ScharfeNov 17, 2021
  32. Carlo ArenasNov 17, 2021
  33. Ævar Arnfjörð BjarmasonNov 17, 2021
  34. Carlo ArenasNov 17, 2021
  35. Ævar Arnfjörð BjarmasonNov 18, 2021
  36. Junio C HamanoNov 18, 2021
  37. René ScharfeNov 18, 2021
  38. Ævar Arnfjörð BjarmasonNov 19, 2021
  39. René ScharfeNov 19, 2021
  40. Carlo ArenasNov 19, 2021
  41. Junio C HamanoNov 19, 2021
  42. 2/3 pretty: colorize pattern matches in commit messagesHamza Mahfooz, Oct 15, 2021
  43. Junio C HamanoOct 15, 2021
  44. Hamza MahfoozOct 15, 2021
  45. Junio C HamanoOct 15, 2021
  46. Hamza MahfoozOct 15, 2021
  47. Junio C HamanoOct 15, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.