Browse Source

fix(debugfile): Recognize chunk-uploaded Proguard files

The `detect_dif_from_path` function does not correctly identify Proguard files which are chunk uploaded because these files have a randomly-assigned temporary path, and the logic tries to guess whether the file is a Proguard file based on its path. However, the `detect_dif_from_path` function also takes an optional `name` option, which for chunk-uploaded files, is the file name that is specified by the client making the assemble call. This has not been a problem yet, since Sentry CLI does not support chunk uploading Proguard files, but we are adding support in https://github.com/getsentry/sentry-cli/issues/2196, and so we need to fix the backend to allow chunk uploads of Proguard files.

Here, we update the logic to check both the `path` and the `name` for potentially matching a Proguard file.
Daniel Szoke 15 hours ago
parent
commit
6fc3398d8e
2 changed files with 91 additions and 4 deletions
  1. 6 3
      src/sentry/models/debugfile.py
  2. 85 1
      tests/sentry/models/test_debugfile.py

+ 6 - 3
src/sentry/models/debugfile.py

@@ -338,7 +338,10 @@ def create_dif_from_id(
     return dif, True
 
 
-def _analyze_progard_filename(filename: str) -> str | None:
+def _analyze_progard_filename(filename: str | None) -> str | None:
+    if filename is None:
+        return None
+
     match = _proguard_file_re.search(filename)
     if match is None:
         return None
@@ -474,9 +477,9 @@ def detect_dif_from_path(
 
     :raises BadDif: If the file is not a valid DIF.
     """
-    # proguard files (proguard/UUID.txt) or
+    # Proguard files have a path or a name like (proguard/UUID.txt) or
     # (proguard/mapping-UUID.txt).
-    proguard_id = _analyze_progard_filename(path)
+    proguard_id = _analyze_progard_filename(path) or _analyze_progard_filename(name)
     if proguard_id is not None:
         data = {"features": ["mapping"]}
         return [

+ 85 - 1
tests/sentry/models/test_debugfile.py

@@ -6,10 +6,16 @@ import zipfile
 from io import BytesIO
 from typing import Any
 
+import pytest
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.urls import reverse
 
-from sentry.models.debugfile import DifMeta, ProjectDebugFile, create_dif_from_id
+from sentry.models.debugfile import (
+    DifMeta,
+    ProjectDebugFile,
+    create_dif_from_id,
+    detect_dif_from_path,
+)
 from sentry.models.files.file import File
 from sentry.testutils.cases import APITestCase, TestCase
 
@@ -298,3 +304,81 @@ class DebugFilesClearTest(APITestCase):
 
         # But it's gone now
         assert not os.path.isfile(difs[PROGUARD_UUID])
+
+
+@pytest.mark.parametrize(
+    ("path", "name", "uuid"),
+    (
+        (
+            "/proguard/mapping-00000000-0000-0000-0000-000000000000.txt",
+            None,
+            "00000000-0000-0000-0000-000000000000",
+        ),
+        (
+            "/proguard/00000000-0000-0000-0000-000000000000.txt",
+            None,
+            "00000000-0000-0000-0000-000000000000",
+        ),
+        (
+            "/var/folders/x5/zw3gnf_x3ts0dwg56362ftrw0000gn/T/tmpbs2r93sr",
+            "/proguard/mapping-00000000-0000-0000-0000-000000000000.txt",
+            "00000000-0000-0000-0000-000000000000",
+        ),
+        (
+            "/var/folders/x5/zw3gnf_x3ts0dwg56362ftrw0000gn/T/tmpbs2r93sr",
+            "/proguard/00000000-0000-0000-0000-000000000000.txt",
+            "00000000-0000-0000-0000-000000000000",
+        ),
+    ),
+)
+def test_proguard_files_detected(path, name, uuid):
+    # ProGuard files are detected by the path/name, not the file contents.
+    # So, the ProGuard check should not depend on the file existing.
+    detected = detect_dif_from_path(path, name)
+
+    # except FileNotFoundError:
+    #     # If the file is not detected as a ProGuard file, detect_dif_from_path
+    #     # attempts to open the file, which likely doesn't exist.
+    #     # ProGuard files are detected by the path/name, not the file contents.
+    #     # So, the ProGuard check should not depend on the file existing.
+    #     assert not should_detect
+    #     return
+
+    assert len(detected) == 1
+
+    (dif_meta,) = detected
+    assert dif_meta.file_format == "proguard"
+    assert dif_meta.debug_id == uuid
+    assert dif_meta.data == {"features": ["mapping"]}
+
+
+@pytest.mark.parametrize(
+    ("path", "name"),
+    ("/var/folders/x5/zw3gnf_x3ts0dwg56362ftrw0000gn/T/tmpbs2r93sr", None),
+    ("/var/folders/x5/zw3gnf_x3ts0dwg56362ftrw0000gn/T/tmpbs2r93sr", "not-a-proguard-file.txt"),
+    (
+        # Note: "/" missing from beginning of path
+        "proguard/mapping-00000000-0000-0000-0000-000000000000.txt",
+        None,
+    ),
+    (
+        "/var/folders/x5/zw3gnf_x3ts0dwg56362ftrw0000gn/T/tmpbs2r93sr",
+        # Note: "/" missing from beginning of path
+        "proguard/mapping-00000000-0000-0000-0000-000000000000.txt",
+    ),
+)
+def test_proguard_file_not_detected(path, name):
+    try:
+        detected = detect_dif_from_path(path, name)
+    except FileNotFoundError:
+        # If the file is not detected as a ProGuard file, detect_dif_from_path
+        # attempts to open the file, which likely doesn't exist.
+        # We handle this by setting detected to an empty list, which will
+        # cause the test to pass.
+        detected = []
+
+    # On the off chance that the file exists and it was detected as a DIF,
+    # it should never be detected as a ProGuard file, since ProGuard files
+    # are detected by the path/name, not the file contents.
+    for dif_meta in detected:
+        assert dif_meta.file_format != "proguard"