Browse Source

fix(java): Make source context work for obufscated filenames (#63588)

Sometimes proguard/r8 can be used with the following option which will
rename all of the filenames to `SourceFile`:
```
# If you keep the line number information, uncomment this to
# hide the original source file name.
-renamesourcefileattribute SourceFile
```

We were not considering that case, so we could end up with a wrong
source filename, e.g. `io.sentry.samples.SourceFile` which doesn't exist
in the linked source bundle. So this PR simply adds a check the filename
should have a `.` in it, otherwise we consider it obfuscated and fall
back to using the `module` field which will have the proper file path
(by default JVM puts a source filename in this field, so it should
always contain a file extension, see [the
spec](https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7.10))

Closes https://github.com/getsentry/sentry-java/issues/3147
Roman Zavarnitsyn 1 year ago
parent
commit
92b54fd35f
2 changed files with 38 additions and 8 deletions
  1. 4 6
      src/sentry/lang/java/plugin.py
  2. 34 2
      tests/relay_integration/lang/java/test_plugin.py

+ 4 - 6
src/sentry/lang/java/plugin.py

@@ -180,12 +180,13 @@ class JavaSourceLookupStacktraceProcessor(StacktraceProcessor):
             return self.proguard_processor.process_exception(exception)
         return False
 
-    # if path contains a $ sign it has most likely been obfuscated
+    # if path contains a '$' sign or doesn't contain a '.' it has most likely been obfuscated
     def _is_valid_path(self, abs_path):
         if abs_path is None:
             return False
         abs_path_dollar_index = abs_path.find("$")
-        return abs_path_dollar_index < 0
+        abs_path_dot_index = abs_path.find(".")
+        return abs_path_dollar_index < 0 and abs_path_dot_index > 0
 
     def _build_source_file_name(self, frame):
         abs_path = frame.get("abs_path")
@@ -200,10 +201,7 @@ class JavaSourceLookupStacktraceProcessor(StacktraceProcessor):
                 source_file_name = ""
 
             abs_path_dot_index = abs_path.rfind(".")
-            if abs_path_dot_index >= 0:
-                source_file_name += abs_path[:abs_path_dot_index]
-            else:
-                source_file_name += abs_path
+            source_file_name += abs_path[:abs_path_dot_index]
         else:
             # use module as filename (excluding inner classes, marked by $) and append .java
             module_dollar_index = module.find("$")

+ 34 - 2
tests/relay_integration/lang/java/test_plugin.py

@@ -81,6 +81,12 @@ class MainActivity : ComponentActivity() {
 
     class AdditionalInnerClass {
         fun whoops3() {
+            OneMoreInnerClass().whoops4()
+        }
+    }
+
+    class OneMoreInnerClass {
+        fun whoops4() {
             throw RuntimeException("whoops")
         }
     }
@@ -775,6 +781,13 @@ class BasicResolvingIntegrationTest(RelayStoreHelper, TransactionTestCase):
                                     "filename": "MainActivity.kt",
                                     "lineno": 32,
                                 },
+                                {
+                                    "function": "whoops4",
+                                    "abs_path": "SourceFile",
+                                    "module": "io.sentry.samples.MainActivity$OneMoreInnerClass",
+                                    "filename": "SourceFile",
+                                    "lineno": 38,
+                                },
                             ]
                         },
                         "module": "io.sentry.samples",
@@ -883,7 +896,7 @@ class BasicResolvingIntegrationTest(RelayStoreHelper, TransactionTestCase):
         assert frames[5].function == "whoops3"
         assert frames[5].module == "io.sentry.samples.MainActivity$AdditionalInnerClass"
         assert frames[5].lineno == 32
-        assert frames[5].context_line == '            throw RuntimeException("whoops")'
+        assert frames[5].context_line == "            OneMoreInnerClass().whoops4()"
         assert frames[5].pre_context == [
             "        }",
             "    }",
@@ -891,7 +904,26 @@ class BasicResolvingIntegrationTest(RelayStoreHelper, TransactionTestCase):
             "    class AdditionalInnerClass {",
             "        fun whoops3() {",
         ]
-        assert frames[5].post_context == ["        }", "    }", "}", ""]
+        assert frames[5].post_context == [
+            "        }",
+            "    }",
+            "",
+            "    class OneMoreInnerClass {",
+            "        fun whoops4() {",
+        ]
+
+        assert frames[6].function == "whoops4"
+        assert frames[6].module == "io.sentry.samples.MainActivity$OneMoreInnerClass"
+        assert frames[6].lineno == 38
+        assert frames[6].context_line == '            throw RuntimeException("whoops")'
+        assert frames[6].pre_context == [
+            "        }",
+            "    }",
+            "",
+            "    class OneMoreInnerClass {",
+            "        fun whoops4() {",
+        ]
+        assert frames[6].post_context == ["        }", "    }", "}", ""]
 
     def test_source_lookup_with_proguard(self):
         self.upload_proguard_mapping(PROGUARD_SOURCE_LOOKUP_UUID, PROGUARD_SOURCE_LOOKUP_SOURCE)