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

[PATCH v2 2/9] grep: don't use PCRE2?_UTF8 with "log --encoding=<non-utf8>"

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jun 27, 2019, 23:39 UTC
Message-ID
<20190627233912.7117-3-avarab@gmail.com>
In-Reply-To
<20190626000329.32475-1-avarab@gmail.com>

Fix a bug introduced in 18547aacf5 ("grep/pcre: support utf-8", 2016-06-25) that was missed due to a blindspot in our tests, as discussed in the previous commit. I then blindly copied the same bug in 94da9193a6 ("grep: add support for PCRE v2", 2017-06-01) when adding the PCRE v2 code.

We should not tell PCRE that we're processing UTF-8 just because we're dealing with non-ASCII. In the case of e.g. "log --encoding=<...>" under is_utf8_locale() the haystack might be in ISO-8859-1, and the needle might be in a non-UTF-8 encoding.

Maybe we should be more strict here and die earlier? Should we also be converting the needle to the encoding in question, and failing if it's not a string that's valid in that encoding? Maybe.

But for now matching this as non-UTF8 at least has some hope of producing sensible results, since we know that our default heuristic of assuming the text to be matched is in the user locale encoding isn't true when we've explicitly encoded it to be in a different encoding.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 grep.c              | 8 ++++----
 grep.h              | 1 +
 revision.c          | 3 +++
 t/t4210-log-i18n.sh | 6 ++----
 4 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/grep.c b/grep.c
index f7c3a5803e..1de4ab49c0 100644
--- a/grep.c
+++ b/grep.c
@@ -388,11 +388,11 @@ static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt)
 	int options = PCRE_MULTILINE;
 
 	if (opt->ignore_case) {
-		if (has_non_ascii(p->pattern))
+		if (!opt->ignore_locale && has_non_ascii(p->pattern))
 			p->pcre1_tables = pcre_maketables();
 		options |= PCRE_CASELESS;
 	}
-	if (is_utf8_locale() && has_non_ascii(p->pattern))
+	if (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern))
 		options |= PCRE_UTF8;
 
 	p->pcre1_regexp = pcre_compile(p->pattern, options, &error, &erroffset,
@@ -498,14 +498,14 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
 	p->pcre2_compile_context = NULL;
 
 	if (opt->ignore_case) {
-		if (has_non_ascii(p->pattern)) {
+		if (!opt->ignore_locale && has_non_ascii(p->pattern)) {
 			character_tables = pcre2_maketables(NULL);
 			p->pcre2_compile_context = pcre2_compile_context_create(NULL);
 			pcre2_set_character_tables(p->pcre2_compile_context, character_tables);
 		}
 		options |= PCRE2_CASELESS;
 	}
-	if (is_utf8_locale() && has_non_ascii(p->pattern))
+	if (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern))
 		options |= PCRE2_UTF;
 
 	p->pcre2_pattern = pcre2_compile((PCRE2_SPTR)p->pattern,
diff --git a/grep.h b/grep.h
index 1875880f37..4bb8a79d93 100644
--- a/grep.h
+++ b/grep.h
@@ -173,6 +173,7 @@ struct grep_opt {
 	int funcbody;
 	int extended_regexp_option;
 	int pattern_type_option;
+	int ignore_locale;
 	char colors[NR_GREP_COLORS][COLOR_MAXLEN];
 	unsigned pre_context;
 	unsigned post_context;
diff --git a/revision.c b/revision.c
index 621feb9df7..a842fb158a 100644
--- a/revision.c
+++ b/revision.c
@@ -28,6 +28,7 @@
 #include "commit-graph.h"
 #include "prio-queue.h"
 #include "hashmap.h"
+#include "utf8.h"
 
 volatile show_early_output_fn_t show_early_output;
 
@@ -2655,6 +2656,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 
 	grep_commit_pattern_type(GREP_PATTERN_TYPE_UNSPECIFIED,
 				 &revs->grep_filter);
+	if (!is_encoding_utf8(get_log_output_encoding()))
+		revs->grep_filter.ignore_locale = 1;
 	compile_grep_patterns(&revs->grep_filter);
 
 	if (revs->reverse && revs->reflog_info)
diff --git a/t/t4210-log-i18n.sh b/t/t4210-log-i18n.sh
index 86d22c1d4c..515bcb7ce1 100755
--- a/t/t4210-log-i18n.sh
+++ b/t/t4210-log-i18n.sh
@@ -59,10 +59,8 @@ test_expect_success 'log --grep does not find non-reencoded values (latin1)' '
 for engine in fixed basic extended perl
 do
 	prereq=
-	result=success
 	if test $engine = "perl"
 	then
-		result=failure
 		prereq="PCRE"
 	else
 		prereq=""
@@ -72,7 +70,7 @@ do
 	then
 	    force_regex=.*
 	fi
-	test_expect_$result GETTEXT_LOCALE,$prereq "-c grep.patternType=$engine log --grep does not find non-reencoded values (latin1 + locale)" "
+	test_expect_success GETTEXT_LOCALE,$prereq "-c grep.patternType=$engine log --grep does not find non-reencoded values (latin1 + locale)" "
 		cat >expect <<-\EOF &&
 		latin1
 		utf8
@@ -86,7 +84,7 @@ do
 		test_must_be_empty actual
 	"
 
-	test_expect_$result GETTEXT_LOCALE,$prereq "-c grep.patternType=$engine log --grep does not die on invalid UTF-8 value (latin1 + locale + invalid needle)" "
+	test_expect_success GETTEXT_LOCALE,$prereq "-c grep.patternType=$engine log --grep does not die on invalid UTF-8 value (latin1 + locale + invalid needle)" "
 		LC_ALL=\"$is_IS_locale\" git -c grep.patternType=$engine log --encoding=ISO-8859-1 --format=%s --grep=\"$force_regex$invalid_e\" >actual &&
 		test_must_be_empty actual
 	"

-- 
2.22.0.455.g172b71a6c5
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 66 of 90 in “Support building with GCC v8.x/v9.x”
  1. 0/4 Support building with GCC v8.x/v9.xJohannes Schindelin via GitGitGadget, Jun 13, 2019
  2. 4/4 config: avoid calling `labs()` on too-large data typeJohannes Schindelin via GitGitGadget, Jun 13, 2019
  3. Junio C HamanoJun 13, 2019
  4. René ScharfeJun 16, 2019
  5. René ScharfeJun 16, 2019
  6. René ScharfeJun 16, 2019
  7. Junio C HamanoJun 16, 2019
  8. René ScharfeJun 20, 2019
  9. Junio C HamanoJun 20, 2019
  10. Johannes SchindelinJun 21, 2019
  11. René ScharfeJun 22, 2019
  12. 1/3 config: use unsigned_mult_overflows to check for overflowsRené Scharfe, Jun 22, 2019
  13. 2/3 config: don't multiply in parse_unit_factor()René Scharfe, Jun 22, 2019
  14. 3/3 config: simplify parsing of unit factorsRené Scharfe, Jun 22, 2019
  15. 3/4 winansi: simplify loading the GetCurrentConsoleFontEx() functionJohannes Schindelin via GitGitGadget, Jun 13, 2019
  16. 1/4 poll (mingw): allow compiling with GCC 8 and DEVELOPER=1Johannes Schindelin via GitGitGadget, Jun 13, 2019
  17. 2/4 kwset: allow building with GCC 8Johannes Schindelin via GitGitGadget, Jun 13, 2019
  18. Junio C HamanoJun 13, 2019
  19. SZEDER GáborJun 14, 2019
  20. 0/4 compat/obstack: update from upstreamSZEDER Gábor, Jun 14, 2019
  21. 2/4 SQUASH??? compat/obstack: fix portability issuesSZEDER Gábor, Jun 14, 2019
  22. 4/4 compat/obstack: fix some sparse warningsSZEDER Gábor, Jun 14, 2019
  23. 3/4 SQUASH??? compat/obstack: fix build errors with ClangSZEDER Gábor, Jun 14, 2019
  24. 1/4 compat/obstack: update from upstreamSZEDER Gábor, Jun 14, 2019
  25. Jeff KingJun 14, 2019
  26. Junio C HamanoJun 14, 2019
  27. Ramsay JonesJun 14, 2019
  28. Ramsay JonesJun 14, 2019
  29. SZEDER GáborJun 17, 2019
  30. Junio C HamanoJun 14, 2019
  31. SZEDER GáborJun 17, 2019
  32. Ævar Arnfjörð BjarmasonJun 14, 2019
  33. Can we just get rid of kwset & obstack in favor of optimistically using PCRE v2 JIT?Ævar Arnfjörð Bjarmason, Jun 14, 2019
  34. Ævar Arnfjörð BjarmasonJun 14, 2019
  35. Jeff KingJun 20, 2019
  36. Carlo ArenasJun 15, 2019
  37. brian m. carlsonJun 15, 2019
  38. Ævar Arnfjörð BjarmasonJun 15, 2019
  39. 0/7 grep: move from kwset to optional PCRE v2Ævar Arnfjörð Bjarmason, Jun 26, 2019
  40. Johannes SchindelinJun 26, 2019
  41. Johannes SchindelinJun 27, 2019
  42. Ævar Arnfjörð BjarmasonJun 27, 2019
  43. Johannes SchindelinJun 27, 2019
  44. 1/9 log tests: test regex backends in "--encode=<enc>" testsÆvar Arnfjörð Bjarmason, Jun 27, 2019
  45. 0/9 grep: move from kwset to optional PCRE v2Ævar Arnfjörð Bjarmason, Jun 27, 2019
  46. Ævar Arnfjörð BjarmasonJun 28, 2019
  47. Junio C HamanoJun 28, 2019
  48. 00/10 grep: move from kwset to optional PCRE v2Ævar Arnfjörð Bjarmason, Jul 1, 2019
  49. Junio C HamanoJul 1, 2019
  50. Ævar Arnfjörð BjarmasonJul 2, 2019
  51. Johannes SchindelinJul 2, 2019
  52. Junio C HamanoJul 2, 2019
  53. Johannes SchindelinJul 3, 2019
  54. Johannes SchindelinJul 3, 2019
  55. Johannes SchindelinJul 3, 2019
  56. 01/10 log tests: test regex backends in "--encode=<enc>" testsÆvar Arnfjörð Bjarmason, Jul 1, 2019
  57. 02/10 grep: don't use PCRE2?_UTF8 with "log --encoding=<non-utf8>"Ævar Arnfjörð Bjarmason, Jul 1, 2019
  58. 03/10 t4210: skip more command-line encoding tests on MinGWÆvar Arnfjörð Bjarmason, Jul 1, 2019
  59. 04/10 grep: inline the return value of a function call used only onceÆvar Arnfjörð Bjarmason, Jul 1, 2019
  60. 05/10 grep tests: move "grep binary" alongside the restÆvar Arnfjörð Bjarmason, Jul 1, 2019
  61. 08/10 grep: drop support for \0 in --fixed-strings <pattern>Ævar Arnfjörð Bjarmason, Jul 1, 2019
  62. 07/10 grep: make the behavior for NUL-byte in patterns saneÆvar Arnfjörð Bjarmason, Jul 1, 2019
  63. 06/10 grep tests: move binary pattern tests into their own fileÆvar Arnfjörð Bjarmason, Jul 1, 2019
  64. 09/10 grep: remove the kwset optimizationÆvar Arnfjörð Bjarmason, Jul 1, 2019
  65. 10/10 grep: use PCRE v2 for optimized fixed-string searchÆvar Arnfjörð Bjarmason, Jul 1, 2019
  66. 2/9 grep: don't use PCRE2?_UTF8 with "log --encoding=<non-utf8>"Ævar Arnfjörð Bjarmason, Jun 27, 2019
  67. 3/9 grep: inline the return value of a function call used only onceÆvar Arnfjörð Bjarmason, Jun 27, 2019
  68. 4/9 grep tests: move "grep binary" alongside the restÆvar Arnfjörð Bjarmason, Jun 27, 2019
  69. 5/9 grep tests: move binary pattern tests into their own fileÆvar Arnfjörð Bjarmason, Jun 27, 2019
  70. 6/9 grep: make the behavior for NUL-byte in patterns saneÆvar Arnfjörð Bjarmason, Jun 27, 2019
  71. 7/9 grep: drop support for \0 in --fixed-strings <pattern>Ævar Arnfjörð Bjarmason, Jun 27, 2019
  72. 8/9 grep: remove the kwset optimizationÆvar Arnfjörð Bjarmason, Jun 27, 2019
  73. 9/9 grep: use PCRE v2 for optimized fixed-string searchÆvar Arnfjörð Bjarmason, Jun 27, 2019
  74. 1/7 grep: inline the return value of a function call used only onceÆvar Arnfjörð Bjarmason, Jun 26, 2019
  75. 2/7 grep tests: move "grep binary" alongside the restÆvar Arnfjörð Bjarmason, Jun 26, 2019
  76. Johannes SchindelinJun 26, 2019
  77. Junio C HamanoJun 26, 2019
  78. 4/7 grep: make the behavior for \0 in patterns saneÆvar Arnfjörð Bjarmason, Jun 26, 2019
  79. brian m. carlsonJun 27, 2019
  80. 3/7 grep tests: move binary pattern tests into their own fileÆvar Arnfjörð Bjarmason, Jun 26, 2019
  81. 5/7 grep: drop support for \0 in --fixed-strings <pattern>Ævar Arnfjörð Bjarmason, Jun 26, 2019
  82. Junio C HamanoJun 26, 2019
  83. 6/7 grep: remove the kwset optimizationÆvar Arnfjörð Bjarmason, Jun 26, 2019
  84. 7/7 grep: use PCRE v2 for optimized fixed-string searchÆvar Arnfjörð Bjarmason, Jun 26, 2019
  85. Johannes SchindelinJun 26, 2019
  86. Junio C HamanoJun 26, 2019
  87. Johannes SchindelinJun 27, 2019
  88. Johannes SchindelinJun 27, 2019
  89. Junio C HamanoJun 27, 2019
  90. Johannes SchindelinJun 28, 2019

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.