Skip to content

Commit b5070da

Browse files
committed
fix(config): read the config file as UTF-8 rather than the platform codec
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 <risky9763@gmail.com>
1 parent 21a0370 commit b5070da

4 files changed

Lines changed: 137 additions & 1 deletion

File tree

symphony/bdk/core/config/loader.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ def load_from_file(cls, config_path: str) -> BdkConfig:
2424
"""
2525
config_path = Path(config_path)
2626
if config_path.exists():
27-
config_content = config_path.read_text()
27+
config_content = config_path.read_text(encoding="utf-8")
2828
return cls.load_from_content(config_content)
2929
raise BdkConfigError(f"Config file has not been found at: {config_path.absolute()}")
3030

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import os
2+
import subprocess
3+
import sys
4+
import textwrap
5+
6+
import pytest
7+
8+
from symphony.bdk.core.config.loader import BdkConfigLoader
9+
from tests.utils.resource_utils import get_config_resource_filepath
10+
11+
# YAML and JSON are both specified as UTF-8. Reading a config with the platform
12+
# codec instead produced no error on a codepage that happens to map the bytes:
13+
# the bot started with a mojibake proxy password and failed later against the
14+
# proxy with an unrelated-looking error.
15+
EXPECTED_PASSWORD = "sésame-café"
16+
EXPECTED_USERNAME = "café-user"
17+
18+
# The importers run in a child interpreter with a legacy locale forced on, so
19+
# the test is meaningful on a UTF-8 CI host too. Without this it passes whether
20+
# or not the fix is present, everywhere the locale is already UTF-8.
21+
LEGACY_LOCALE_ENV = {
22+
"PYTHONUTF8": "0",
23+
"PYTHONCOERCECLOCALE": "0",
24+
"LC_ALL": "C",
25+
"LANG": "C",
26+
}
27+
28+
LOAD_SCRIPT = textwrap.dedent(
29+
"""
30+
import json, sys
31+
from symphony.bdk.core.config.loader import BdkConfigLoader
32+
33+
config = BdkConfigLoader.load_from_file(sys.argv[1])
34+
sys.stdout.buffer.write(
35+
json.dumps(
36+
{"username": config.proxy.username, "password": config.proxy.password},
37+
ensure_ascii=False,
38+
).encode("utf-8")
39+
)
40+
"""
41+
)
42+
43+
44+
@pytest.fixture(name="utf8_config_path", params=["config_utf8.json", "config_utf8.yaml"])
45+
def fixture_utf8_config_path(request):
46+
return get_config_resource_filepath(request.param)
47+
48+
49+
def test_load_from_file_reads_utf8_under_legacy_locale(utf8_config_path, tmp_path):
50+
"""A UTF-8 config loads identically regardless of the host locale."""
51+
script = tmp_path / "load.py"
52+
script.write_text(LOAD_SCRIPT, encoding="utf-8")
53+
54+
completed = subprocess.run(
55+
[sys.executable, str(script), utf8_config_path],
56+
capture_output=True,
57+
env={**os.environ, **LEGACY_LOCALE_ENV},
58+
)
59+
60+
# Catches the loud failure: on a locale that cannot map the bytes at all,
61+
# read_text raises and the bot never starts.
62+
assert completed.returncode == 0, completed.stderr.decode("utf-8", "replace")
63+
64+
# Catches the silent one. Asserting on bytes decoded as UTF-8 is the point:
65+
# comparing strings that came back through the same broken default would
66+
# round-trip the mojibake and pass either way.
67+
import json
68+
69+
loaded = json.loads(completed.stdout.decode("utf-8"))
70+
assert loaded["password"] == EXPECTED_PASSWORD
71+
assert loaded["username"] == EXPECTED_USERNAME
72+
73+
74+
def test_load_from_file_matches_load_from_content(utf8_config_path):
75+
"""The two entry points agree.
76+
77+
load_from_content is handed already-decoded text and was always correct,
78+
so it serves as the oracle for what load_from_file should produce.
79+
"""
80+
from pathlib import Path
81+
82+
from_file = BdkConfigLoader.load_from_file(utf8_config_path)
83+
from_content = BdkConfigLoader.load_from_content(
84+
Path(utf8_config_path).read_text(encoding="utf-8")
85+
)
86+
87+
assert from_file.proxy.password == from_content.proxy.password
88+
assert from_file.proxy.password == EXPECTED_PASSWORD
89+
90+
91+
def test_ascii_config_is_unaffected():
92+
"""The ASCII path the existing fixtures cover is unchanged.
93+
94+
A control: this passes with and without the fix, so a failure here means
95+
the harness broke rather than the encoding handling.
96+
"""
97+
config = BdkConfigLoader.load_from_file(get_config_resource_filepath("config.yaml"))
98+
assert config.bot.username == "youbot"
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"_version": "2.0",
3+
4+
"pod": { "host": "devx1.symphony.com" },
5+
"agent": { "host": "devx1.symphony.com" },
6+
"keyManager": { "host": "devx1.symphony.com" },
7+
"bot": {
8+
"username": "youbot",
9+
"privateKey": { "path": "/Users/local/conf/agent/privatekey.pem" }
10+
},
11+
"proxy": {
12+
"host": "proxy.symphony.com",
13+
"port": 1234,
14+
"username": "café-user",
15+
"password": "sésame-café"
16+
}
17+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
_version: '2.0'
2+
3+
pod:
4+
host: devx1.symphony.com
5+
6+
agent:
7+
host: devx1.symphony.com
8+
9+
keyManager:
10+
host: devx1.symphony.com
11+
12+
bot:
13+
username: youbot
14+
privateKey:
15+
path: /Users/local/conf/agent/privatekey.pem
16+
17+
proxy:
18+
host: proxy.symphony.com
19+
port: 1234
20+
username: café-user
21+
password: sésame-café

0 commit comments

Comments
 (0)