brbzull0 commented on PR #13636: URL: https://github.com/apache/trafficserver/pull/13636#issuecomment-5631688367
Copilot's latest review left three suppressed findings rather than inline comments, so answering them here. None change this PR; two belong in #13655. - **`_read_insert_without_name_ref()` does not release `name` when the value decode fails.** Correct, and only reachable once the guards in #13655 return on failure; on master that path falls through instead. The one-line release sits inside the guard body #13655 rewrites, so it goes there, not here, to keep the two branches merging cleanly. - **No encoder-stream test for the Insert Without Name Ref branch.** Correct. It also has to wait for #13655: on master a failed decode on that path re-reads the same bytes indefinitely, so a regression test would not terminate. `test_QPACK.cc` already has the stream harness for it. - **The repeated-decode test does not detect a reverted free order.** Correct, and the description says so. `QPACK::_arena` is private, so that test shows the path decodes correctly; the free-order invariant is asserted in `test_arena.cc`, where swapping the order fails it. Catching it through `QPACK` would need a test-only accessor, which this PR does not add. -- 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]
