Copilot commented on code in PR #13831:
URL: https://github.com/apache/trafficserver/pull/13831#discussion_r4268835310
##########
tests/gold_tests/pluginTest/traffic_dump/traffic_dump_response_body.test.py:
##########
@@ -79,6 +80,11 @@
ts_dump_3 = os.path.join(replay_dir, "127", "0000000000000003")
ts.Disk.File(ts_dump_3, exists=True)
+utf8_bodies = ['café 中文 😀', '中文 "café" \\ 😀\n']
Review Comment:
The second body contains a literal newline (`\\n`), which means the
constructed command string will contain an actual newline character. That can
split the command across lines (or otherwise confuse the harness), making this
AuTest flaky or outright failing depending on how `Command` is executed. Prefer
passing the expected body via a file/stdin, or pass an escaped representation
(e.g., base64/hex) that `verify_replay.py` decodes, so the command line never
contains raw newlines.
##########
tests/gold_tests/pluginTest/traffic_dump/traffic_dump_response_body.test.py:
##########
@@ -142,3 +148,11 @@
tr.Processes.Default.ReturnCode = 0
tr.StillRunningAfter = server
tr.StillRunningAfter = ts
+
+for dump_file, body in zip(utf8_dumps, utf8_bodies):
+ tr = Test.AddTestRun("Verify UTF-8 response bytes survive JSON
serialization.")
+ tr.Setup.CopyAs(verify_replay, Test.RunDirectory)
+ tr.Processes.Default.Command = f'{verify_command_prefix} {dump_file}
--response_body {shlex.quote(body)}'
Review Comment:
The second body contains a literal newline (`\\n`), which means the
constructed command string will contain an actual newline character. That can
split the command across lines (or otherwise confuse the harness), making this
AuTest flaky or outright failing depending on how `Command` is executed. Prefer
passing the expected body via a file/stdin, or pass an escaped representation
(e.g., base64/hex) that `verify_replay.py` decodes, so the command line never
contains raw newlines.
##########
tests/gold_tests/pluginTest/traffic_dump/verify_replay.py:
##########
@@ -338,10 +338,10 @@ def verify_proxy_response_body_bytes(replay_json,
expected_body_bytes):
print("The replay file did not have a server-response content size
element in the first transaction.")
return False
- if int(server_response_body_size) != len(expected_body_bytes):
+ expected_size = len(expected_body_bytes.encode('utf-8'))
Review Comment:
`expected_body_bytes` is encoded unconditionally. If callers pass actual
`bytes` (the name and the function name both suggest that), this will raise
(`AttributeError: 'bytes' object has no attribute 'encode'`). Make the size
calculation accept both `str` and `bytes` (e.g., branch on type) or ensure the
variable/parameter is consistently a `str` and rename it accordingly to avoid
confusion.
##########
tests/gold_tests/pluginTest/traffic_dump/traffic_dump_response_body.test.py:
##########
@@ -142,3 +148,11 @@
tr.Processes.Default.ReturnCode = 0
tr.StillRunningAfter = server
tr.StillRunningAfter = ts
+
+for dump_file, body in zip(utf8_dumps, utf8_bodies):
+ tr = Test.AddTestRun("Verify UTF-8 response bytes survive JSON
serialization.")
Review Comment:
This test run name is reused for both UTF-8 cases, which makes failures
harder to distinguish in logs/results. Consider including the index/uuid/path
(or a short label like 'case 4/5') in the test run name so it’s immediately
clear which body failed.
##########
tests/gold_tests/pluginTest/traffic_dump/traffic_dump_response_body.test.py:
##########
@@ -142,3 +148,11 @@
tr.Processes.Default.ReturnCode = 0
tr.StillRunningAfter = server
tr.StillRunningAfter = ts
+
+for dump_file, body in zip(utf8_dumps, utf8_bodies):
+ tr = Test.AddTestRun("Verify UTF-8 response bytes survive JSON
serialization.")
+ tr.Setup.CopyAs(verify_replay, Test.RunDirectory)
+ tr.Processes.Default.Command = f'{verify_command_prefix} {dump_file}
--response_body {shlex.quote(body)}'
Review Comment:
`dump_file` is interpolated into the command without quoting/escaping. If
the run directory path ever contains spaces or shell-metacharacters, the test
can break. Quote `dump_file` consistently (or, if supported by the test
harness, provide the command as an argv list to avoid shell-quoting issues
entirely).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
