standardize config on ruamel.yaml with a YAML 1.2 config

This is a key piece of the ongoing `PUBLISH` _config.yml_ migration. There was uneven implementation of which YAML parser to use, and that could lead to bugs where one parser might read a value one way, and a different parser will read the value a different way. I wanted to be sure that YAML 1.2 would always work.

This makes all code that handles config files use the same `ruamel.yaml` parsers.  This only touches other usages of YAML parsers when there is overlap.  This does not port all of _fdroidserver_ to `ruamel.yaml` and YAML 1.2.  The metadata files should already be YAML 1.2 anyway.

# Conflicts:
#	fdroidserver/lint.py

fdroidserver/_yaml.py

@@ -0,0 +1,40 @@
+# Copyright (C) 2025, Hans-Christoph Steiner <hans@eds.org>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Standard YAML parsing and dumping.
+
+YAML 1.2 is the preferred format for all data files.  When loading
+F-Droid formats like config.yml and <Application ID>.yml, YAML 1.2 is
+forced, and older YAML constructs should be considered an error.
+
+It is OK to load and dump files in other YAML versions if they are
+externally defined formats, like FUNDING.yml.  In those cases, these
+common instances might not be appropriate to use.
+
+There is a separate instance for dumping based on the "round trip" aka
+"rt" mode.  The "rt" mode maintains order while the "safe" mode sorts
+the output.  Also, yaml.version is not forced in the dumper because that
+makes it write out a "%YAML 1.2" header.  F-Droid's formats are
+explicitly defined as YAML 1.2 and meant to be human-editable.  So that
+header gets in the way.
+
+"""
+
+import ruamel.yaml
+
+yaml = ruamel.yaml.YAML(typ='safe')
+yaml.version = (1, 2)
+
+yaml_dumper = ruamel.yaml.YAML(typ='rt')

fdroidserver/common.py

@@ -39,6 +39,7 @@ import sys
 import re
 import ast
 import gzip
+import ruamel.yaml
 import shutil
 import stat
 import subprocess
@@ -48,7 +49,6 @@ import logging
 import hashlib
 import socket
 import base64
-import yaml
 import zipfile
 import tempfile
 import json
@@ -67,6 +67,7 @@ from zipfile import ZipFile
 
 import fdroidserver.metadata
 from fdroidserver import _
+from fdroidserver._yaml import yaml, yaml_dumper
 from fdroidserver.exception import FDroidException, VCSException, NoSubmodulesException, \
     BuildException, VerificationException, MetaDataException
 from .asynchronousfilereader import AsynchronousFileReader
@@ -549,7 +550,7 @@ def read_config():
     if os.path.exists(CONFIG_FILE):
         logging.debug(_("Reading '{config_file}'").format(config_file=CONFIG_FILE))
         with open(CONFIG_FILE, encoding='utf-8') as fp:
-            config = yaml.safe_load(fp)
+            config = yaml.load(fp)
         if not config:
             config = {}
         config_type_check(CONFIG_FILE, config)
@@ -706,7 +707,7 @@ def load_localized_config(name, repodir):
         if len(f.parts) == 2:
             locale = DEFAULT_LOCALE
         with open(f, encoding="utf-8") as fp:
-            elem = yaml.safe_load(fp)
+            elem = yaml.load(fp)
             if not isinstance(elem, dict):
                 msg = _('{path} is not "key: value" dict, but a {datatype}!')
                 raise TypeError(msg.format(path=f, datatype=type(elem).__name__))
@@ -4229,7 +4230,9 @@ def write_to_config(thisconfig, key, value=None):
             lines[-1] += '\n'
 
     pattern = re.compile(r'^[\s#]*' + key + r':.*\n')
-    repl = yaml.dump({key: value})
+    with ruamel.yaml.compat.StringIO() as fp:
+        yaml_dumper.dump({key: value}, fp)
+        repl = fp.getvalue()
 
     # If we replaced this line once, we make sure won't be a
     # second instance of this line for this key in the document.

fdroidserver/index.py

@@ -26,7 +26,6 @@ import json
 import logging
 import os
 import re
-import ruamel.yaml
 import shutil
 import sys
 import tempfile
@@ -45,6 +44,7 @@ from . import metadata
 from . import net
 from . import signindex
 from fdroidserver.common import ANTIFEATURES_CONFIG_NAME, CATEGORIES_CONFIG_NAME, CONFIG_CONFIG_NAME, MIRRORS_CONFIG_NAME, RELEASECHANNELS_CONFIG_NAME, DEFAULT_LOCALE, FDroidPopen, FDroidPopenBytes, load_stats_fdroid_signing_key_fingerprints
+from fdroidserver._yaml import yaml
 from fdroidserver.exception import FDroidException, VerificationException
 
 
@@ -1445,7 +1445,7 @@ def add_mirrors_to_repodict(repo_section, repodict):
                 )
             )
         with mirrors_yml.open() as fp:
-            mirrors_config = ruamel.yaml.YAML(typ='safe').load(fp)
+            mirrors_config = yaml.load(fp)
         if not isinstance(mirrors_config, list):
             msg = _('{path} is not list, but a {datatype}!')
             raise TypeError(

fdroidserver/lint.py

@@ -24,9 +24,8 @@ import urllib.parse
 from argparse import ArgumentParser
 from pathlib import Path
 
-import ruamel.yaml
-
 from . import _, common, metadata, rewritemeta
+from fdroidserver._yaml import yaml
 
 config = None
 
@@ -853,7 +852,7 @@ def lint_config(arg):
         passed = False
 
     with path.open() as fp:
-        data = ruamel.yaml.YAML(typ='safe').load(fp)
+        data = yaml.load(fp)
     common.config_type_check(arg, data)
 
     if path.name == mirrors_name:

fdroidserver/metadata.py

@@ -31,6 +31,7 @@ from collections import OrderedDict
 from . import common
 from . import _
 from .exception import MetaDataException
+from ._yaml import yaml
 
 srclibs = None
 warnings_action = None
@@ -472,7 +473,6 @@ def parse_yaml_srclib(metadatapath):
 
     with metadatapath.open("r", encoding="utf-8") as f:
         try:
-            yaml = ruamel.yaml.YAML(typ='safe')
             data = yaml.load(f)
             if type(data) is not dict:
                 if platform.system() == 'Windows':
@@ -709,8 +709,7 @@ def parse_yaml_metadata(mf):
 
     """
     try:
-        yaml = ruamel.yaml.YAML(typ='safe')
+        yamldata = common.yaml.load(mf)
-        yamldata = yaml.load(mf)
     except ruamel.yaml.YAMLError as e:
         _warn_or_exception(
             _("could not parse '{path}'").format(path=mf.name)
@@ -1249,19 +1248,24 @@ def _app_to_yaml(app):
 def write_yaml(mf, app):
     """Write metadata in yaml format.
 
+    This requires the 'rt' round trip dumper to maintain order and needs
+    custom indent settings, so it needs to instantiate its own YAML
+    instance.  Therefore, this function deliberately avoids using any of
+    the common YAML parser setups.
+
     Parameters
     ----------
     mf
       active file discriptor for writing
     app
-      app metadata to written to the yaml file
+      app metadata to written to the YAML file
 
     """
     _del_duplicated_NoSourceSince(app)
     yaml_app = _app_to_yaml(app)
-    yaml = ruamel.yaml.YAML()
+    yamlmf = ruamel.yaml.YAML(typ='rt')
-    yaml.indent(mapping=2, sequence=4, offset=2)
+    yamlmf.indent(mapping=2, sequence=4, offset=2)
-    yaml.dump(yaml_app, stream=mf)
+    yamlmf.dump(yaml_app, stream=mf)
 
 
 def write_metadata(metadatapath, app):

tests/metadata/dump/app.with.special.build.params.yaml

@@ -1,3 +1,5 @@
+%YAML 1.2
+---
 AllowedAPKSigningKeys: []
 AntiFeatures:
   UpstreamNonFree: {}

tests/metadata/dump/com.politedroid.yaml

@@ -1,3 +1,5 @@
+%YAML 1.2
+---
 AllowedAPKSigningKeys: []
 AntiFeatures:
   NoSourceSince:

tests/metadata/dump/org.adaway.yaml

@@ -1,3 +1,5 @@
+%YAML 1.2
+---
 AllowedAPKSigningKeys: []
 AntiFeatures: {}
 ArchivePolicy: null

tests/metadata/dump/org.smssecure.smssecure.yaml

@@ -1,3 +1,5 @@
+%YAML 1.2
+---
 AllowedAPKSigningKeys: []
 AntiFeatures: {}
 ArchivePolicy: null

tests/metadata/dump/org.videolan.vlc.yaml

@@ -1,3 +1,5 @@
+%YAML 1.2
+---
 AllowedAPKSigningKeys: []
 AntiFeatures: {}
 ArchivePolicy: 9

tests/test_common.py

@@ -17,7 +17,6 @@ import tempfile
 import time
 import unittest
 import textwrap
-import yaml
 import gzip
 from argparse import ArgumentParser
 from datetime import datetime, timezone
@@ -32,6 +31,7 @@ import fdroidserver.common
 import fdroidserver.metadata
 from .shared_test_code import TmpCwd, mkdtemp
 from fdroidserver.common import ANTIFEATURES_CONFIG_NAME, CATEGORIES_CONFIG_NAME
+from fdroidserver._yaml import yaml, yaml_dumper
 from fdroidserver.exception import FDroidException, VCSException,\
     MetaDataException, VerificationException
 from fdroidserver.looseversion import LooseVersion
@@ -77,6 +77,26 @@ class CommonTest(unittest.TestCase):
         if os.path.exists(self.tmpdir):
             shutil.rmtree(self.tmpdir)
 
+    def test_yaml_1_2(self):
+        """Return a ruamel.yaml instance that supports YAML 1.2
+
+        There should be no "Norway Problem", and other things like this:
+        https://yaml.org/spec/1.2.2/ext/changes/
+
+        YAML 1.2 says "underlines _ cannot be used within numerical
+        values", but ruamel.yaml seems to ignore that. 1_0 should be a
+        string, but it is read as a 10.
+
+        """
+        os.chdir(self.testdir)
+        yaml12file = Path('YAML 1.2.yml')
+        yaml12file.write_text('[true, no, 0b010, 010, 0o10, "\\/"]', encoding='utf-8')
+        with yaml12file.open() as fp:
+            self.assertEqual(
+                [True, 'no', 2, 10, 8, '/'],
+                yaml.load(fp),
+            )
+
     def test_parse_human_readable_size(self):
         for k, v in (
             (9827, 9827),
@@ -417,7 +437,7 @@ class CommonTest(unittest.TestCase):
         metadata['RepoType'] = 'git'
         metadata['Repo'] = git_url
         with open(os.path.join('metadata', packageName + '.yml'), 'w') as fp:
-            yaml.dump(metadata, fp)
+            yaml_dumper.dump(metadata, fp)
 
         gitrepo = os.path.join(self.tmpdir, 'build', packageName)
         vcs0 = fdroidserver.common.getvcs('git', git_url, gitrepo)
@@ -1913,7 +1933,7 @@ class CommonTest(unittest.TestCase):
         os.chdir(self.tmpdir)
         teststr = '/πÇÇ现代通用字-български-عربي1/ö/yml'
         with open(fdroidserver.common.CONFIG_FILE, 'w', encoding='utf-8') as fp:
-            yaml.dump({'apksigner': teststr}, fp)
+            yaml_dumper.dump({'apksigner': teststr}, fp)
         self.assertTrue(os.path.exists(fdroidserver.common.CONFIG_FILE))
         config = fdroidserver.common.read_config()
         self.assertEqual(teststr, config.get('apksigner'))
@@ -1937,7 +1957,7 @@ class CommonTest(unittest.TestCase):
     def test_with_config_yml_is_not_mixed_type(self):
         os.chdir(self.tmpdir)
         Path(fdroidserver.common.CONFIG_FILE).write_text('k: v\napksigner = /bin/apk')
-        with self.assertRaises(yaml.scanner.ScannerError):
+        with self.assertRaises(ruamel.yaml.scanner.ScannerError):
             fdroidserver.common.read_config()
 
     def test_config_perm_warning(self):
@@ -2613,7 +2633,7 @@ class CommonTest(unittest.TestCase):
                 ' -providerClass sun.security.pkcs11.SunPKCS11'
                 ' -providerArg opensc-fdroid.cfg'
             }
-            yaml.dump(d, fp)
+            yaml_dumper.dump(d, fp)
         config = fdroidserver.common.read_config()
         fdroidserver.common.config = config
         self.assertTrue(isinstance(d['smartcardoptions'], str))
@@ -2829,21 +2849,21 @@ class CommonTest(unittest.TestCase):
 
     def test_parse_mirrors_config_str(self):
         s = 'foo@example.com:/var/www'
-        mirrors = ruamel.yaml.YAML(typ='safe').load("""'%s'""" % s)
+        mirrors = yaml.load("""'%s'""" % s)
         self.assertEqual(
             [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors)
         )
 
     def test_parse_mirrors_config_list(self):
         s = 'foo@example.com:/var/www'
-        mirrors = ruamel.yaml.YAML(typ='safe').load("""- '%s'""" % s)
+        mirrors = yaml.load("""- '%s'""" % s)
         self.assertEqual(
             [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors)
         )
 
     def test_parse_mirrors_config_dict(self):
         s = 'foo@example.com:/var/www'
-        mirrors = ruamel.yaml.YAML(typ='safe').load("""- url: '%s'""" % s)
+        mirrors = yaml.load("""- url: '%s'""" % s)
         self.assertEqual(
             [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors)
         )

tests/test_integration.py

@@ -11,13 +11,12 @@ from datetime import datetime, timezone
 from http.server import SimpleHTTPRequestHandler, ThreadingHTTPServer
 from pathlib import Path
 
-from ruamel.yaml import YAML
-
 try:
     from androguard.core.bytecodes.apk import get_apkid  # androguard <4
 except ModuleNotFoundError:
     from androguard.core.apk import get_apkid
 
+from fdroidserver._yaml import yaml, yaml_dumper
 from .shared_test_code import mkdir_testfiles
 
 # TODO: port generic tests that use index.xml to index-v2 (test that
@@ -81,7 +80,6 @@ class IntegrationTest(unittest.TestCase):
     @staticmethod
     def update_yaml(path, items, replace=False):
         """Update a .yml file, e.g. config.yml, with the given items."""
-        yaml = YAML()
         doc = {}
         if not replace:
             try:
@@ -91,7 +89,7 @@ class IntegrationTest(unittest.TestCase):
                 pass
         doc.update(items)
         with open(path, "w") as f:
-            yaml.dump(doc, f)
+            yaml_dumper.dump(doc, f)
 
     @staticmethod
     def remove_lines(path, unwanted_strings):

tests/test_lint.py

@@ -7,13 +7,12 @@ import tempfile
 import unittest
 from pathlib import Path
 
-import ruamel.yaml
-
 from .shared_test_code import mkdtemp
 
 import fdroidserver.common
 import fdroidserver.lint
 import fdroidserver.metadata
+from fdroidserver._yaml import yaml_dumper
 
 basedir = Path(__file__).parent
 
@@ -365,40 +364,41 @@ class LintTest(unittest.TestCase):
 
     def test_lint_config_basic_mirrors_yml(self):
         os.chdir(self.testdir)
-        yaml = ruamel.yaml.YAML(typ='safe')
         with Path('mirrors.yml').open('w') as fp:
-            yaml.dump([{'url': 'https://example.com/fdroid/repo'}], fp)
+            yaml_dumper.dump([{'url': 'https://example.com/fdroid/repo'}], fp)
         self.assertTrue(fdroidserver.lint.lint_config('mirrors.yml'))
 
     def test_lint_config_mirrors_yml_kenya_countryCode(self):
         os.chdir(self.testdir)
-        yaml = ruamel.yaml.YAML(typ='safe')
         with Path('mirrors.yml').open('w') as fp:
-            yaml.dump([{'url': 'https://foo.com/fdroid/repo', 'countryCode': 'KE'}], fp)
+            yaml_dumper.dump(
+                [{'url': 'https://foo.com/fdroid/repo', 'countryCode': 'KE'}], fp
+            )
         self.assertTrue(fdroidserver.lint.lint_config('mirrors.yml'))
 
     def test_lint_config_mirrors_yml_invalid_countryCode(self):
         """WV is "indeterminately reserved" so it should never be used."""
         os.chdir(self.testdir)
-        yaml = ruamel.yaml.YAML(typ='safe')
         with Path('mirrors.yml').open('w') as fp:
-            yaml.dump([{'url': 'https://foo.com/fdroid/repo', 'countryCode': 'WV'}], fp)
+            yaml_dumper.dump(
+                [{'url': 'https://foo.com/fdroid/repo', 'countryCode': 'WV'}], fp
+            )
         self.assertFalse(fdroidserver.lint.lint_config('mirrors.yml'))
 
     def test_lint_config_mirrors_yml_alpha3_countryCode(self):
         """Only ISO 3166-1 alpha 2 are supported"""
         os.chdir(self.testdir)
-        yaml = ruamel.yaml.YAML(typ='safe')
         with Path('mirrors.yml').open('w') as fp:
-            yaml.dump([{'url': 'https://de.com/fdroid/repo', 'countryCode': 'DEU'}], fp)
+            yaml_dumper.dump(
+                [{'url': 'https://de.com/fdroid/repo', 'countryCode': 'DEU'}], fp
+            )
         self.assertFalse(fdroidserver.lint.lint_config('mirrors.yml'))
 
     def test_lint_config_mirrors_yml_one_invalid_countryCode(self):
         """WV is "indeterminately reserved" so it should never be used."""
         os.chdir(self.testdir)
-        yaml = ruamel.yaml.YAML(typ='safe')
         with Path('mirrors.yml').open('w') as fp:
-            yaml.dump(
+            yaml_dumper.dump(
                 [
                     {'url': 'https://bar.com/fdroid/repo', 'countryCode': 'BA'},
                     {'url': 'https://foo.com/fdroid/repo', 'countryCode': 'FO'},

tests/test_metadata.py

@@ -17,6 +17,7 @@ import fdroidserver
 from fdroidserver import metadata
 from fdroidserver.exception import MetaDataException
 from fdroidserver.common import DEFAULT_LOCALE
+from fdroidserver._yaml import yaml
 from .shared_test_code import TmpCwd, mkdtemp
 
 
@@ -178,7 +179,6 @@ class MetadataTest(unittest.TestCase):
     def test_valid_funding_yml_regex(self):
         """Check the regex can find all the cases"""
         with (basedir / 'funding-usernames.yaml').open() as fp:
-            yaml = ruamel.yaml.YAML(typ='safe')
             data = yaml.load(fp)
 
         for k, entries in data.items():
@@ -207,7 +207,6 @@ class MetadataTest(unittest.TestCase):
         fdroidserver.common.config = config
         fdroidserver.metadata.warnings_action = None
 
-        yaml = ruamel.yaml.YAML(typ='safe')
         apps = fdroidserver.metadata.read_metadata()
         for appid in (
             'app.with.special.build.params',
@@ -337,7 +336,6 @@ class MetadataTest(unittest.TestCase):
 
     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)))
@@ -378,7 +376,6 @@ class MetadataTest(unittest.TestCase):
     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))
@@ -441,7 +438,6 @@ class MetadataTest(unittest.TestCase):
     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.assertEqual(yamldata['AllowedAPKSigningKeys'], [v])
@@ -2287,7 +2283,6 @@ class PostMetadataParseTest(unittest.TestCase):
         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_int(sha256, int(str_sha256)))

tests/test_publish.py

@@ -13,7 +13,6 @@
 import json
 import os
 import pathlib
-import ruamel.yaml
 import shutil
 import sys
 import unittest
@@ -24,6 +23,7 @@ from fdroidserver import publish
 from fdroidserver import common
 from fdroidserver import metadata
 from fdroidserver import signatures
+from fdroidserver._yaml import yaml
 from fdroidserver.exception import FDroidException
 from .shared_test_code import mkdtemp, VerboseFalseOptions
 
@@ -116,7 +116,6 @@ class PublishTest(unittest.TestCase):
         }
         self.assertEqual(expected, common.load_stats_fdroid_signing_key_fingerprints())
 
-        yaml = ruamel.yaml.YAML(typ='safe')
         with open(common.CONFIG_FILE) as fp:
             config = yaml.load(fp)
         self.assertEqual(