diff --git a/src/mvt/android/artifacts/tombstone_crashes.py b/src/mvt/android/artifacts/tombstone_crashes.py index bcfb389..d22e65a 100644 --- a/src/mvt/android/artifacts/tombstone_crashes.py +++ b/src/mvt/android/artifacts/tombstone_crashes.py @@ -131,6 +131,13 @@ class TombstoneCrashArtifact(AndroidArtifact): self, file_name: str, file_timestamp: datetime.datetime, data: bytes ) -> None: """Parse Android tombstone crash files from a protobuf object.""" + self.results.append(self.parse_protobuf_record(file_name, file_timestamp, data)) + + def parse_protobuf_record( + self, file_name: str, file_timestamp: datetime.datetime, data: bytes + ) -> dict: + if not data: + raise ValueError("empty protobuf tombstone") tombstone_pb = Tombstone().parse(data) tombstone_dict = tombstone_pb.to_dict( casing=betterproto2.Casing.SNAKE, include_default_values=True @@ -143,20 +150,31 @@ class TombstoneCrashArtifact(AndroidArtifact): tombstone_dict["file_name"] = file_name tombstone_dict["file_timestamp"] = convert_datetime_to_iso(file_timestamp) tombstone_dict["process_name"] = self._proccess_name_from_thread(tombstone_dict) + if isinstance(tombstone_dict.get("selinux_label"), str): + tombstone_dict["selinux_label"] = tombstone_dict["selinux_label"].rstrip( + "\x00" + ) # Confirm the tombstone is valid, and matches the output model tombstone = TombstoneCrashResult.model_validate(tombstone_dict) - self.results.append(tombstone.model_dump()) + return tombstone.model_dump() def parse( self, file_name: str, file_timestamp: datetime.datetime, content: bytes ) -> None: """Parse text Android tombstone crash files.""" + self.results.append(self.parse_text_record(file_name, file_timestamp, content)) + + def parse_text_record( + self, file_name: str, file_timestamp: datetime.datetime, content: bytes + ) -> dict: + if not content: + raise ValueError("empty plaintext tombstone") tombstone_dict = { "file_name": file_name, "file_timestamp": convert_datetime_to_iso(file_timestamp), } - lines = content.decode("utf-8").splitlines() + lines = content.decode("utf-8", errors="replace").splitlines() for line_num, line in enumerate(lines, 1): if not line.strip() or TOMBSTONE_DELIMITER in line: continue @@ -171,7 +189,7 @@ class TombstoneCrashArtifact(AndroidArtifact): # Validate the tombstone and add it to the results tombstone = TombstoneCrashResult.model_validate(tombstone_dict) - self.results.append(tombstone.model_dump()) + return tombstone.model_dump() def _parse_tombstone_line( self, line: str, key: str, destination_key: str, tombstone: dict @@ -195,7 +213,9 @@ class TombstoneCrashArtifact(AndroidArtifact): if line_key != key: raise ValueError(f"Expected key {key}, got {line_key}") - value_clean = value.strip().strip("'") + value_clean = value.strip() + if len(value_clean) >= 2 and value_clean[0] == value_clean[-1] == "'": + value_clean = value_clean[1:-1] if destination_key == "uid": tombstone[destination_key] = int(value_clean) elif destination_key == "process_uptime": @@ -269,9 +289,7 @@ class TombstoneCrashArtifact(AndroidArtifact): @staticmethod def _parse_timestamp_string(timestamp: str) -> str: timestamp_parsed = parser.parse(timestamp) - # Preserve the source wall-clock time while returning the project-wide ISO format. - local_timestamp = timestamp_parsed.replace(tzinfo=datetime.timezone.utc) - return convert_datetime_to_iso(local_timestamp) + return convert_datetime_to_iso(timestamp_parsed) @staticmethod def _proccess_name_from_thread(tombstone_dict: dict) -> str: diff --git a/src/mvt/android/modules/bugreport/tombstones.py b/src/mvt/android/modules/bugreport/tombstones.py index c4a7afb..4d121a1 100644 --- a/src/mvt/android/modules/bugreport/tombstones.py +++ b/src/mvt/android/modules/bugreport/tombstones.py @@ -43,21 +43,76 @@ class Tombstones(TombstoneCrashArtifact, BugReportModule): ) return - for tombstone_file in sorted(tombstone_files): - tombstone_filename = tombstone_file.split("/")[-1] - modification_time = self._get_file_modification_time(tombstone_file) - tombstone_data = self._get_file_content(tombstone_file) + grouped: dict[str, dict[str, str]] = {} + for file_path in tombstone_files: + file_name = file_path.rsplit("/", 1)[-1] + source = "protobuf" if file_name.endswith(".pb") else "text" + crash_id = file_name.removesuffix(".pb") + grouped.setdefault(crash_id, {})[source] = file_path - try: - if tombstone_file.endswith(".pb"): - self.parse_protobuf( - tombstone_filename, modification_time, tombstone_data + for crash_id, paths in sorted(grouped.items()): + parsed_sources: dict[str, dict] = {} + source_records: dict[str, dict] = {} + for source in ("text", "protobuf"): + file_path = paths.get(source) + if file_path is None: + continue + file_name = file_path.rsplit("/", 1)[-1] + file_timestamp = self._get_file_modification_time(file_path) + source_info = { + "file_name": file_name, + "file_timestamp": file_timestamp.isoformat(), + "parsed": False, + "error": None, + "record": None, + } + try: + data = self._get_file_content(file_path) + if source == "protobuf": + record = self.parse_protobuf_record( + file_name, file_timestamp, data + ) + else: + record = self.parse_text_record(file_name, file_timestamp, data) + source_info["parsed"] = True + source_info["record"] = record + source_records[source] = record + except Exception as exc: + source_info["error"] = str(exc) + self.log.error( + "Error parsing tombstone file %s: %s", file_path, exc ) - else: - self.parse(tombstone_filename, modification_time, tombstone_data) - except ValueError as e: - # Catch any exceptions raised during parsing or validation. - self.log.error(f"Error parsing tombstone file {tombstone_file}: {e}") + parsed_sources[source] = source_info + + if not source_records: + continue + preferred = source_records.get("protobuf") or source_records["text"] + canonical = dict(preferred) + text_record = source_records.get("text") + if text_record: + for key, value in text_record.items(): + if canonical.get(key) in (None, "", [], {}): + canonical[key] = value + + differences = {} + protobuf_record = source_records.get("protobuf") + if text_record and protobuf_record: + for key in text_record.keys() & protobuf_record.keys(): + if key in ("file_name", "file_timestamp"): + continue + if text_record[key] != protobuf_record[key]: + differences[key] = { + "text": text_record[key], + "protobuf": protobuf_record[key], + } + canonical.update( + { + "crash_id": crash_id, + "sources": parsed_sources, + "differences": differences, + } + ) + self.results.append(canonical) self.log.info( "Extracted a total of %d tombstone files", diff --git a/tests/android/test_artifact_tombstones.py b/tests/android/test_artifact_tombstones.py index f88a9c2..e9159bd 100644 --- a/tests/android/test_artifact_tombstones.py +++ b/tests/android/test_artifact_tombstones.py @@ -128,8 +128,5 @@ class TestTombstoneCrashArtifact: assert tombstone_result.get("pid") == 25541 assert tombstone_result.get("process_name") == "mtk.ape.decoder" - # With Android logs we want to keep timestamps as device local time for consistency. - # We often don't know the time offset for a log entry and so can't convert everything to UTC. - # MVT should output the local time only: - # So original 2023-04-12 12:32:40.518290770+0200 -> 2023-04-12 12:32:40.000000 - assert tombstone_result.get("timestamp") == "2023-04-12 12:32:40.518290" + # Tombstones include an explicit offset, so normalize them to UTC. + assert tombstone_result.get("timestamp") == "2023-04-12 10:32:40.518290"