grep: don't use PCRE2?_UTF8 with "log --encoding=<non-utf8>"

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Jun 28, 2019 at 01:39 UTC 44570188a0e324048decf06b845d34c45b08a4fa
4 files changed +10 -8
grep.c
+4 -4
@@ -388,11 +388,11 @@ static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt)
388 int options = PCRE_MULTILINE;
389
390 if (opt->ignore_case) {
391 - if (has_non_ascii(p->pattern))
391 + if (!opt->ignore_locale && has_non_ascii(p->pattern))
392 p->pcre1_tables = pcre_maketables();
393 options |= PCRE_CASELESS;
394 }
395 - if (is_utf8_locale() && has_non_ascii(p->pattern))
395 + if (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern))
396 options |= PCRE_UTF8;
397
398 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
498 p->pcre2_compile_context = NULL;
499
500 if (opt->ignore_case) {
501 - if (has_non_ascii(p->pattern)) {
501 + if (!opt->ignore_locale && has_non_ascii(p->pattern)) {
502 character_tables = pcre2_maketables(NULL);
503 p->pcre2_compile_context = pcre2_compile_context_create(NULL);
504 pcre2_set_character_tables(p->pcre2_compile_context, character_tables);
505 }
506 options |= PCRE2_CASELESS;
507 }
508 - if (is_utf8_locale() && has_non_ascii(p->pattern))
508 + if (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern))
509 options |= PCRE2_UTF;
510
511 p->pcre2_pattern = pcre2_compile((PCRE2_SPTR)p->pattern,
grep.h
+1
@@ -173,6 +173,7 @@ struct grep_opt {
173 int funcbody;
174 int extended_regexp_option;
175 int pattern_type_option;
176 + int ignore_locale;
177 char colors[NR_GREP_COLORS][COLOR_MAXLEN];
178 unsigned pre_context;
179 unsigned post_context;
revision.c
+3
@@ -28,6 +28,7 @@
28 #include "commit-graph.h"
29 #include "prio-queue.h"
30 #include "hashmap.h"
31 +#include "utf8.h"
32
33 volatile show_early_output_fn_t show_early_output;
34
@@ -2655,6 +2656,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
2656
2657 grep_commit_pattern_type(GREP_PATTERN_TYPE_UNSPECIFIED,
2658 &revs->grep_filter);
2659 + if (!is_encoding_utf8(get_log_output_encoding()))
2660 + revs->grep_filter.ignore_locale = 1;
2661 compile_grep_patterns(&revs->grep_filter);
2662
2663 if (revs->reverse && revs->reflog_info)
t/t4210-log-i18n.sh
+2 -4
@@ -59,10 +59,8 @@ test_expect_success 'log --grep does not find non-reencoded values (latin1)' '
59 for engine in fixed basic extended perl
60 do
61 prereq=
62 - result=success
62 if test $engine = "perl"
63 then
65 - result=failure
64 prereq="PCRE"
65 else
66 prereq=""
@@ -72,7 +70,7 @@ do
70 then
71 force_regex=.*
72 fi
75 - test_expect_$result GETTEXT_LOCALE,$prereq "-c grep.patternType=$engine log --grep does not find non-reencoded values (latin1 + locale)" "
73 + test_expect_success GETTEXT_LOCALE,$prereq "-c grep.patternType=$engine log --grep does not find non-reencoded values (latin1 + locale)" "
74 cat >expect <<-\EOF &&
75 latin1
76 utf8
@@ -86,7 +84,7 @@ do
84 test_must_be_empty actual
85 "
86
89 - test_expect_$result GETTEXT_LOCALE,$prereq "-c grep.patternType=$engine log --grep does not die on invalid UTF-8 value (latin1 + locale + invalid needle)" "
87 + test_expect_success GETTEXT_LOCALE,$prereq "-c grep.patternType=$engine log --grep does not die on invalid UTF-8 value (latin1 + locale + invalid needle)" "
88 LC_ALL=\"$is_IS_locale\" git -c grep.patternType=$engine log --encoding=ISO-8859-1 --format=%s --grep=\"$force_regex$invalid_e\" >actual &&
89 test_must_be_empty actual
90 "