PiiElider: apply to multi-line messages. Simplify how frames are identified.

Makes sanitizeStacktrace() assume that all stack frames have "\tat"
prefix.

Adds a URL exception for common storage paths (e.g. /data/data)

Bug: 1267770
Change-Id: I6f2e827acdee601b893ab1c843f6e3e8cea6f07f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5075138
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Mohamed Heikal <mheikal@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1238115}

base/android/java/src/org/chromium/base/PiiElider.java

@@ -75,23 +75,15 @@ public class PiiElider {
                             + "*)?" // Rest of the URI path.
                             + "(\\b|$)"); // Always end on a word boundary or end of string.
 
-    // Example variant info chromium-TrichromeChromeGoogle6432.aab
-    private static final String CHROME_VARIANT_INFO = "chromium-[^\\.]+\\.aab";
-    private static final Pattern LIKELY_EXCEPTION_LOG =
+    private static final Pattern NOT_URLS_PATTERN =
             Pattern.compile(
-                    "\\sat\\s"
-                            // These are all package prefixes of classes that are likely to
-                            // exist on a stacktrace and are very unlikely to be a PII url.
-                            + "(org\\.chromium|com\\.google|java|android|com\\.android)\\.[^ ]+.|"
-                            // if a line has what looks like line number info, it's probably an
-                            // exception log.
-                            + "\\("
-                            + CHROME_VARIANT_INFO
-                            + "[^:]+:\\d+\\)|"
+                    ""
                             // When a class is not found it can fail to satisfy our isClass
                             // check but is still worth noting what it was.
-                            + "Caused by: java\\.lang\\."
-                            + "(ClassNotFoundException|NoClassDefFoundError):");
+                            + "^(?:Caused by: )?java\\.lang\\."
+                            + "(?:ClassNotFoundException|NoClassDefFoundError):|"
+                            // Ensure common local paths are not interpreted as URLs.
+                            + "(?:[\"' ]/(?:apex|data|mnt|proc|sdcard|storage|system))/");
 
     private static final String IP_ELISION = "1.2.3.4";
     private static final String MAC_ELISION = "01:23:45:67:89:AB";
@@ -140,7 +132,7 @@ public class PiiElider {
      */
     public static String elideUrl(String original) {
         // Url-matching is fussy. If something looks like an exception message, just return.
-        if (LIKELY_EXCEPTION_LOG.matcher(original).find()) return original;
+        if (NOT_URLS_PATTERN.matcher(original).find()) return original;
         StringBuilder buffer = new StringBuilder(original);
         Matcher matcher = WEB_URL.matcher(buffer);
         int start = 0;
@@ -242,15 +234,19 @@ public class PiiElider {
         if (TextUtils.isEmpty(stacktrace)) {
             return "";
         }
-        String[] frames = stacktrace.split("\\n");
-        // Sanitize first stacktrace line which contains the exception message.
-        frames[0] = elideUrl(frames[0]);
-        for (int i = 1; i < frames.length; i++) {
-            // Nested exceptions should also have their message sanitized.
-            if (frames[i].startsWith("Caused by:")) {
-                frames[i] = elideUrl(frames[i]);
+        String[] lines = stacktrace.split("\\n");
+        boolean foundAtLine = false;
+        for (int i = 0; i < lines.length; i++) {
+            if (lines[i].startsWith("\tat ")) {
+                foundAtLine = true;
+            } else {
+                lines[i] = elideUrl(lines[i]);
             }
         }
-        return TextUtils.join("\n", frames);
+        // Guard against non-properly formatted stacktraces to ensure checking for "\tat :" is
+        // sufficient (e.g.: no logging-related line prefixes).
+        // There can be no frames when a native thread creates an exception using JNI.
+        assert foundAtLine || lines.length == 1 : "Was not a stack trace: " + stacktrace;
+        return TextUtils.join("\n", lines);
     }
 }

base/android/junit/src/org/chromium/base/PiiEliderTest.java

@@ -72,22 +72,6 @@ public class PiiEliderTest {
         assertEquals(original, PiiElider.elideUrl(original));
     }
 
-    @Test
-    public void testElideUrl8() {
-        String original =
-                "exception at org.chromium.chrome.browser.compositor.scene_layer."
-                        + "TabListSceneLayer.nativeUpdateLayer(Native Method)";
-        assertEquals(original, PiiElider.elideUrl(original));
-    }
-
-    @Test
-    public void testElideUrl9() {
-        String original =
-                "I/dalvikvm( 5083): at org.chromium.chrome.browser.compositor."
-                        + "scene_layer.TabListSceneLayer.nativeUpdateLayer(Native Method)";
-        assertEquals(original, PiiElider.elideUrl(original));
-    }
-
     @Test
     public void testElideUrl10() {
         String original =
@@ -109,14 +93,6 @@ public class PiiEliderTest {
         assertEquals(original, PiiElider.elideUrl(original));
     }
 
-    @Test
-    public void testElideUrl12() {
-        String original =
-                "System.err: at kH.onAnimationEnd"
-                        + "(chromium-TrichromeChromeGoogle6432.aab-canary-530200034:42)";
-        assertEquals(original, PiiElider.elideUrl(original));
-    }
-
     @Test
     public void testElideNonHttpUrl() {
         String original = "test some-other-scheme://address/01010?param=33&other_param=AAA !!!";
@@ -130,6 +106,15 @@ public class PiiEliderTest {
         assertEquals(original, PiiElider.elideUrl(original));
     }
 
+    @Test
+    public void testDontElideFilePaths() {
+        String original =
+                """
+            dlopen failed: library "/data/app/com.chrome.dev-Lo4Mduh0dhPARVPBiAM_ag==/Chrome.apk!/\
+            lib/arm64-v8a/libelements.so" not found""";
+        assertEquals(original, PiiElider.elideUrl(original));
+    }
+
     @Test
     public void testDontElideAndroidPermission() {
         String original =
@@ -170,23 +155,25 @@ public class PiiEliderTest {
     public void testElideUrlInStacktrace() {
         String original =
                 "java.lang.RuntimeException: Outer Exception crbug.com/12345\n"
-                    + "  at org.chromium.base.PiiElider.sanitizeStacktrace (PiiElider.java:120)\n"
-                    + "Caused by: java.lang.NullPointerException: Inner Exception"
+                    + "\tat org.chromium.base.PiiElider.sanitizeStacktrace (PiiElider.java:120)\n"
+                    + "Caused by: java.lang.NullPointerException: Inner Exception\n"
                     + " shorturl.com/bxyj5";
         String expected =
                 "java.lang.RuntimeException: Outer Exception HTTP://WEBADDRESS.ELIDED\n"
-                    + "  at org.chromium.base.PiiElider.sanitizeStacktrace (PiiElider.java:120)\n"
-                    + "Caused by: java.lang.NullPointerException: Inner Exception "
-                    + "HTTP://WEBADDRESS.ELIDED";
+                    + "\tat org.chromium.base.PiiElider.sanitizeStacktrace (PiiElider.java:120)\n"
+                    + "Caused by: java.lang.NullPointerException: Inner Exception\n"
+                    + " HTTP://WEBADDRESS.ELIDED";
         assertEquals(expected, PiiElider.sanitizeStacktrace(original));
     }
 
     @Test
     public void testDoesNotElideMethodNameInStacktrace() {
         String original =
-                "java.lang.NullPointerException: Attempt to invoke virtual method 'int"
-                    + " org.robolectric.internal.AndroidSandbox.getBackStackEntryCount()' on a null"
-                    + " object reference";
+                """
+                java.lang.NullPointerException: Attempt to invoke virtual method 'int \
+                org.robolectric.internal.AndroidSandbox.getBackStackEntryCount()' on a null \
+                object reference
+                \tat ...""";
         assertEquals(original, PiiElider.sanitizeStacktrace(original));
     }
 }