bryancall commented on PR #13420: URL: https://github.com/apache/trafficserver/pull/13420#issuecomment-5746879974
Checking in on this one. It has been a month since I requested changes, and the branch has gone to conflicting with master in the meantime, so it needs a rebase before it can be evaluated again. Both things I asked for are small: 1. Delete `tests/gold_tests/h2/http2_fc_iso.test.py`. It is a 348-line scratch copy of `http2_flow_control.test.py` with no new coverage, and it carries `proxy.config.diags.debug.enabled: 3`, so it grows `traffic.out` on every CI run and for everyone running the h2 gold tests locally. 2. One negative scenario per fast-path gate in `h2_request_handling.replay.yaml`. Every proxy-response in the added replay is 200, 204 or 304, so there is no test that fails if the `&&` chain is later reordered and `:authority: [email protected]` reaches the origin unvalidated. None of that is a defect in the merged code path, and I want to be clear the review was positive on the substance: I read the whole diff, the in-place HPACK decode out of the frame reader is genuinely safe, and the authority parsing change closes a real hole. I would rather this land than rot. If you are short of time, say so and I will look at whether the test gap can reasonably move to a follow-up issue so the rest is not held up. Are you still planning to take this one forward? -- 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]
