Add content delay for server chaos testing - #389
Conversation
bneradt
left a comment
bneradt
left a comment
There was a problem hiding this comment.
To fix the format, there's a tools/format.sh
I like this feature a lot, but from a usability standpoint, I would like content:delay to:
- apply across h2 and h3 as well (we don't support h3 on the server side, so this isn't applicalbe to h3 response bodies, of course).
- apply to request bodies for all protocols (I think your PR already applies to request bodies for h1, which is good).
- apply to 100-continue requests sending the body after the 100 response is received
We don't need to suport that in this PR if you don't want, but before cutting a release with this, it would be good for the project to support all of the above. And I can help with some of the development if you like. Or you can do it. Either way. Just let me know how you would like to proceed.
Thank you so much for the contribution!
Add a delay key inside a content node which pauses between writing a message's headers and writing its body. This lets a replay file simulate an origin that starts a response and then stalls, which exercises proxy timeout and partial-response handling that no existing replay construct could reach. The delay is inserted in Session::write(HttpHeader) immediately before the write_body call, so it covers HTTP/1.1 both in the clear and over TLS. HTTP/2 is unaffected because H2Session overrides that method; those messages already express the same behavior with a delay on their DATA frame, so combining a content delay with a frames node is rejected with a diagnostic pointing at the DATA frame alternative. A shutdown during the delay abandons the body write, matching how the existing transaction delay behaves. A peer which departs during the delay is the expected outcome when the delay is used to trigger a proxy timeout, so the resulting body write failure is reported through the errata and annotated with the delay as its likely cause, without affecting the process exit code. The AuTest duration verifier only matched the plural form of the client timing line, so its regex now also accepts the singular form which a single transaction replay emits.
The HTTP/2 and HTTP/3 write paths hand the whole message to nghttp2 / nghttp3 at once, so a content delay had no place to land. Instead of sleeping mid-write, withhold the body from the library: the data source callback reports NGHTTP2_ERR_DEFERRED / NGHTTP3_ERR_WOULDBLOCK while the stream's content delay is non-zero, which flushes HEADERS and stops. write() then waits out the delay, zeroes the delay, and resumes the stream so the body follows in its own DATA frame. HTTP/2 keeps draining its receive window during the wait so flow control and peer-initiated frames are not stalled. HTTP/3 deliberately does not read: ngtcp2_progress_ingress treats a poll timeout as fatal and closes the session, and silence is the point of the feature anyway. It services ngtcp2's expiry timers in 20 ms slices instead. Both write() paths now hold a shared_ptr to the stream state, since servicing the connection mid-write can retire a stream from the map. Expect: 100-continue requests still skip the delay on all three protocols, matching the existing HTTP/1 behavior. Delayed 100-continue bodies are left for a follow-up.
dc94de2 to
0ddfa2a
Compare
masaori335
changed the title
|
The latest commit has HTTP/2 and HTTP/3 support. For 100-continue, we need to start from adding it for H2 and H3, so I eave it. |
bneradt
left a comment
bneradt
left a comment
There was a problem hiding this comment.
Thank you for adding the HTTP/2 and HTTP/3 support. I found two issues in the updated delay paths that I think need to be addressed before merging.
-
Early responses can bypass verification.
HTTP/2 and HTTP/3 now process incoming traffic while
write()is waiting out a content delay, but the expected response is attached to the stream only afterwrite()returns (H2Session::run_transactionandH3Session::run_transaction). If a proxy sends a final response before the delayed request body—for example a 401, 413, or locally generated timeout—the response callbacks see a null expected-response pointer and skip status, header, and body verification. A status-onlyproxy-responsecan therefore falsely pass because status is not included in the later unprocessed-verification check.The expected response needs to be registered on the stream before the delay begins servicing ingress.
-
The stream-state lifetime guard ends too early.
The new
shared_ptrguards retain stream state only for the duration ofwrite(). In the same early-response scenario, receivingEND_STREAMremoves the state from the stream map while the delayed request body may still be flow-controlled or blocked. When the next transaction replaces_last_added_stream, nghttp2/nghttp3 can later invoke their raw user-data callbacks with a freed stream state. The owning reference needs to survive until the send side has completed or the protocol's stream-close callback fires.
One smaller documentation point: the README currently says content delay works for both request and response bodies across HTTP/1.x, HTTP/2, and HTTP/3. Since server-side HTTP/3 is not supported, this should qualify HTTP/3 as request-body support only.
The HTTP/2 and HTTP/3 happy-path coverage looks good. The missing regression case is a peer that sends a final response after receiving the headers but before the delayed request body.
HTTP/2 and HTTP/3 service ingress while write() waits out a content delay, so a peer can answer before the body goes out. Two things broke in that window: the expected response was attached to the stream only after write() returned, leaving such a response unverified, and the stream state was owned only for the duration of write(), so nghttp2/nghttp3 could call back into freed state once the peer's END_STREAM retired the stream. The expected response is now stamped on the stream when it is created, and ownership is held until the protocol library's stream-close callback. The test proxy grows an EarlyResponse directive to reproduce this: it answers on the request HEADERS instead of waiting for the whole request. Server-side HTTP/3 is not implemented, so the README qualifies HTTP/3 content delay as request bodies only.
Three ways a content delay could fail the run it was meant to exercise: the delay counted against Transaction_Delay_Cutoff, so anything at or over ten seconds -- the point of the feature -- reported the transaction as too slow; resuming the HTTP/2 DATA frame errored when the peer had already reset the stream, which is the expected outcome when the delay is used to trigger a proxy timeout; and errata.sink() cleared the severity of every drain failure, so a protocol or TLS error during the delay was reported and then swallowed, letting the client exit zero. Each is now distinguished from the failure it was masking or inventing. The HTTP/2 delay also services the session in bounded slices and checks the shutdown flag, matching interruptible_sleep_for on HTTP/1 and the HTTP/3 loop.
2 participants
Add a delay key inside a content node which pauses between writing a message's headers and writing its body. This lets a replay file simulate an origin that starts a response and then stalls, which exercises proxy timeout and partial-response handling that no existing replay construct could reach.
The delay is inserted in Session::write(HttpHeader) immediately before the write_body call, so it covers HTTP/1.1 both in the clear and over TLS. HTTP/2 is unaffected because H2Session overrides that method; those messages already express the same behavior with a delay on their DATA frame, so combining a content delay with a frames node is rejected with a diagnostic pointing at the DATA frame alternative.
A shutdown during the delay abandons the body write, matching how the existing transaction delay behaves. A peer which departs during the delay is the expected outcome when the delay is used to trigger a proxy timeout, so the resulting body write failure is reported through the errata and annotated with the delay as its likely cause, without affecting the process exit code.
The AuTest duration verifier only matched the plural form of the client timing line, so its regex now also accepts the singular form which a single transaction replay emits.
I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.