Skip to content

Commit d4e092e

Browse files
LHMQ878copybara-github
authored andcommitted
fix: read and write CLI text files as UTF-8
Merge #6763 Ensures all text file operations in the CLI package explicitly specify UTF-8 encoding rather than relying on the platform default locale. This supersedes earlier piecemeal fixes (#2049, #5820, #6288, #6298) with a package-wide invariant and regression test. PiperOrigin-RevId: 993702704
1 parent 6be386d commit d4e092e

7 files changed

Lines changed: 172 additions & 9 deletions

File tree

‎src/google/adk/cli/_telemetry/_metrics_collector.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ def _is_rate_limited() -> bool:
135135
if not os.path.exists(_constants.LOCK_FILE):
136136
return False
137137
try:
138-
with open(_constants.LOCK_FILE, "r") as f:
138+
with open(_constants.LOCK_FILE, "r", encoding="utf-8") as f:
139139
lock_time = float(f.read().strip())
140140
# If current time is less than the lock endpoint, we are rate limited.
141141
return time.time() < lock_time

‎src/google/adk/cli/_telemetry/_metrics_reporter.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ def _set_rate_limit_timestamp(wait_ms: int) -> None:
106106
try:
107107
lock_time = time.time() + (wait_ms / 1000.0)
108108
os.makedirs(os.path.dirname(_constants.LOCK_FILE), exist_ok=True)
109-
with open(_constants.LOCK_FILE, "w") as lf:
109+
with open(_constants.LOCK_FILE, "w", encoding="utf-8") as lf:
110110
lf.write(str(lock_time))
111111
except Exception: # pylint: disable=broad-exception-caught
112112
pass

‎src/google/adk/cli/cli_deploy.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,7 @@ def _get_ignore_patterns_func(
686686
if os.path.exists(filepath):
687687
click.echo(f'Reading ignore patterns from {filename}...')
688688
try:
689-
with open(filepath, 'r') as f:
689+
with open(filepath, 'r', encoding='utf-8') as f:
690690
for line in f:
691691
line = line.strip()
692692
if line and not line.startswith('#'):
@@ -1150,7 +1150,7 @@ def to_agent_engine(
11501150
click.echo(
11511151
f'Reading agent platform config from {agent_engine_config_file}'
11521152
)
1153-
with open(agent_engine_config_file, 'r') as f:
1153+
with open(agent_engine_config_file, 'r', encoding='utf-8') as f:
11541154
agent_config = json.load(f)
11551155
if display_name:
11561156
if 'display_name' in agent_config:

‎src/google/adk/cli/cli_tools_click.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1769,10 +1769,10 @@ def cli_add_eval_case(
17691769
eval_sets_manager = get_eval_sets_manager(eval_storage_uri, agents_dir)
17701770

17711771
try:
1772-
with open(session_input_file, "r") as f:
1772+
with open(session_input_file, "r", encoding="utf-8") as f:
17731773
session_input = SessionInput.model_validate_json(f.read())
17741774

1775-
with open(scenarios_file, "r") as f:
1775+
with open(scenarios_file, "r", encoding="utf-8") as f:
17761776
conversation_scenarios = ConversationScenarios.model_validate_json(
17771777
f.read()
17781778
)
@@ -1883,7 +1883,7 @@ def cli_generate_eval_cases(
18831883
else:
18841884
click.echo(f"Eval set '{eval_set_id}' already exists.")
18851885

1886-
with open(user_simulation_config_file, "r") as f:
1886+
with open(user_simulation_config_file, "r", encoding="utf-8") as f:
18871887
config = ConversationGenerationConfig.model_validate_json(f.read())
18881888

18891889
generator = ScenarioGenerator()

‎src/google/adk/cli/conformance/_generate_markdown_utils.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ def generate_markdown_report(
6666

6767
streaming_modes.sort()
6868

69-
with open(report_path, "w") as f:
69+
with open(report_path, "w", encoding="utf-8") as f:
7070
f.write("# ADK Python Conformance Test Report\n\n")
7171
f.write("## Summary\n\n")
7272
f.write(f"- **ADK Version**: {server_version}\n")

‎tests/unittests/cli/conformance/test_generate_markdown_utils.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ def _report_text(tmp_path, version_data, summaries):
4343
generate_markdown_report(version_data, summaries, str(tmp_path))
4444
written = list(tmp_path.glob('*.md'))
4545
assert len(written) == 1, written
46-
return written[0], written[0].read_text()
46+
# The report is written as UTF-8; read it back the same way rather than
47+
# through the platform locale, which fails on non-UTF-8 defaults (cp936 etc.).
48+
return written[0], written[0].read_text(encoding='utf-8')
4749

4850

4951
def test_generate_markdown_report_names_the_file_after_the_server_version(
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
# Copyright 2026 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Tests that the CLI reads and writes text files as UTF-8.
16+
17+
`open()` without an explicit `encoding` uses the platform's locale encoding,
18+
which is not UTF-8 on a large share of Windows installs (`cp936` for zh-CN,
19+
`cp932` for ja-JP, `cp1252` for much of the West). The CLI's JSON and Markdown
20+
files carry agent prompts, model responses and display names, so they routinely
21+
hold non-ASCII text -- and JSON is defined as UTF-8 for interchange (RFC 8259
22+
§8.1). Reading those files under a non-UTF-8 locale either raises
23+
`UnicodeDecodeError` or, when the UTF-8 bytes happen to form a valid sequence in
24+
the locale codec, silently yields mojibake.
25+
26+
CI runs on a UTF-8 default, so a test that merely reads a UTF-8 file cannot
27+
catch a missing `encoding=`. `_non_utf8_default_locale` therefore simulates the
28+
locale by forcing a non-UTF-8 codec onto every `open()` call that omits
29+
`encoding`, which makes these tests fail on any platform if the argument is
30+
dropped again.
31+
"""
32+
33+
from __future__ import annotations
34+
35+
import ast
36+
import builtins
37+
import contextlib
38+
import pathlib
39+
from typing import Iterator
40+
41+
import google.adk.cli as adk_cli
42+
from google.adk.cli.cli_deploy import _get_ignore_patterns_func
43+
import pytest
44+
45+
_NON_ASCII = "北京天气-café-🌤"
46+
47+
48+
@contextlib.contextmanager
49+
def _non_utf8_default_locale(codec: str = "ascii") -> Iterator[None]:
50+
"""Forces `codec` onto text-mode `open()` calls that omit `encoding`.
51+
52+
Patching `locale.getpreferredencoding` does not work: CPython resolves the
53+
default text encoding internally, so `open()` ignores it. Wrapping
54+
`builtins.open` is what actually reproduces a non-UTF-8 locale on a UTF-8 CI
55+
machine.
56+
57+
`ascii` is the default because it rejects every non-ASCII byte, so a missing
58+
`encoding=` fails deterministically. A real locale codec is laxer: cp936
59+
accepts most UTF-8 byte pairs and yields mojibake instead of raising, which
60+
`test_locale_codec_can_corrupt_silently` covers separately.
61+
"""
62+
real_open = builtins.open
63+
64+
def patched_open( # pylint: disable=redefined-builtin
65+
file, mode="r", buffering=-1, encoding=None, *args, **kwargs
66+
):
67+
if "b" not in mode and encoding is None:
68+
encoding = codec
69+
return real_open(file, mode, buffering, encoding, *args, **kwargs)
70+
71+
builtins.open = patched_open
72+
try:
73+
yield
74+
finally:
75+
builtins.open = real_open
76+
77+
78+
def test_non_utf8_default_locale_helper_actually_bites(tmp_path):
79+
"""Guards the guard: the helper must really change `open()`'s behavior."""
80+
path = tmp_path / "probe.txt"
81+
path.write_text(_NON_ASCII, encoding="utf-8")
82+
83+
with _non_utf8_default_locale():
84+
with pytest.raises(UnicodeDecodeError):
85+
with open(path) as f: # no encoding= -> forced cp936
86+
f.read()
87+
88+
# An explicit encoding still wins over the simulated locale.
89+
with open(path, encoding="utf-8") as f:
90+
assert f.read() == _NON_ASCII
91+
92+
93+
def test_locale_codec_can_corrupt_silently(tmp_path):
94+
"""Documents the quieter failure mode: no exception, wrong text.
95+
96+
cp936 decodes most UTF-8 byte pairs without complaint, so a missing
97+
`encoding=` on a zh-CN Windows box can persist mojibake into an eval set
98+
instead of raising.
99+
"""
100+
path = tmp_path / "probe.txt"
101+
path.write_text(_NON_ASCII, encoding="utf-8")
102+
103+
with _non_utf8_default_locale("cp936"):
104+
with open(path) as f:
105+
decoded = f.read()
106+
107+
assert decoded != _NON_ASCII # silently wrong, and no error was raised
108+
109+
110+
def test_get_ignore_patterns_reads_utf8_ignore_file(tmp_path):
111+
"""`.gitignore` entries with non-ASCII names survive a non-UTF-8 locale."""
112+
(tmp_path / ".gitignore").write_text(
113+
f"# comment\n{_NON_ASCII}/\n__pycache__\n", encoding="utf-8"
114+
)
115+
116+
with _non_utf8_default_locale():
117+
ignore_func = _get_ignore_patterns_func(str(tmp_path))
118+
119+
patterns = ignore_func(str(tmp_path), [f"{_NON_ASCII}", "__pycache__"])
120+
assert _NON_ASCII in patterns
121+
assert "__pycache__" in patterns
122+
123+
124+
def _text_mode_open_calls_without_encoding(root: pathlib.Path) -> list[str]:
125+
"""Returns `file:line` for each text-mode `open()` that omits `encoding`."""
126+
offenders = []
127+
for path in sorted(root.rglob("*.py")):
128+
tree = ast.parse(path.read_text(encoding="utf-8"))
129+
for node in ast.walk(tree):
130+
# Bare `open(...)` only: `ZipFile.open`/`z.open` are binary and take no
131+
# `encoding`, and `os.fdopen` sites here are binary too.
132+
if (
133+
not isinstance(node, ast.Call)
134+
or getattr(node.func, "id", None) != "open"
135+
):
136+
continue
137+
if any(kw.arg == "encoding" for kw in node.keywords):
138+
continue
139+
mode = "r"
140+
if len(node.args) >= 2 and isinstance(node.args[1], ast.Constant):
141+
mode = node.args[1].value
142+
for kw in node.keywords:
143+
if kw.arg == "mode" and isinstance(kw.value, ast.Constant):
144+
mode = kw.value.value
145+
if isinstance(mode, str) and "b" not in mode:
146+
offenders.append(f"{path}:{node.lineno}")
147+
return offenders
148+
149+
150+
def test_cli_package_has_no_implicit_encoding_open():
151+
"""Every text-mode `open()` under `cli/` must pass `encoding` explicitly.
152+
153+
Ensures all text file operations across the CLI package use an explicit
154+
encoding rather than relying on the host platform's default locale.
155+
"""
156+
root = pathlib.Path(adk_cli.__file__).parent
157+
offenders = _text_mode_open_calls_without_encoding(root)
158+
assert not offenders, (
159+
"text-mode open() without encoding= (locale-dependent, breaks on"
160+
f" non-UTF-8 locales): {offenders}"
161+
)

0 commit comments

Comments
 (0)