Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(debugfile): Recognize chunk-uploaded Proguard files #81131

Merged
merged 1 commit into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/sentry/models/debugfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 [
Expand Down
72 changes: 71 additions & 1 deletion tests/sentry/models/test_debugfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,16 @@
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

Expand Down Expand Up @@ -298,3 +304,67 @@ def test_simple_cache_clear(self):

# 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)

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):
with pytest.raises(FileNotFoundError):
# If the file is not detected as a ProGuard file, detect_dif_from_path
# attempts to open the file, which probably doesn't exist.
# Note that if the path or name does exist as a file on the filesystem,
# this test will fail.
detect_dif_from_path(path, name)
Loading