On 10.01.2017 01:13, Emanuel Bronshtein wrote:
> The result of comparing AndroidManifest.xml (Binary XML) file from APK file
> in apk.py comparator is shown twice,
> first, as AndroidManifest.xml (XML file decoded from original file by apktool)
> second, as original/AndroidManifest.xml (the original undecoded binary file)
>
> fix:
> 1. if there is a difference in decoded AndroidManifest.xml file, remove
> the original/AndroidManifest.xml.
> if there is no difference in AndroidManifest.xml but
> original/AndroidManifest.xml differ, show "No file format specific
> differences found inside, yet data differs" message (which shown by
> diffoscope in similar scenarios [when tool failed to find differences])
>
> 2. it will be better to indicate the AndroidManifest.xml type, such as
> for example:
> show AndroidManifest.xml as "AndroidManifest.xml
> (decoded)"
> show original/AndroidManifest.xml as "AndroidManifest.xml
> (original / undecoded)"
Thanks; Implemented in the attached patches.
Would appreciate if someone reviewed that.
From 5dab5dbc8763a11416aa6af4269cd56892dcca89 Mon Sep 17 00:00:00 2001
From: Maria Glukhova <siamez...@gmail.com>
Date: Mon, 6 Mar 2017 05:54:08 +0200
Subject: [PATCH 1/2] Improve AndroidManifest.xml comparison for APK files.
Instead of comparing both decoded and undecoded AndroidManifest.xml file,
try to first compare decoded one and resort to comparing undecoded ones
only if no difference found in the former.
---
diffoscope/comparators/apk.py | 53 +++++++++++++++++++++++++++++++++++++++++++
tests/comparators/test_apk.py | 15 ++++++++----
2 files changed, 63 insertions(+), 5 deletions(-)
diff --git a/diffoscope/comparators/apk.py b/diffoscope/comparators/apk.py
index 65e9773..fc0b5f1 100644
--- a/diffoscope/comparators/apk.py
+++ b/diffoscope/comparators/apk.py
@@ -28,7 +28,9 @@ from diffoscope.difference import Difference
from .utils.file import File
from .utils.archive import Archive
+from .utils.compare import compare_files
from .zip import Zipinfo, ZipinfoVerbose
+from .missing_file import MissingFile
logger = logging.getLogger(__name__)
@@ -45,6 +47,8 @@ class ApkContainer(Archive):
get_temporary_directory().name,
os.path.basename(self.source.name),
)
+ self._andmanifest = None
+ self._andmanifest_orig = None
logger.debug("Extracting %s to %s", self.source.name, self._unpacked)
@@ -72,12 +76,30 @@ class ApkContainer(Archive):
continue
relpath = abspath[len(self._unpacked)+1:]
+
+ if filename == 'AndroidManifest.xml':
+ containing_dir = root[len(self._unpacked)+1:]
+ if containing_dir == 'original':
+ self._andmanifest_orig = relpath
+ if containing_dir == '':
+ self._andmanifest = relpath
+ continue
+
current_dir.append(relpath)
self._members.extend(current_dir)
return self
+ def get_android_manifest(self):
+ return self.get_member(self._andmanifest) \
+ if self._andmanifest else None
+
+ def get_original_android_manifest(self):
+ if self._andmanifest_orig:
+ return self.get_member(self._andmanifest_orig)
+ return MissingFile('/dev/null', self._andmanifest_orig)
+
def close_archive(self):
pass
@@ -88,6 +110,37 @@ class ApkContainer(Archive):
src_path = os.path.join(self._unpacked, member_name)
return src_path
+ def compare_manifests(self, other):
+ my_android_manifest = self.get_android_manifest()
+ other_android_manifest = other.get_android_manifest()
+ comment = None
+ diff_manifests = None
+ if my_android_manifest and other_android_manifest:
+ diff_manifests = compare_files(my_android_manifest,
+ other_android_manifest)
+ if diff_manifests is None:
+ comment = 'No difference found for decoded AndroidManifest.xml'
+ else:
+ comment = 'No decoded AndroidManifest.xml found ' + \
+ 'for one of the APK files.'
+ if diff_manifests:
+ return diff_manifests
+
+ diff_manifests = compare_files(self.get_original_android_manifest(),
+ other.get_original_android_manifest())
+ if diff_manifests is not None:
+ diff_manifests.add_comment(comment)
+ return diff_manifests
+
+ def compare(self, other, source=None):
+ differences = []
+ try:
+ differences.append(self.compare_manifests(other))
+ except AttributeError: # no apk-specific methods, e.g. MissingArchive
+ pass
+ differences.extend(super().compare(other, source=source))
+ return differences
+
class ApkFile(File):
RE_FILE_TYPE = re.compile(r'^(Java|Zip) archive data.*\b')
RE_FILE_EXTENSION = re.compile(r'\.apk$')
diff --git a/tests/comparators/test_apk.py b/tests/comparators/test_apk.py
index 77b3648..27c46e8 100644
--- a/tests/comparators/test_apk.py
+++ b/tests/comparators/test_apk.py
@@ -52,12 +52,17 @@ def test_zipinfo(differences):
@skip_unless_tools_exist('apktool', 'zipinfo')
def test_android_manifest(differences):
- assert differences[2].source1 == 'AndroidManifest.xml'
- assert differences[2].source2 == 'AndroidManifest.xml'
+ assert differences[1].source1 == 'AndroidManifest.xml'
+ assert differences[1].source2 == 'AndroidManifest.xml'
expected_diff = get_data('apk_manifest_expected_diff')
- assert differences[2].unified_diff == expected_diff
+ assert differences[1].unified_diff == expected_diff
@skip_unless_tools_exist('apktool', 'zipinfo')
def test_apk_metadata_source(differences):
- assert differences[1].source1 == 'APK metadata'
- assert differences[1].source2 == 'APK metadata'
+ assert differences[2].source1 == 'APK metadata'
+ assert differences[2].source2 == 'APK metadata'
+
+@skip_unless_tools_exist('apktool', 'zipinfo')
+def test_skip_undecoded_android_manifest(differences):
+ assert not any(difference.source1 == 'original/AndroidManifest.xml' for difference in differences)
+ assert not any(difference.source2 == 'original/AndroidManifest.xml' for difference in differences)
--
2.11.0
From eb849230e8d258109230613c0725d29898c45a96 Mon Sep 17 00:00:00 2001
From: Maria Glukhova <siamez...@gmail.com>
Date: Mon, 6 Mar 2017 06:03:27 +0200
Subject: [PATCH 2/2] Indicate the AndroidManifest.xml type.
Show the decoded/undecoded type of AndroidManifest.xml file.
Closes: #850758
---
diffoscope/comparators/apk.py | 8 ++++++--
diffoscope/comparators/utils/compare.py | 4 +++-
tests/comparators/test_apk.py | 14 ++++++++++----
3 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/diffoscope/comparators/apk.py b/diffoscope/comparators/apk.py
index fc0b5f1..cd3ef21 100644
--- a/diffoscope/comparators/apk.py
+++ b/diffoscope/comparators/apk.py
@@ -116,8 +116,10 @@ class ApkContainer(Archive):
comment = None
diff_manifests = None
if my_android_manifest and other_android_manifest:
+ source = 'AndroidManifest.xml (decoded)'
diff_manifests = compare_files(my_android_manifest,
- other_android_manifest)
+ other_android_manifest,
+ source=source)
if diff_manifests is None:
comment = 'No difference found for decoded AndroidManifest.xml'
else:
@@ -126,8 +128,10 @@ class ApkContainer(Archive):
if diff_manifests:
return diff_manifests
+ source = 'AndroidManifest.xml (original / undecoded)'
diff_manifests = compare_files(self.get_original_android_manifest(),
- other.get_original_android_manifest())
+ other.get_original_android_manifest(),
+ source=source)
if diff_manifests is not None:
diff_manifests.add_comment(comment)
return diff_manifests
diff --git a/diffoscope/comparators/utils/compare.py b/diffoscope/comparators/utils/compare.py
index b069a0e..4259409 100644
--- a/diffoscope/comparators/utils/compare.py
+++ b/diffoscope/comparators/utils/compare.py
@@ -107,9 +107,11 @@ def bail_if_non_existing(*paths):
def compare_binary_files(file1, file2, source=None):
try:
+ if source is None:
+ source = [file1.name, file2.name]
return Difference.from_command(
Xxd, file1.path, file2.path,
- source=[file1.name, file2.name], has_internal_linenos=True)
+ source=source, has_internal_linenos=True)
except RequiredToolNotFound:
hexdump1 = hexdump_fallback(file1.path)
hexdump2 = hexdump_fallback(file2.path)
diff --git a/tests/comparators/test_apk.py b/tests/comparators/test_apk.py
index 27c46e8..ef68c6e 100644
--- a/tests/comparators/test_apk.py
+++ b/tests/comparators/test_apk.py
@@ -52,8 +52,8 @@ def test_zipinfo(differences):
@skip_unless_tools_exist('apktool', 'zipinfo')
def test_android_manifest(differences):
- assert differences[1].source1 == 'AndroidManifest.xml'
- assert differences[1].source2 == 'AndroidManifest.xml'
+ assert differences[1].source1 == 'AndroidManifest.xml (decoded)'
+ assert differences[1].source2 == 'AndroidManifest.xml (decoded)'
expected_diff = get_data('apk_manifest_expected_diff')
assert differences[1].unified_diff == expected_diff
@@ -64,5 +64,11 @@ def test_apk_metadata_source(differences):
@skip_unless_tools_exist('apktool', 'zipinfo')
def test_skip_undecoded_android_manifest(differences):
- assert not any(difference.source1 == 'original/AndroidManifest.xml' for difference in differences)
- assert not any(difference.source2 == 'original/AndroidManifest.xml' for difference in differences)
+ assert not any(difference.source1 == 'original/AndroidManifest.xml' for
+ difference in differences)
+ assert not any(difference.source2 == 'original/AndroidManifest.xml' for
+ difference in differences)
+ assert not any(difference.source1 == 'AndroidManifest.xml (original / undecoded)' for
+ difference in differences)
+ assert not any(difference.source2 == 'AndroidManifest.xml (original / undecoded)' for
+ difference in differences)
--
2.11.0
