From b5070dab3534ff14fef381e87a7facbf1f940ba0 Mon Sep 17 00:00:00 2001 From: ppcvote Date: Fri, 21 Aug 2026 04:21:41 +0800 Subject: [PATCH] fix(config): read the config file as UTF-8 rather than the platform codec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BdkConfigLoader.load_from_file called read_text() with no encoding, so the config was decoded with locale.getpreferredencoding(). YAML and JSON are both specified as UTF-8, so any non-ASCII value was decoded with the wrong codec on a non-UTF-8 host. The failure is silent where it matters. Measured on Windows with cp950, a config carrying a non-ASCII proxy credential: on disk : password: "sésame-café" (b'...s\xc3\xa9same-caf\xc3\xa9...') loaded : password='s矇same-caf矇' username: 'caf矇-user' No exception. The bot starts and fails later against the proxy with an error that points nowhere near the config. load_from_content on the same content returns the right values, which isolates it to the read. The repository already uses the correct pattern at service/user/user_service.py:798, so this is one argument for consistency with it. Two other implicit-encoding reads are deliberately left out of scope rather than bundled: bdk_rsa_key_config.py:59 reads a PEM key (base64, ASCII) and on_disk_datafeed_id_repository.py:56 reads an id this library writes itself. The test drives the loader in a child interpreter with PYTHONUTF8=0 LC_ALL=C so it reproduces on a UTF-8 CI host, and uses load_from_content as the oracle for what load_from_file should produce. Fixtures are written as raw bytes and their UTF-8 sequences verified in the index, so no locale can influence them. Verified by reverting the one-line change: 4 failed / 1 passed before, 5 passed after, the passing case being an ASCII control. Full suite: 558 passed before, 563 after, same 2 pre-existing failures. Closes #395 Signed-off-by: ppcvote --- symphony/bdk/core/config/loader.py | 2 +- tests/core/config/bdk_config_encoding_test.py | 98 +++++++++++++++++++ tests/resources/config/config_utf8.json | 17 ++++ tests/resources/config/config_utf8.yaml | 21 ++++ 4 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 tests/core/config/bdk_config_encoding_test.py create mode 100644 tests/resources/config/config_utf8.json create mode 100644 tests/resources/config/config_utf8.yaml diff --git a/symphony/bdk/core/config/loader.py b/symphony/bdk/core/config/loader.py index 5c6e5a10..bce209b3 100644 --- a/symphony/bdk/core/config/loader.py +++ b/symphony/bdk/core/config/loader.py @@ -24,7 +24,7 @@ def load_from_file(cls, config_path: str) -> BdkConfig: """ config_path = Path(config_path) if config_path.exists(): - config_content = config_path.read_text() + config_content = config_path.read_text(encoding="utf-8") return cls.load_from_content(config_content) raise BdkConfigError(f"Config file has not been found at: {config_path.absolute()}") diff --git a/tests/core/config/bdk_config_encoding_test.py b/tests/core/config/bdk_config_encoding_test.py new file mode 100644 index 00000000..68ca0bd7 --- /dev/null +++ b/tests/core/config/bdk_config_encoding_test.py @@ -0,0 +1,98 @@ +import os +import subprocess +import sys +import textwrap + +import pytest + +from symphony.bdk.core.config.loader import BdkConfigLoader +from tests.utils.resource_utils import get_config_resource_filepath + +# YAML and JSON are both specified as UTF-8. Reading a config with the platform +# codec instead produced no error on a codepage that happens to map the bytes: +# the bot started with a mojibake proxy password and failed later against the +# proxy with an unrelated-looking error. +EXPECTED_PASSWORD = "sésame-café" +EXPECTED_USERNAME = "café-user" + +# The importers run in a child interpreter with a legacy locale forced on, so +# the test is meaningful on a UTF-8 CI host too. Without this it passes whether +# or not the fix is present, everywhere the locale is already UTF-8. +LEGACY_LOCALE_ENV = { + "PYTHONUTF8": "0", + "PYTHONCOERCECLOCALE": "0", + "LC_ALL": "C", + "LANG": "C", +} + +LOAD_SCRIPT = textwrap.dedent( + """ + import json, sys + from symphony.bdk.core.config.loader import BdkConfigLoader + + config = BdkConfigLoader.load_from_file(sys.argv[1]) + sys.stdout.buffer.write( + json.dumps( + {"username": config.proxy.username, "password": config.proxy.password}, + ensure_ascii=False, + ).encode("utf-8") + ) + """ +) + + +@pytest.fixture(name="utf8_config_path", params=["config_utf8.json", "config_utf8.yaml"]) +def fixture_utf8_config_path(request): + return get_config_resource_filepath(request.param) + + +def test_load_from_file_reads_utf8_under_legacy_locale(utf8_config_path, tmp_path): + """A UTF-8 config loads identically regardless of the host locale.""" + script = tmp_path / "load.py" + script.write_text(LOAD_SCRIPT, encoding="utf-8") + + completed = subprocess.run( + [sys.executable, str(script), utf8_config_path], + capture_output=True, + env={**os.environ, **LEGACY_LOCALE_ENV}, + ) + + # Catches the loud failure: on a locale that cannot map the bytes at all, + # read_text raises and the bot never starts. + assert completed.returncode == 0, completed.stderr.decode("utf-8", "replace") + + # Catches the silent one. Asserting on bytes decoded as UTF-8 is the point: + # comparing strings that came back through the same broken default would + # round-trip the mojibake and pass either way. + import json + + loaded = json.loads(completed.stdout.decode("utf-8")) + assert loaded["password"] == EXPECTED_PASSWORD + assert loaded["username"] == EXPECTED_USERNAME + + +def test_load_from_file_matches_load_from_content(utf8_config_path): + """The two entry points agree. + + load_from_content is handed already-decoded text and was always correct, + so it serves as the oracle for what load_from_file should produce. + """ + from pathlib import Path + + from_file = BdkConfigLoader.load_from_file(utf8_config_path) + from_content = BdkConfigLoader.load_from_content( + Path(utf8_config_path).read_text(encoding="utf-8") + ) + + assert from_file.proxy.password == from_content.proxy.password + assert from_file.proxy.password == EXPECTED_PASSWORD + + +def test_ascii_config_is_unaffected(): + """The ASCII path the existing fixtures cover is unchanged. + + A control: this passes with and without the fix, so a failure here means + the harness broke rather than the encoding handling. + """ + config = BdkConfigLoader.load_from_file(get_config_resource_filepath("config.yaml")) + assert config.bot.username == "youbot" diff --git a/tests/resources/config/config_utf8.json b/tests/resources/config/config_utf8.json new file mode 100644 index 00000000..5779294a --- /dev/null +++ b/tests/resources/config/config_utf8.json @@ -0,0 +1,17 @@ +{ + "_version": "2.0", + + "pod": { "host": "devx1.symphony.com" }, + "agent": { "host": "devx1.symphony.com" }, + "keyManager": { "host": "devx1.symphony.com" }, + "bot": { + "username": "youbot", + "privateKey": { "path": "/Users/local/conf/agent/privatekey.pem" } + }, + "proxy": { + "host": "proxy.symphony.com", + "port": 1234, + "username": "café-user", + "password": "sésame-café" + } +} diff --git a/tests/resources/config/config_utf8.yaml b/tests/resources/config/config_utf8.yaml new file mode 100644 index 00000000..278b6ebb --- /dev/null +++ b/tests/resources/config/config_utf8.yaml @@ -0,0 +1,21 @@ +_version: '2.0' + +pod: + host: devx1.symphony.com + +agent: + host: devx1.symphony.com + +keyManager: + host: devx1.symphony.com + +bot: + username: youbot + privateKey: + path: /Users/local/conf/agent/privatekey.pem + +proxy: + host: proxy.symphony.com + port: 1234 + username: café-user + password: sésame-café