From aa98d67c86f8429c18842a48e8a338746b197d45 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 24 May 2023 11:54:27 +0200 Subject: [PATCH 1/6] metadata: test None in post_metadata_parse --- tests/metadata.TestCase | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 59c2bafe..8f232f4b 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -1999,6 +1999,16 @@ class PostMetadataParseTest(unittest.TestCase): app['Builds'][0][tested_key] = expected return app, post + def test_post_metadata_parse_none(self): + """Run None aka YAML null or blank through the various field and flag types.""" + self.assertEqual(*self._post_metadata_parse_app_list(None, None)) + self.assertEqual(*self._post_metadata_parse_app_string(None, None)) + self.assertEqual(*self._post_metadata_parse_build_bool(None, None)) + self.assertEqual(*self._post_metadata_parse_build_int(None, None)) + self.assertEqual(*self._post_metadata_parse_build_list(None, None)) + self.assertEqual(*self._post_metadata_parse_build_script(None, None)) + self.assertEqual(*self._post_metadata_parse_build_string(None, None)) + def test_post_metadata_parse_int(self): """Run the int 123456 through the various field and flag types.""" with self.assertRaises(TypeError): From 8374842faa9100fcffbf68583f6cdd627b9f99b5 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Wed, 24 May 2023 09:03:23 +0200 Subject: [PATCH 2/6] metadata: normalize TYPE_BOOL to YAML 1.2 booleans This makes the internal representation always be a boolean, and that also means that YAML 1.2 booleans will be written out, e.g. rewritemeta. --- .gitlab-ci.yml | 1 + fdroidserver/metadata.py | 4 ++++ tests/metadata.TestCase | 20 ++++++++++---------- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 0978622d..7a9dd403 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -57,6 +57,7 @@ metadata_v0: - cd fdroiddata - ../tests/dump_internal_metadata_format.py - sed -i + -e '/RequiresRoot:/d' -e "/buildozer/d" -e '/^comments\W /d' -e 's,maven\(\W\) false,maven\1 null,' diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 68826d85..c4b332d0 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -979,6 +979,8 @@ def post_parse_yaml_metadata(yamldata): elif _fieldtype == TYPE_STRINGMAP: if v or v == 0: # TODO probably want just `if v:` yamldata[k] = _normalize_type_stringmap(k, v) + elif _fieldtype == TYPE_BOOL: + yamldata[k] = bool(v) else: if type(v) in (float, int): yamldata[k] = str(v) @@ -1017,6 +1019,8 @@ def post_parse_yaml_metadata(yamldata): elif _flagtype == TYPE_STRINGMAP: if v or v == 0: build[k] = _normalize_type_stringmap(k, v) + elif _flagtype == TYPE_BOOL: + build[k] = bool(v) builds.append(build) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 8f232f4b..89ce3e8b 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -2014,7 +2014,7 @@ class PostMetadataParseTest(unittest.TestCase): with self.assertRaises(TypeError): self._post_metadata_parse_app_list(123456, TypeError) self.assertEqual(*self._post_metadata_parse_app_string(123456, '123456')) - self.assertEqual(*self._post_metadata_parse_build_bool(123456, 123456)) + self.assertEqual(*self._post_metadata_parse_build_bool(123456, True)) self.assertEqual(*self._post_metadata_parse_build_int(123456, 123456)) self.assertEqual(*self._post_metadata_parse_build_list(123456, ['123456'])) self.assertEqual(*self._post_metadata_parse_build_script(123456, ['123456'])) @@ -2024,7 +2024,7 @@ class PostMetadataParseTest(unittest.TestCase): """Run the int 0 through the various field and flag types.""" self.assertEqual(*self._post_metadata_parse_app_list(0, 0)) self.assertEqual(*self._post_metadata_parse_app_string(0, '0')) - self.assertEqual(*self._post_metadata_parse_build_bool(0, 0)) + self.assertEqual(*self._post_metadata_parse_build_bool(0, False)) self.assertEqual(*self._post_metadata_parse_build_int(0, 0)) self.assertEqual(*self._post_metadata_parse_build_list(0, ['0'])) self.assertEqual(*self._post_metadata_parse_build_script(0, ['0'])) @@ -2034,7 +2034,7 @@ class PostMetadataParseTest(unittest.TestCase): """Run the float 0.0 through the various field and flag types.""" self.assertEqual(*self._post_metadata_parse_app_list(0.0, 0.0)) self.assertEqual(*self._post_metadata_parse_app_string(0.0, '0.0')) - self.assertEqual(*self._post_metadata_parse_build_bool(0.0, 0.0)) + self.assertEqual(*self._post_metadata_parse_build_bool(0.0, False)) with self.assertRaises(MetaDataException): self._post_metadata_parse_build_int(0.0, MetaDataException) self.assertEqual(*self._post_metadata_parse_build_list(0.0, 0.0)) @@ -2046,7 +2046,7 @@ class PostMetadataParseTest(unittest.TestCase): with self.assertRaises(TypeError): self._post_metadata_parse_app_list(0.1, TypeError) self.assertEqual(*self._post_metadata_parse_app_string(0.1, '0.1')) - self.assertEqual(*self._post_metadata_parse_build_bool(0.1, 0.1)) + self.assertEqual(*self._post_metadata_parse_build_bool(0.1, True)) with self.assertRaises(MetaDataException): self._post_metadata_parse_build_int(0.1, MetaDataException) self.assertEqual(*self._post_metadata_parse_build_list(0.1, 0.1)) @@ -2058,7 +2058,7 @@ class PostMetadataParseTest(unittest.TestCase): with self.assertRaises(TypeError): self._post_metadata_parse_app_list(1.0, TypeError) self.assertEqual(*self._post_metadata_parse_app_string(1.0, '1.0')) - self.assertEqual(*self._post_metadata_parse_build_bool(1.0, 1.0)) + self.assertEqual(*self._post_metadata_parse_build_bool(1.0, True)) with self.assertRaises(MetaDataException): self._post_metadata_parse_build_int(1.0, MetaDataException) self.assertEqual(*self._post_metadata_parse_build_list(1.0, 1.0)) @@ -2068,7 +2068,7 @@ class PostMetadataParseTest(unittest.TestCase): def test_post_metadata_parse_empty_list(self): self.assertEqual(*self._post_metadata_parse_app_list(list(), list())) self.assertEqual(*self._post_metadata_parse_app_string(list(), list())) - self.assertEqual(*self._post_metadata_parse_build_bool(list(), list())) + self.assertEqual(*self._post_metadata_parse_build_bool(list(), False)) with self.assertRaises(MetaDataException): self._post_metadata_parse_build_int(list(), MetaDataException) self.assertEqual(*self._post_metadata_parse_build_list(list(), list())) @@ -2078,7 +2078,7 @@ class PostMetadataParseTest(unittest.TestCase): def test_post_metadata_parse_set_of_1(self): self.assertEqual(*self._post_metadata_parse_app_list({1}, ['1'])) self.assertEqual(*self._post_metadata_parse_app_string({1}, '{1}')) - self.assertEqual(*self._post_metadata_parse_build_bool({1}, {1})) + self.assertEqual(*self._post_metadata_parse_build_bool({1}, True)) with self.assertRaises(MetaDataException): self._post_metadata_parse_build_int({1}, MetaDataException) self.assertEqual(*self._post_metadata_parse_build_list({1}, {1})) @@ -2088,7 +2088,7 @@ class PostMetadataParseTest(unittest.TestCase): def test_post_metadata_parse_empty_dict(self): self.assertEqual(*self._post_metadata_parse_app_list(dict(), dict())) self.assertEqual(*self._post_metadata_parse_app_string(dict(), dict())) - self.assertEqual(*self._post_metadata_parse_build_bool(dict(), dict())) + self.assertEqual(*self._post_metadata_parse_build_bool(dict(), False)) with self.assertRaises(MetaDataException): self._post_metadata_parse_build_int(dict(), MetaDataException) self.assertEqual(*self._post_metadata_parse_build_list(dict(), dict())) @@ -2098,7 +2098,7 @@ class PostMetadataParseTest(unittest.TestCase): def test_post_metadata_parse_list_int_string(self): self.assertEqual(*self._post_metadata_parse_app_list([1, 'a'], ['1', 'a'])) self.assertEqual(*self._post_metadata_parse_app_string([1, 'a'], "[1, 'a']")) - self.assertEqual(*self._post_metadata_parse_build_bool([1, 'a'], [1, 'a'])) + self.assertEqual(*self._post_metadata_parse_build_bool([1, 'a'], True)) with self.assertRaises(MetaDataException): self._post_metadata_parse_build_int([1, 'a'], MetaDataException) self.assertEqual(*self._post_metadata_parse_build_list([1, 'a'], [1, 'a'])) @@ -2108,7 +2108,7 @@ class PostMetadataParseTest(unittest.TestCase): def test_post_metadata_parse_dict_int_string(self): self.assertEqual(*self._post_metadata_parse_app_list({'k': 1}, ['k'])) self.assertEqual(*self._post_metadata_parse_app_string({'k': 1}, "{'k': 1}")) - self.assertEqual(*self._post_metadata_parse_build_bool({'k': 1}, {'k': 1})) + self.assertEqual(*self._post_metadata_parse_build_bool({'k': 1}, True)) with self.assertRaises(MetaDataException): self._post_metadata_parse_build_int({'k': 1}, MetaDataException) self.assertEqual(*self._post_metadata_parse_build_list({'k': 1}, {'k': 1})) From 9f606d0fbb16a11536ff7b1256c163d8bfd2824f Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 4 May 2023 18:34:50 +0200 Subject: [PATCH 3/6] metadata: auto-convert YAML special float values: .nan .inf -.inf Even for people who know what the special floats not-a-number, infinity, and negative infinity, they don't necessarily know the YAML 1.2 syntax for these. I didn't. And I've spent some quality time fighting things with those values. They are also easy to reliably convert to string values. --- fdroidserver/metadata.py | 9 +++++++++ tests/metadata.TestCase | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index c4b332d0..7f406e7d 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -20,6 +20,7 @@ import git from pathlib import Path +import math import platform import os import re @@ -897,6 +898,14 @@ def _normalize_type_string(v): if v: return 'true' return 'false' + if isinstance(v, float): + # YAML 1.2 values for NaN, Inf, and -Inf + if math.isnan(v): + return '.nan' + if math.isinf(v): + if v > 0: + return '.inf' + return '-.inf' return str(v) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 89ce3e8b..2da4b16c 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -965,6 +965,24 @@ class MetadataTest(unittest.TestCase): {'AntiFeatures': {'true': {}}}, ) + def test_parse_yaml_app_antifeatures_float_nan(self): + self.assertEqual( + metadata.parse_yaml_metadata(io.StringIO('AntiFeatures: .nan')), + {'AntiFeatures': {'.nan': {}}}, + ) + + def test_parse_yaml_app_antifeatures_float_inf(self): + self.assertEqual( + metadata.parse_yaml_metadata(io.StringIO('AntiFeatures: .inf')), + {'AntiFeatures': {'.inf': {}}}, + ) + + def test_parse_yaml_app_antifeatures_float_negative_inf(self): + self.assertEqual( + metadata.parse_yaml_metadata(io.StringIO('AntiFeatures: -.inf')), + {'AntiFeatures': {'-.inf': {}}}, + ) + def test_parse_yaml_app_antifeatures_int(self): self.assertEqual( metadata.parse_yaml_metadata(io.StringIO('AntiFeatures: 1')), From 4711b632b8a78e8b0273609b29ece3b6c7c81549 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Mon, 24 Apr 2023 23:15:18 +0200 Subject: [PATCH 4/6] metadata: _normalize_type_int to handle exceptions --- fdroidserver/metadata.py | 31 +++++++++++++++------- tests/metadata.TestCase | 57 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 75 insertions(+), 13 deletions(-) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 7f406e7d..6d105816 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -884,6 +884,21 @@ def parse_localized_antifeatures(app): app['AntiFeatures'][f.stem][locale] = f.read_text() +def _normalize_type_int(k, v): + """Normalize anything that can be reliably converted to an integer.""" + if isinstance(v, int) and not isinstance(v, bool): + return v + if v is None: + return None + if isinstance(v, str): + try: + return int(v) + except ValueError: + pass + msg = _('{build_flag} must be an integer, found: {value}') + _warn_or_exception(msg.format(build_flag=k, value=v)) + + def _normalize_type_string(v): """Normalize any data to TYPE_STRING. @@ -980,8 +995,9 @@ def post_parse_yaml_metadata(yamldata): elif v: yamldata[k] = [str(i) for i in v] elif _fieldtype == TYPE_INT: - if v: - yamldata[k] = int(v) + v = _normalize_type_int(k, v) + if v or v == 0: + yamldata[k] = v elif _fieldtype == TYPE_STRING: if v or v == 0: yamldata[k] = _normalize_type_string(v) @@ -1005,14 +1021,9 @@ def post_parse_yaml_metadata(yamldata): if v or v == 0: build[k] = _normalize_type_string(v) elif _flagtype == TYPE_INT: - build[k] = v - # versionCode must be int - if not isinstance(v, int): - _warn_or_exception( - _('{build_flag} must be an integer, found: {value}').format( - build_flag=k, value=v - ) - ) + v = _normalize_type_int(k, v) + if v or v == 0: + build[k] = v elif _flagtype in (TYPE_LIST, TYPE_SCRIPT): if isinstance(v, str) or isinstance(v, int): build[k] = [_normalize_type_string(v)] diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index 2da4b16c..bb41a3a2 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -355,6 +355,27 @@ class MetadataTest(unittest.TestCase): metadata._normalize_type_stringmap('AntiFeatures', ['Ads', 'Tracking']), ) + def test_normalize_type_int(self): + """TYPE_INT should be an int whenever possible.""" + self.assertEqual(0, metadata._normalize_type_int('key', 0)) + self.assertEqual(1, metadata._normalize_type_int('key', 1)) + self.assertEqual(-5, metadata._normalize_type_int('key', -5)) + self.assertEqual(0, metadata._normalize_type_int('key', '0')) + self.assertEqual(1, metadata._normalize_type_int('key', '1')) + self.assertEqual(-5, metadata._normalize_type_int('key', '-5')) + self.assertEqual( + 12345678901234567890, + metadata._normalize_type_int('key', 12345678901234567890), + ) + + def test_normalize_type_int_fails(self): + with self.assertRaises(MetaDataException): + metadata._normalize_type_int('key', '1a') + with self.assertRaises(MetaDataException): + metadata._normalize_type_int('key', 1.1) + with self.assertRaises(MetaDataException): + metadata._normalize_type_int('key', True) + def test_post_parse_yaml_metadata(self): yamldata = dict() metadata.post_parse_yaml_metadata(yamldata) @@ -383,7 +404,7 @@ class MetadataTest(unittest.TestCase): yamldata, ) - build['versionCode'] = '1' + build['versionCode'] = '1a' self.assertRaises( fdroidserver.exception.MetaDataException, fdroidserver.metadata.post_parse_yaml_metadata, @@ -1124,6 +1145,34 @@ class MetadataTest(unittest.TestCase): }, ) + def test_parse_yaml_build_type_int_fail(self): + mf = io.StringIO('Builds: [{versionCode: 1a}]') + with self.assertRaises(MetaDataException): + fdroidserver.metadata.parse_yaml_metadata(mf) + + def test_parse_yaml_int_strict_typing_fails(self): + """Things that cannot be preserved when parsing as YAML.""" + mf = io.StringIO('Builds: [{versionCode: 1, rm: 0xf}]') + self.assertEqual( + {'Builds': [{'rm': ['15'], 'versionCode': 1}]}, # 15 != 0xf + fdroidserver.metadata.parse_yaml_metadata(mf), + ) + mf = io.StringIO('Builds: [{versionCode: 1, rm: 0x010}]') + self.assertEqual( + {'Builds': [{'rm': ['16'], 'versionCode': 1}]}, # 16 != 0x010 + fdroidserver.metadata.parse_yaml_metadata(mf), + ) + mf = io.StringIO('Builds: [{versionCode: 1, rm: 0o015}]') + self.assertEqual( + {'Builds': [{'rm': ['13'], 'versionCode': 1}]}, # 13 != 0o015 + fdroidserver.metadata.parse_yaml_metadata(mf), + ) + mf = io.StringIO('Builds: [{versionCode: 1, rm: 10_000}]') + self.assertEqual( + {'Builds': [{'rm': ['10000'], 'versionCode': 1}]}, # 10000 != 10_000 + fdroidserver.metadata.parse_yaml_metadata(mf), + ) + def test_write_yaml_1_line_scripts_as_string(self): mf = io.StringIO() app = fdroidserver.metadata.App() @@ -2137,7 +2186,8 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_app_list(False, False)) self.assertEqual(*self._post_metadata_parse_app_string(False, 'false')) self.assertEqual(*self._post_metadata_parse_build_bool(False, False)) - self.assertEqual(*self._post_metadata_parse_build_int(False, False)) + with self.assertRaises(MetaDataException): + self._post_metadata_parse_build_int(False, MetaDataException) self.assertEqual(*self._post_metadata_parse_build_list(False, ['false'])) self.assertEqual(*self._post_metadata_parse_build_script(False, ['false'])) self.assertEqual(*self._post_metadata_parse_build_string(False, 'false')) @@ -2147,7 +2197,8 @@ class PostMetadataParseTest(unittest.TestCase): self._post_metadata_parse_app_list(True, TypeError) self.assertEqual(*self._post_metadata_parse_app_string(True, 'true')) self.assertEqual(*self._post_metadata_parse_build_bool(True, True)) - self.assertEqual(*self._post_metadata_parse_build_int(True, True)) + with self.assertRaises(MetaDataException): + self._post_metadata_parse_build_int(True, MetaDataException) self.assertEqual(*self._post_metadata_parse_build_list(True, ['true'])) self.assertEqual(*self._post_metadata_parse_build_script(True, ['true'])) self.assertEqual(*self._post_metadata_parse_build_string(True, 'true')) From 642e444cfa6107b233943cd89b17828b124a8302 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Tue, 25 Apr 2023 12:07:02 +0200 Subject: [PATCH 5/6] metadata: _normalize_type_list for TYPE_LIST quirks and errors This should reduce surprises when dealing with filenames in things like `rm:`. So any float/int/bool value can be used directly, without quoting. * A plain str/int/float value is interpreted as a list of one string. * Dictionaries as values throws error. * A set is treated like a list. --- fdroidserver/metadata.py | 35 ++++++----- tests/metadata.TestCase | 123 +++++++++++++++++++++++++++++---------- 2 files changed, 111 insertions(+), 47 deletions(-) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 6d105816..4cd6b04d 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -979,21 +979,35 @@ def _normalize_type_stringmap(k, v): return retdict +def _normalize_type_list(k, v): + """Normalize any data to TYPE_LIST, which is always a list of strings.""" + if isinstance(v, dict): + msg = _('{build_flag} must be list or string, found: {value}') + _warn_or_exception(msg.format(build_flag=k, value=v)) + elif type(v) not in (list, tuple, set): + v = [v] + return [_normalize_type_string(i) for i in v] + + def post_parse_yaml_metadata(yamldata): """Convert human-readable metadata data structures into consistent data structures. + "Be conservative in what is written out, be liberal in what is parsed." + https://en.wikipedia.org/wiki/Robustness_principle + This also handles conversions that make metadata YAML behave something like StrictYAML. Specifically, a field should have a fixed value type, regardless of YAML 1.2's type auto-detection. + TODO: None values should probably be treated as the string 'null', + since YAML 1.2 uses that for nulls + """ for k, v in yamldata.items(): _fieldtype = fieldtype(k) if _fieldtype == TYPE_LIST: - if isinstance(v, str): - yamldata[k] = [v] - elif v: - yamldata[k] = [str(i) for i in v] + if v or v == 0: + yamldata[k] = _normalize_type_list(k, v) elif _fieldtype == TYPE_INT: v = _normalize_type_int(k, v) if v or v == 0: @@ -1025,17 +1039,8 @@ def post_parse_yaml_metadata(yamldata): if v or v == 0: build[k] = v elif _flagtype in (TYPE_LIST, TYPE_SCRIPT): - if isinstance(v, str) or isinstance(v, int): - build[k] = [_normalize_type_string(v)] - else: - build[k] = v - # float and dict are here only to keep things compatible - if type(build[k]) not in (list, tuple, set, float, dict): - _warn_or_exception( - _('{build_flag} must be list or string, found: {value}').format( - build_flag=k, value=v - ) - ) + if v or v == 0: + build[k] = _normalize_type_list(k, v) elif _flagtype == TYPE_STRINGMAP: if v or v == 0: build[k] = _normalize_type_stringmap(k, v) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index bb41a3a2..deaf51e9 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -376,6 +376,36 @@ class MetadataTest(unittest.TestCase): with self.assertRaises(MetaDataException): metadata._normalize_type_int('key', True) + def test_normalize_type_list(self): + """TYPE_LIST is always a list of strings, no matter what YAML thinks.""" + k = 'placeholder' + yaml = ruamel.yaml.YAML(typ='safe') + self.assertEqual(['1.0'], metadata._normalize_type_list(k, 1.0)) + self.assertEqual(['1234567890'], metadata._normalize_type_list(k, 1234567890)) + self.assertEqual(['false'], metadata._normalize_type_list(k, False)) + self.assertEqual(['true'], metadata._normalize_type_list(k, True)) + self.assertEqual(['foo'], metadata._normalize_type_list(k, 'foo')) + self.assertEqual([], metadata._normalize_type_list(k, list())) + self.assertEqual([], metadata._normalize_type_list(k, tuple())) + self.assertEqual([], metadata._normalize_type_list(k, set())) + self.assertEqual(['0', '1', '2'], metadata._normalize_type_list(k, {0, 1, 2})) + self.assertEqual( + ['a', 'b', 'c', '0', '0.0'], + metadata._normalize_type_list(k, yaml.load('[a, b, c, 0, 0.0]')), + ) + self.assertEqual( + ['1', '1.0', 's', 'true', '{}'], + metadata._normalize_type_list(k, yaml.load('[1, 1.0, s, true, {}]')), + ) + self.assertEqual( + ['1', '1.0', 's', 'true', '{}'], + metadata._normalize_type_list(k, (1, 1.0, 's', True, dict())), + ) + + def test_normalize_type_list_fails(self): + with self.assertRaises(MetaDataException): + metadata._normalize_type_list('placeholder', dict()) + def test_post_parse_yaml_metadata(self): yamldata = dict() metadata.post_parse_yaml_metadata(yamldata) @@ -386,10 +416,18 @@ class MetadataTest(unittest.TestCase): metadata.post_parse_yaml_metadata(yamldata) def test_post_parse_yaml_metadata_fails(self): - yamldata = {'AllowedAPKSigningKeys': True} - with self.assertRaises(TypeError): + yamldata = {'AllowedAPKSigningKeys': {'bad': 'dict-placement'}} + with self.assertRaises(MetaDataException): metadata.post_parse_yaml_metadata(yamldata) + def test_post_parse_yaml_metadata_0padding_fails(self): + """Special case: ideally 0 padding would be kept in string, but it is not.""" + v = '0027293472934293872934729834729834729834729834792837487293847926' + yaml = ruamel.yaml.YAML(typ='safe') + yamldata = yaml.load('AllowedAPKSigningKeys: ' + v) + metadata.post_parse_yaml_metadata(yamldata) + self.assertNotEqual(yamldata['AllowedAPKSigningKeys'], v) + def test_post_parse_yaml_metadata_builds(self): yamldata = OrderedDict() builds = [] @@ -496,14 +534,36 @@ class MetadataTest(unittest.TestCase): ) def test_parse_yaml_metadata_app_type_list_fails(self): - mf = _get_mock_mf('AllowedAPKSigningKeys: true') - with self.assertRaises(TypeError): - metadata.parse_yaml_metadata(mf) - mf = _get_mock_mf('AllowedAPKSigningKeys: 1') - with self.assertRaises(TypeError): + mf = _get_mock_mf('AllowedAPKSigningKeys: {t: f}') + with self.assertRaises(MetaDataException): metadata.parse_yaml_metadata(mf) - self.assertEqual(fdroidserver.metadata.parse_yaml_metadata(mf), dict()) + def test_parse_yaml_metadata_build_type_list_fails(self): + mf = _get_mock_mf('Builds: [{versionCode: 1, rm: {bad: dict-placement}}]') + with self.assertRaises(MetaDataException): + metadata.parse_yaml_metadata(mf) + + def test_parse_yaml_metadata_0padding_fails(self): + """Special case: ideally this would keep 0 padding in string, but it does not. + + It seems the only option is to quote values like that, or add + quirks to the YAML parsing. + + """ + v = '0027293472934293872934729834729834729834729834792837487293847926' + mf = io.StringIO('AllowedAPKSigningKeys: %s' % v) + mf.name = 'mock_filename.yaml' + self.assertNotEqual( + v, + metadata.parse_yaml_metadata(mf)['AllowedAPKSigningKeys'][0], + ) + + mf = io.StringIO('AllowedAPKSigningKeys: "%s"' % v) + mf.name = 'mock_filename.yaml' + self.assertEqual( + v, + metadata.parse_yaml_metadata(mf)['AllowedAPKSigningKeys'][0], + ) def test_parse_yaml_metadata_unknown_app_field(self): mf = io.StringIO( @@ -2078,8 +2138,7 @@ class PostMetadataParseTest(unittest.TestCase): def test_post_metadata_parse_int(self): """Run the int 123456 through the various field and flag types.""" - with self.assertRaises(TypeError): - self._post_metadata_parse_app_list(123456, TypeError) + self.assertEqual(*self._post_metadata_parse_app_list(123456, ['123456'])) self.assertEqual(*self._post_metadata_parse_app_string(123456, '123456')) self.assertEqual(*self._post_metadata_parse_build_bool(123456, True)) self.assertEqual(*self._post_metadata_parse_build_int(123456, 123456)) @@ -2089,7 +2148,7 @@ class PostMetadataParseTest(unittest.TestCase): def test_post_metadata_parse_int_0(self): """Run the int 0 through the various field and flag types.""" - self.assertEqual(*self._post_metadata_parse_app_list(0, 0)) + self.assertEqual(*self._post_metadata_parse_app_list(0, ['0'])) self.assertEqual(*self._post_metadata_parse_app_string(0, '0')) self.assertEqual(*self._post_metadata_parse_build_bool(0, False)) self.assertEqual(*self._post_metadata_parse_build_int(0, 0)) @@ -2099,37 +2158,35 @@ class PostMetadataParseTest(unittest.TestCase): def test_post_metadata_parse_float_0_0(self): """Run the float 0.0 through the various field and flag types.""" - self.assertEqual(*self._post_metadata_parse_app_list(0.0, 0.0)) + self.assertEqual(*self._post_metadata_parse_app_list(0.0, ['0.0'])) self.assertEqual(*self._post_metadata_parse_app_string(0.0, '0.0')) self.assertEqual(*self._post_metadata_parse_build_bool(0.0, False)) with self.assertRaises(MetaDataException): self._post_metadata_parse_build_int(0.0, MetaDataException) - self.assertEqual(*self._post_metadata_parse_build_list(0.0, 0.0)) - self.assertEqual(*self._post_metadata_parse_build_script(0.0, 0.0)) + self.assertEqual(*self._post_metadata_parse_build_list(0.0, ['0.0'])) + self.assertEqual(*self._post_metadata_parse_build_script(0.0, ['0.0'])) self.assertEqual(*self._post_metadata_parse_build_string(0.0, '0.0')) def test_post_metadata_parse_float_0_1(self): """Run the float 0.1 through the various field and flag types.""" - with self.assertRaises(TypeError): - self._post_metadata_parse_app_list(0.1, TypeError) + self.assertEqual(*self._post_metadata_parse_app_list(0.1, ['0.1'])) self.assertEqual(*self._post_metadata_parse_app_string(0.1, '0.1')) self.assertEqual(*self._post_metadata_parse_build_bool(0.1, True)) with self.assertRaises(MetaDataException): self._post_metadata_parse_build_int(0.1, MetaDataException) - self.assertEqual(*self._post_metadata_parse_build_list(0.1, 0.1)) - self.assertEqual(*self._post_metadata_parse_build_script(0.1, 0.1)) + self.assertEqual(*self._post_metadata_parse_build_list(0.1, ['0.1'])) + self.assertEqual(*self._post_metadata_parse_build_script(0.1, ['0.1'])) self.assertEqual(*self._post_metadata_parse_build_string(0.1, '0.1')) def test_post_metadata_parse_float_1_0(self): """Run the float 1.0 through the various field and flag types.""" - with self.assertRaises(TypeError): - self._post_metadata_parse_app_list(1.0, TypeError) + self.assertEqual(*self._post_metadata_parse_app_list(1.0, ['1.0'])) self.assertEqual(*self._post_metadata_parse_app_string(1.0, '1.0')) self.assertEqual(*self._post_metadata_parse_build_bool(1.0, True)) with self.assertRaises(MetaDataException): self._post_metadata_parse_build_int(1.0, MetaDataException) - self.assertEqual(*self._post_metadata_parse_build_list(1.0, 1.0)) - self.assertEqual(*self._post_metadata_parse_build_script(1.0, 1.0)) + self.assertEqual(*self._post_metadata_parse_build_list(1.0, ['1.0'])) + self.assertEqual(*self._post_metadata_parse_build_script(1.0, ['1.0'])) self.assertEqual(*self._post_metadata_parse_build_string(1.0, '1.0')) def test_post_metadata_parse_empty_list(self): @@ -2148,8 +2205,8 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_build_bool({1}, True)) with self.assertRaises(MetaDataException): self._post_metadata_parse_build_int({1}, MetaDataException) - self.assertEqual(*self._post_metadata_parse_build_list({1}, {1})) - self.assertEqual(*self._post_metadata_parse_build_script({1}, {1})) + self.assertEqual(*self._post_metadata_parse_build_list({1}, ['1'])) + self.assertEqual(*self._post_metadata_parse_build_script({1}, ['1'])) self.assertEqual(*self._post_metadata_parse_build_string({1}, '{1}')) def test_post_metadata_parse_empty_dict(self): @@ -2168,22 +2225,25 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_build_bool([1, 'a'], True)) with self.assertRaises(MetaDataException): self._post_metadata_parse_build_int([1, 'a'], MetaDataException) - self.assertEqual(*self._post_metadata_parse_build_list([1, 'a'], [1, 'a'])) - self.assertEqual(*self._post_metadata_parse_build_script([1, 'a'], [1, 'a'])) + self.assertEqual(*self._post_metadata_parse_build_list([1, 'a'], ['1', 'a'])) + self.assertEqual(*self._post_metadata_parse_build_script([1, 'a'], ['1', 'a'])) self.assertEqual(*self._post_metadata_parse_build_string([1, 'a'], "[1, 'a']")) def test_post_metadata_parse_dict_int_string(self): - self.assertEqual(*self._post_metadata_parse_app_list({'k': 1}, ['k'])) + with self.assertRaises(MetaDataException): + self._post_metadata_parse_app_list({'k': 1}, MetaDataException) self.assertEqual(*self._post_metadata_parse_app_string({'k': 1}, "{'k': 1}")) self.assertEqual(*self._post_metadata_parse_build_bool({'k': 1}, True)) with self.assertRaises(MetaDataException): self._post_metadata_parse_build_int({'k': 1}, MetaDataException) - self.assertEqual(*self._post_metadata_parse_build_list({'k': 1}, {'k': 1})) - self.assertEqual(*self._post_metadata_parse_build_script({'k': 1}, {'k': 1})) + with self.assertRaises(MetaDataException): + self._post_metadata_parse_build_list({'k': 1}, MetaDataException) + with self.assertRaises(MetaDataException): + self._post_metadata_parse_build_script({'k': 1}, MetaDataException) self.assertEqual(*self._post_metadata_parse_build_string({'k': 1}, "{'k': 1}")) def test_post_metadata_parse_false(self): - self.assertEqual(*self._post_metadata_parse_app_list(False, False)) + self.assertEqual(*self._post_metadata_parse_app_list(False, ['false'])) self.assertEqual(*self._post_metadata_parse_app_string(False, 'false')) self.assertEqual(*self._post_metadata_parse_build_bool(False, False)) with self.assertRaises(MetaDataException): @@ -2193,8 +2253,7 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_build_string(False, 'false')) def test_post_metadata_parse_true(self): - with self.assertRaises(TypeError): - self._post_metadata_parse_app_list(True, TypeError) + self.assertEqual(*self._post_metadata_parse_app_list(True, ['true'])) self.assertEqual(*self._post_metadata_parse_app_string(True, 'true')) self.assertEqual(*self._post_metadata_parse_build_bool(True, True)) with self.assertRaises(MetaDataException): From 2aa0403208348e0d1ff4b1845a71b5be6521a1d1 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 4 May 2023 22:06:42 +0200 Subject: [PATCH 6/6] metadata: handle SHA-256 values that parse as decimal ints https://gitlab.com/fdroid/fdroidserver/-/merge_requests/1350#note_1370665635 --- fdroidserver/metadata.py | 8 ++++++ tests/metadata.TestCase | 53 +++++++++++++++++++++------------------- 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index 4cd6b04d..a4203b08 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -908,6 +908,11 @@ def _normalize_type_string(v): numbers. Like "versionName: 1.0" would be a YAML float, but should be a string. + SHA-256 values are string values, but YAML 1.2 can interpret some + unquoted values as decimal ints. This converts those to a string + if they are over 50 digits. In the wild, the longest 0 padding on + a SHA-256 key fingerprint I found was 8 zeros. + """ if isinstance(v, bool): if v: @@ -921,6 +926,9 @@ def _normalize_type_string(v): if v > 0: return '.inf' return '-.inf' + if v and isinstance(v, int): + if math.log10(v) > 50: # only if the int has this many digits + return '%064d' % v return str(v) diff --git a/tests/metadata.TestCase b/tests/metadata.TestCase index deaf51e9..777002bc 100755 --- a/tests/metadata.TestCase +++ b/tests/metadata.TestCase @@ -343,6 +343,13 @@ class MetadataTest(unittest.TestCase): self.assertEqual('false', metadata._normalize_type_string(False)) self.assertEqual('true', metadata._normalize_type_string(True)) + def test_normalize_type_string_sha256(self): + """SHA-256 values are TYPE_STRING, which YAML can parse as decimal ints.""" + yaml = ruamel.yaml.YAML(typ='safe') + for v in range(1, 1000): + s = '%064d' % (v * (10**51)) + self.assertEqual(s, metadata._normalize_type_string(yaml.load(s))) + def test_normalize_type_stringmap_none(self): self.assertEqual(dict(), metadata._normalize_type_stringmap('key', None)) @@ -420,13 +427,13 @@ class MetadataTest(unittest.TestCase): with self.assertRaises(MetaDataException): metadata.post_parse_yaml_metadata(yamldata) - def test_post_parse_yaml_metadata_0padding_fails(self): - """Special case: ideally 0 padding would be kept in string, but it is not.""" + def test_post_parse_yaml_metadata_0padding_sha256(self): + """SHA-256 values are strings, but YAML 1.2 will read some as decimal ints.""" v = '0027293472934293872934729834729834729834729834792837487293847926' yaml = ruamel.yaml.YAML(typ='safe') yamldata = yaml.load('AllowedAPKSigningKeys: ' + v) metadata.post_parse_yaml_metadata(yamldata) - self.assertNotEqual(yamldata['AllowedAPKSigningKeys'], v) + self.assertEqual(yamldata['AllowedAPKSigningKeys'], [v]) def test_post_parse_yaml_metadata_builds(self): yamldata = OrderedDict() @@ -543,28 +550,6 @@ class MetadataTest(unittest.TestCase): with self.assertRaises(MetaDataException): metadata.parse_yaml_metadata(mf) - def test_parse_yaml_metadata_0padding_fails(self): - """Special case: ideally this would keep 0 padding in string, but it does not. - - It seems the only option is to quote values like that, or add - quirks to the YAML parsing. - - """ - v = '0027293472934293872934729834729834729834729834792837487293847926' - mf = io.StringIO('AllowedAPKSigningKeys: %s' % v) - mf.name = 'mock_filename.yaml' - self.assertNotEqual( - v, - metadata.parse_yaml_metadata(mf)['AllowedAPKSigningKeys'][0], - ) - - mf = io.StringIO('AllowedAPKSigningKeys: "%s"' % v) - mf.name = 'mock_filename.yaml' - self.assertEqual( - v, - metadata.parse_yaml_metadata(mf)['AllowedAPKSigningKeys'][0], - ) - def test_parse_yaml_metadata_unknown_app_field(self): mf = io.StringIO( textwrap.dedent( @@ -2146,6 +2131,24 @@ class PostMetadataParseTest(unittest.TestCase): self.assertEqual(*self._post_metadata_parse_build_script(123456, ['123456'])) self.assertEqual(*self._post_metadata_parse_build_string(123456, '123456')) + def test_post_metadata_parse_sha256(self): + """Run a SHA-256 that YAML calls an int through the various types. + + The current f-droid.org signer set has SHA-256 values with a + maximum of two leading zeros, but this will handle more. + + """ + yaml = ruamel.yaml.YAML(typ='safe', pure=True) + str_sha256 = '0000000000000498456908409534729834729834729834792837487293847926' + sha256 = yaml.load('a: ' + str_sha256)['a'] + self.assertEqual(*self._post_metadata_parse_app_list(sha256, [str_sha256])) + self.assertEqual(*self._post_metadata_parse_app_string(sha256, str_sha256)) + self.assertEqual(*self._post_metadata_parse_build_bool(sha256, True)) + self.assertEqual(*self._post_metadata_parse_build_int(sha256, sha256)) + self.assertEqual(*self._post_metadata_parse_build_list(sha256, [str_sha256])) + self.assertEqual(*self._post_metadata_parse_build_script(sha256, [str_sha256])) + self.assertEqual(*self._post_metadata_parse_build_string(sha256, str_sha256)) + def test_post_metadata_parse_int_0(self): """Run the int 0 through the various field and flag types.""" self.assertEqual(*self._post_metadata_parse_app_list(0, ['0']))