From 6744828dc4621a33691b054fd9ca23b282134ed0 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 18 Jun 2019 11:33:04 +0200 Subject: [PATCH 1/5] feat(assemble): Permit uploads of PDBs and PEs --- src/sentry/api/endpoints/chunk.py | 1 + src/sentry/constants.py | 2 ++ src/sentry/models/debugfile.py | 15 ++++++++++++--- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/sentry/api/endpoints/chunk.py b/src/sentry/api/endpoints/chunk.py index ee5b719248d9d6..ca1a5a8294ebaa 100644 --- a/src/sentry/api/endpoints/chunk.py +++ b/src/sentry/api/endpoints/chunk.py @@ -28,6 +28,7 @@ CHUNK_UPLOAD_ACCEPT = ( 'debug_files', # DIF assemble 'release_files', # Artifacts assemble + 'pdbs', # PDB upload and debug id override ) diff --git a/src/sentry/constants.py b/src/sentry/constants.py index 4edfb32c3869fc..ca959e4c86d0be 100644 --- a/src/sentry/constants.py +++ b/src/sentry/constants.py @@ -229,6 +229,8 @@ def get_all_languages(): 'text/x-breakpad': 'breakpad', 'application/x-mach-binary': 'macho', 'application/x-elf-binary': 'elf', + 'application/x-dosexec': 'pe', + 'application/x-ms-pdb': 'pdb', 'text/x-proguard+plain': 'proguard', } diff --git a/src/sentry/models/debugfile.py b/src/sentry/models/debugfile.py index c6d352fb52f7db..aed148a6d6491e 100644 --- a/src/sentry/models/debugfile.py +++ b/src/sentry/models/debugfile.py @@ -139,16 +139,25 @@ def file_format(self): ct = self.file.headers.get('Content-Type', 'unknown').lower() return KNOWN_DIF_FORMATS.get(ct, 'unknown') + @property + def file_type(self): + if self.data: + return self.data.get('type') + @property def file_extension(self): if self.file_format == 'breakpad': return '.sym' if self.file_format == 'macho': - return '.dSYM' + return '' if self.file_type == 'exe' else '.dSYM' if self.file_format == 'proguard': return '.txt' if self.file_format == 'elf': - return '.debug' + return '' if self.file_type == 'exe' else '.debug' + if self.file_format == 'pe': + return '.exe' if self.file_type == 'exe' else '.dll' + if self.file_format == 'pdb': + return '.pdb' return '' @@ -189,7 +198,7 @@ def create_dif_from_id(project, meta, fileobj=None, file=None): """ if meta.file_format == 'proguard': object_name = 'proguard-mapping' - elif meta.file_format in ('macho', 'elf'): + elif meta.file_format in ('macho', 'elf', 'pdb', 'pe'): object_name = meta.name elif meta.file_format == 'breakpad': object_name = meta.name[:-4] if meta.name.endswith('.sym') else meta.name From e2014831bbd8a6214ec0650c18436414c0050de7 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 18 Jun 2019 11:33:47 +0200 Subject: [PATCH 2/5] feat(assemble): Permit debugid age overrides --- src/sentry/api/endpoints/debug_files.py | 5 ++++- src/sentry/models/debugfile.py | 23 ++++++++++++++++++----- src/sentry/tasks/assemble.py | 8 ++++++-- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/sentry/api/endpoints/debug_files.py b/src/sentry/api/endpoints/debug_files.py index 5d12357071e027..03ea628e36732a 100644 --- a/src/sentry/api/endpoints/debug_files.py +++ b/src/sentry/api/endpoints/debug_files.py @@ -251,6 +251,7 @@ def post(self, request, project): "required": ["name", "chunks"], "properties": { "name": {"type": "string"}, + "debug_id": {"type": "string"}, "chunks": { "type": "array", "items": { @@ -259,7 +260,7 @@ def post(self, request, project): } } }, - "additionalProperties": False + "additionalProperties": True } }, "additionalProperties": False @@ -279,6 +280,7 @@ def post(self, request, project): for checksum, file_to_assemble in six.iteritems(files): name = file_to_assemble.get('name', None) + debug_id = file_to_assemble.get('debug_id', None) chunks = file_to_assemble.get('chunks', []) # First, check the cached assemble status. During assembling, a @@ -348,6 +350,7 @@ def post(self, request, project): kwargs={ 'project_id': project.id, 'name': name, + 'debug_id': debug_id, 'checksum': checksum, 'chunks': chunks, } diff --git a/src/sentry/models/debugfile.py b/src/sentry/models/debugfile.py index aed148a6d6491e..a158de5aabc75b 100644 --- a/src/sentry/models/debugfile.py +++ b/src/sentry/models/debugfile.py @@ -21,7 +21,7 @@ from django.db import models -from symbolic import Archive, SymbolicError, ObjectErrorUnsupportedObject +from symbolic import Archive, SymbolicError, ObjectErrorUnsupportedObject, normalize_debug_id from sentry import options from sentry.constants import KNOWN_DIF_FORMATS @@ -293,11 +293,24 @@ def __init__(self, file_format, arch, debug_id, path, code_id=None, name=None, d self.name = os.path.basename(path) @classmethod - def from_object(cls, obj, path, name=None): + def from_object(cls, obj, path, name=None, debug_id=None): + if debug_id is not None: + try: + debug_id = normalize_debug_id(debug_id) + except SymbolicError: + debug_id = None + + # Only allow overrides in the debug_id's age if the rest of the debug id + # matches with what we determine from the object file. We generally + # trust the server more than the client. + obj_id = obj.debug_id + if obj_id and debug_id and obj_id[:36] == debug_id[:36]: + obj_id = debug_id + return cls( file_format=obj.file_format, arch=obj.arch, - debug_id=obj.debug_id, + debug_id=obj_id, code_id=obj.code_id, path=path, # TODO: Extract the object name from the object @@ -313,7 +326,7 @@ def basename(self): return os.path.basename(self.path) -def detect_dif_from_path(path, name=None): +def detect_dif_from_path(path, name=None, debug_id=None): """This detects which kind of dif(Debug Information File) the path provided is. It returns an array since an Archive can contain more than one Object. @@ -344,7 +357,7 @@ def detect_dif_from_path(path, name=None): else: objs = [] for obj in archive.iter_objects(): - objs.append(DifMeta.from_object(obj, path, name=name)) + objs.append(DifMeta.from_object(obj, path, name=name, debug_id=debug_id)) return objs diff --git a/src/sentry/tasks/assemble.py b/src/sentry/tasks/assemble.py index f7aa9c44a3b300..ed703a652c1407 100644 --- a/src/sentry/tasks/assemble.py +++ b/src/sentry/tasks/assemble.py @@ -79,7 +79,7 @@ def set_assemble_status(task, scope, checksum, state, detail=None): @instrumented_task(name='sentry.tasks.assemble.assemble_dif', queue='assemble') -def assemble_dif(project_id, name, checksum, chunks, **kwargs): +def assemble_dif(project_id, name, debug_id, checksum, chunks, **kwargs): """ Assembles uploaded chunks into a ``ProjectDebugFile``. """ @@ -109,7 +109,11 @@ def assemble_dif(project_id, name, checksum, chunks, **kwargs): # We only permit split difs to hit this endpoint. The # client is required to split them up first or we error. try: - result = debugfile.detect_dif_from_path(temp_file.name, name=name) + result = debugfile.detect_dif_from_path( + temp_file.name, + name=name, + debug_id=debug_id, + ) except BadDif as e: set_assemble_status(AssembleTask.DIF, project.id, checksum, ChunkFileState.ERROR, detail=e.args[0]) From 64837abf346bafef53263a4504f2084db5e019f3 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 18 Jun 2019 13:47:56 +0200 Subject: [PATCH 3/5] fix(assemble): Make assemble_dif backward compatible --- src/sentry/tasks/assemble.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/tasks/assemble.py b/src/sentry/tasks/assemble.py index ed703a652c1407..db383fc304ca86 100644 --- a/src/sentry/tasks/assemble.py +++ b/src/sentry/tasks/assemble.py @@ -79,7 +79,7 @@ def set_assemble_status(task, scope, checksum, state, detail=None): @instrumented_task(name='sentry.tasks.assemble.assemble_dif', queue='assemble') -def assemble_dif(project_id, name, debug_id, checksum, chunks, **kwargs): +def assemble_dif(project_id, name, checksum, chunks, debug_id=None, **kwargs): """ Assembles uploaded chunks into a ``ProjectDebugFile``. """ From ae9a80d732718d802c263a449e74509a20d2bdae Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 18 Jun 2019 13:50:39 +0200 Subject: [PATCH 4/5] test(assemble): Add a test for debug id overrides --- tests/sentry/tasks/test_assemble.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/sentry/tasks/test_assemble.py b/tests/sentry/tasks/test_assemble.py index 808078f7b0f6ac..5d3560310f52e5 100644 --- a/tests/sentry/tasks/test_assemble.py +++ b/tests/sentry/tasks/test_assemble.py @@ -151,6 +151,30 @@ def test_assemble_duplicate_blobs(self): assert f.checksum == file_checksum.hexdigest() assert f.type == 'dummy.type' + def test_assemble_debug_id_override(self): + sym_file = self.load_fixture('crash.sym') + blob1 = FileBlob.from_file(ContentFile(sym_file)) + total_checksum = sha1(sym_file).hexdigest() + + assemble_dif( + project_id=self.project.id, + name='crash.sym', + checksum=total_checksum, + chunks=[blob1.checksum], + debug_id='67e9247c-814e-392b-a027-dbde6748fcbf-beef' + ) + + status, _ = get_assemble_status(AssembleTask.DIF, self.project.id, total_checksum) + assert status == ChunkFileState.OK + + dif = ProjectDebugFile.objects.filter( + project=self.project, + file__checksum=total_checksum, + ).get() + + assert dif.file.headers == {'Content-Type': 'text/x-breakpad'} + assert dif.debug_id == '67e9247c-814e-392b-a027-dbde6748fcbf-beef' + class AssembleArtifactsTest(BaseAssembleTest): def setUp(self): From 90d18cf944305b767358c63f5647eea10a72aec5 Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Tue, 18 Jun 2019 14:58:16 +0200 Subject: [PATCH 5/5] test(assemble): Fix another test --- tests/sentry/api/endpoints/test_dif_assemble.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/sentry/api/endpoints/test_dif_assemble.py b/tests/sentry/api/endpoints/test_dif_assemble.py index 2ed33d8e76d2b4..2217a80962acfa 100644 --- a/tests/sentry/api/endpoints/test_dif_assemble.py +++ b/tests/sentry/api/endpoints/test_dif_assemble.py @@ -252,6 +252,7 @@ def test_assemble(self, mock_assemble_dif): 'name': 'test', 'chunks': chunks, 'checksum': total_checksum, + 'debug_id': None, } )