Copilot commented on code in PR #12811:
URL: https://github.com/apache/trafficserver/pull/12811#discussion_r2729607830
##########
tests/gold_tests/pluginTest/ja4_fingerprint/ja4_fingerprint.test.py:
##########
@@ -152,3 +157,43 @@ def test1(params: TestParams) -> None:
client.Streams.stdout += Testers.ContainsExpression(r'Yay!', 'We should
receive the expected body.')
params['ts'].Disk.traffic_out.Content += Testers.ContainsExpression(
r'JA4 fingerprint:', 'We should receive the expected log message.')
+
+
[email protected]('With --preserve, existing JA4 headers should be
preserved.', use_preserve=True)
+def test_preserve(params: TestParams) -> None:
+ '''Test that --preserve skips adding headers when JA4 headers exist.'''
+ tr = params['tr']
+ client = tr.Processes.Default
+ server = params['server_one']
+
+ # Request 1: No existing JA4 headers - should add them.
+ tr.MakeCurlCommand('-k -v
"https://localhost:{0}/resource";'.format(params['port_one']), ts=params['ts'])
+ client.ReturnCode = 0
+ client.Streams.stdout += Testers.ContainsExpression(r'Yay!', 'First
request should succeed.')
Review Comment:
The replay file now requires a `uuid` header (e.g. `uuid:
no-existing-headers`), but the curl request built here doesn’t send one. Proxy
Verifier will likely fail to match/validate the transaction. Either add a
matching `-H "uuid: ..."` to each curl request (for /resource,
/resource-with-headers, /resource-via-only) or remove the `uuid` requirements
from the replay YAML.
##########
tests/gold_tests/pluginTest/ja3_fingerprint/ja3_fingerprint.test.py:
##########
@@ -81,13 +81,18 @@ def _configure_server(self, tr: 'TestRun') -> None:
self._server.Streams.All +=
Testers.ContainsExpression("https-request", "Verify the HTTPS request was
received.")
self._server.Streams.All +=
Testers.ContainsExpression("http2-request", "Verify the HTTP/2 request was
received.")
if not self._test_remap:
- # Verify --preserve worked.
- self._server.Streams.All += Testers.ContainsExpression("x-ja3-raw:
.*,", "Verify the new raw header was added.")
+ # The first request has no existing JA3 headers, so headers are
added.
self._server.Streams.All += Testers.ContainsExpression(
- "x-ja3-raw: first-signature", "Verify the already-existing raw
header was preserved.")
- self._server.Streams.All += Testers.ExcludesExpression(
- "x-ja3-raw: first-signature;", "Verify no extra values were
added due to preserve.")
+ "x-ja3-raw: .*,", "Verify the new raw header was added.",
reflags=re.IGNORECASE)
self._server.Streams.All += Testers.ContainsExpression("x-ja3-via:
test.proxy.com", "The x-ja3-via string was added.")
+ # The second request has existing JA3 headers. With --preserve,
+ # no new JA3 headers are added (including x-ja3-sig).
+ self._server.Streams.All += Testers.ContainsExpression(
+ "x-ja3-raw: first-signature", "Verify the already-existing raw
header was preserved.", reflags=re.IGNORECASE)
+ self._server.Streams.All += Testers.ExcludesExpression(
+ "x-ja3-sig:.*http2-request",
+ "Verify no JA3-Sig was added when other JA3 headers existed.",
+ reflags=re.IGNORECASE | re.DOTALL)
Review Comment:
This ExcludesExpression regex can match across *both* requests in the server
log because it uses `re.DOTALL` and looks for `x-ja3-sig` before
`http2-request`. Since the first request legitimately contains `x-ja3-sig`,
this check can produce false failures/flakiness depending on log ordering.
Prefer relying on the replay.yaml per-transaction assertion (`x-ja3-sig` is
absent for the http2 transaction) or scope the regex to the `http2-request`
block (e.g., match `http2-request` first, then ensure `x-ja3-sig` does not
appear within that block).
```suggestion
```
##########
tests/gold_tests/pluginTest/ja4_fingerprint/ja4_fingerprint.replay.yaml:
##########
@@ -33,11 +35,12 @@ sessions:
fields:
- [ Connection, keep-alive ]
- [ Content-Length, 0 ]
+ - [ uuid, no-existing-headers ]
Review Comment:
The replay now requires a `uuid` header (e.g. `uuid: no-existing-headers`),
but the ja4_fingerprint tests use curl and don’t set any `uuid` header. This
will likely cause Proxy Verifier replay matching/validation failures. Either
remove the `uuid` requirements from the replay or update the curl commands to
send matching `uuid` values for each session.
--
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]
