Fix flaky filter_body autest for request blocking #12876
Conversation
The request-block test asserted proxy-response status 502, but this is inherently racy. Request body transforms run after headers are sent to the origin, so the origin may respond before ATS acts on the block. The client can see a 502, a closed connection, or the origin's 200 response depending on timing. Remove the proxy-response status assertion and rely on the diags.log checks which verify the plugin correctly matched the rule and took the blocking action. Fixes apache#12875
| @@ -194,8 +198,8 @@ sessions: | |||
| content: | |||
| data: "OK" | |||
|
|
|||
| proxy-response: | |||
| status: 502 | |||
| # Note: proxy-response status is not checked because request body | |||
| # blocking is inherently racy. See comment above. | |||
There was a problem hiding this comment.
But, isn't ATS supposed to be dropping this connection before the request body is sent? I'm pretty sure Proxy Verifier server will await the 49 bytes before sending a response.
If this test fails sometimes, I'm concerned that the block functionality of the plugin itself is flakey, not the test.
I disagree, Claude. The meaningful check is whether the body got blocked or whether it incorrectly leaked to the origin. Logs are nice for identifying offensive traffic, but if the body gets to the origin, but our logs say we blocked it, users will be pretty unhappy. If the offensive body triggers a crash on the origin, or remote executes code, for example, but ATS said it blocked it when it didn't, no one is going to be happy with ATS's behavior. |
2 participants
Summary
proxy-response: status: 502assertion from the request-block test because the response status is inherently racy -- the origin may respond before ATS acts on the blockDetails
Request body transforms run after headers are sent to the origin. By the time the body is inspected and the plugin decides to block, the origin may have already responded. The client can see a 502, a closed connection, or the origin's 200 response depending on timing.
The diags.log validations already verify:
Matched rule: xxe_request_block)Blocking request due to rule)These are the meaningful checks -- the HTTP status code to the client is a side effect of the race.
Fixes #12875