Subject: bug#18266: grep -P and invalid exits with error

From: Paul Eggert <[email-masked]>
Date: Tue, 09 Sep 2014 12:59:27 -0700

Norihiro Tanaka wrote:
> I'm worried that to re-run for invalid UTF-8 makes slowness for searching
> of the large number of binary files.

Yes, that could be a problem, but even so it's better for grep to report 
matches than to give up and fail.  Perhaps someone could optimize this 
better later, but to be honest given how flaky libpcre is we're probably 
better off spending our scarce development resources elsewhere.

Santiago's latest patch still had some troubles, unfortunately.  It 
could mishandle '^' by having it match just past an encoding error.  It 
was less efficient than it could be, as it checked all valid bytes for 
UTF-8-edness twice.  If I understand PCRE correctly (which quite 
possibly I don't), it also appeared to mishandle matches that contain 
nested subexpressions.  But the worst part was that the code was too 
complicated (and this was true even before Santiago's patch was 
applied).  So I rewrote it and installed the attached patch instead. 
Please give it a try.
From 29855e7bbe47b91680ae0cba5729c5becfaa3216 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Tue, 9 Sep 2014 12:41:54 -0700
Subject: [PATCH] grep: -P now treats invalid UTF-8 input as non-matching

Problem reported by Santiago Vila in: http://bugs.gnu.org/18266
* NEWS: Mention this.
* src/pcresearch.c (Pexecute): Treat UTF-8 encoding errors
as non-matching data, instead of exiting 'grep'.
* tests/pcre-infloop: grep now exits with status 1, not 2.
* tests/pcre-invalid-utf8-input: grep now exits with status 0, not 2.
---
 NEWS                          |  3 ++
 src/pcresearch.c              | 70 +++++++++++++++++--------------------------
 tests/pcre-infloop            |  2 +-
 tests/pcre-invalid-utf8-input |  2 +-
 4 files changed, 33 insertions(+), 44 deletions(-)

diff --git a/NEWS b/NEWS
index 550bf4c..ca79525 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,9 @@ GNU grep NEWS                                    -*- outline -*-
 
   Performance has improved for very long strings in patterns.
 
+  grep -P no longer reports an error and exits when given invalid UTF-8 data.
+  Instead, it considers the data to be non-matching.
+
 ** Bug fixes
 
   grep -E rejected unmatched ')', instead of treating it like '\)'.
diff --git a/src/pcresearch.c b/src/pcresearch.c
index 820dd00..2a01e6d 100644
--- a/src/pcresearch.c
+++ b/src/pcresearch.c
@@ -136,34 +136,41 @@ Pexecute (char const *buf, size_t size, size_t *match_size,
 #else
   /* This array must have at least two elements; everything after that
      is just for performance improvement in pcre_exec.  */
-  int sub[300];
+  enum { nsub = 300 };
+  int sub[nsub];
 
-  const char *line_buf, *line_end, *line_next;
+  char const *p = start_ptr ? start_ptr : buf;
+  int options = p == buf || p[-1] == eolbyte ? 0 : PCRE_NOTBOL;
+  char const *line_start = buf;
   int e = PCRE_ERROR_NOMATCH;
-  ptrdiff_t start_ofs = start_ptr ? start_ptr - buf : 0;
+  char const *line_end;
 
   /* PCRE can't limit the matching to single lines, therefore we have to
      match each line in the buffer separately.  */
-  for (line_next = buf;
-       e == PCRE_ERROR_NOMATCH && line_next < buf + size;
-       start_ofs -= line_next - line_buf)
+  for (; p < buf + size; p = line_start = line_end + 1)
     {
-      line_buf = line_next;
-      line_end = memchr (line_buf, eolbyte, (buf + size) - line_buf);
-      if (line_end == NULL)
-        line_next = line_end = buf + size;
-      else
-        line_next = line_end + 1;
-
-      if (start_ptr && start_ptr >= line_end)
-        continue;
+      line_end = memchr (p, eolbyte, buf + size - p);
 
-      if (INT_MAX < line_end - line_buf)
+      if (INT_MAX < line_end - p)
         error (EXIT_TROUBLE, 0, _("exceeded PCRE's line length limit"));
 
-      e = pcre_exec (cre, extra, line_buf, line_end - line_buf,
-                     start_ofs < 0 ? 0 : start_ofs, 0,
-                     sub, sizeof sub / sizeof *sub);
+      /* Treat encoding-error bytes as data that cannot match.  */
+      for (;;)
+        {
+          e = pcre_exec (cre, extra, p, line_end - p, 0, options, sub, nsub);
+          if (e != PCRE_ERROR_BADUTF8)
+            break;
+          e = pcre_exec (cre, extra, p, sub[0], 0,
+                         options | PCRE_NO_UTF8_CHECK, sub, nsub);
+          if (e != PCRE_ERROR_NOMATCH)
+            break;
+          p += sub[0] + 1;
+          options = PCRE_NOTBOL;
+        }
+
+      if (e != PCRE_ERROR_NOMATCH)
+        break;
+      options = 0;
     }
 
   if (e <= 0)
@@ -180,10 +187,6 @@ Pexecute (char const *buf, size_t size, size_t *match_size,
           error (EXIT_TROUBLE, 0,
                  _("exceeded PCRE's backtracking limit"));
 
-        case PCRE_ERROR_BADUTF8:
-          error (EXIT_TROUBLE, 0,
-                 _("invalid UTF-8 byte sequence in input"));
-
         default:
           /* For now, we lump all remaining PCRE failures into this basket.
              If anyone cares to provide sample grep usage that can trigger
@@ -197,25 +200,8 @@ Pexecute (char const *buf, size_t size, size_t *match_size,
     }
   else
     {
-      /* Narrow down to the line we've found.  */
-      char const *beg = line_buf + sub[0];
-      char const *end = line_buf + sub[1];
-      char const *buflim = buf + size;
-      char eol = eolbyte;
-      if (!start_ptr)
-        {
-          /* FIXME: The case when '\n' is not found indicates a bug:
-             Since grep is line oriented, the match should never contain
-             a newline, so there _must_ be a newline following.
-           */
-          if (!(end = memchr (end, eol, buflim - end)))
-            end = buflim;
-          else
-            end++;
-          while (buf < beg && beg[-1] != eol)
-            --beg;
-        }
-
+      char const *beg = start_ptr ? p + sub[0] : line_start;
+      char const *end = start_ptr ? p + sub[1] : line_end + 1;
       *match_size = end - beg;
       return beg - buf;
     }
diff --git a/tests/pcre-infloop b/tests/pcre-infloop
index 1b33e72..b92f8e1 100755
--- a/tests/pcre-infloop
+++ b/tests/pcre-infloop
@@ -28,6 +28,6 @@ printf 'a\201b\r' > in || framework_failure_
 fail=0
 
 LC_ALL=en_US.UTF-8 timeout 3 grep -P 'a.?..b' in
-test $? = 2 || fail_ "libpcre's match function appears to infloop"
+test $? = 1 || fail_ "libpcre's match function appears to infloop"
 
 Exit $fail
diff --git a/tests/pcre-invalid-utf8-input b/tests/pcre-invalid-utf8-input
index 913e8ee..f42e0dd 100755
--- a/tests/pcre-invalid-utf8-input
+++ b/tests/pcre-invalid-utf8-input
@@ -16,6 +16,6 @@ fail=0
 printf 'j\202\nj\n' > in || framework_failure_
 
 LC_ALL=en_US.UTF-8 grep -P j in
-test $? -eq 2 || fail=1
+test $? -eq 0 || fail=1
 
 Exit $fail
-- 
1.9.3