Stream: git-wasmtime

Topic: wasmtime / PR #14988 wasi-http: validate the Host header ...

view this post on Zulip Wasmtime GitHub notifications bot (Oct 27 2026 at 09:22):

xia-chao edited PR #14988.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 27 2026 at 09:23):

xia-chao commented on PR #14988:

one helper is fine. but p2 still looks up authority in p2/types.rs, and that's the 500 on a non-ASCII Host -- let _ = authority isn't dedup. also the serve tests send no Host, and rfc 9112 3.2 makes that a client must, so i fixed the client, not the server.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 27 2026 at 10:29):

:memo: xia-chao submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 27 2026 at 10:29):

:speech_balloon: xia-chao created PR review comment:

can't be just let _ = authority; -- for the 400s the helper returns Err and it has to go through tx or serve sends 500.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 27 2026 at 10:31):

xia-chao deleted a comment on PR #14988:

one helper is fine. but p2 still looks up authority in p2/types.rs, and that's the 500 on a non-ASCII Host -- let _ = authority isn't dedup. also the serve tests send no Host, and rfc 9112 3.2 makes that a client must, so i fixed the client, not the server.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 27 2026 at 18:47):

:memo: pchickey submitted PR review:

Thanks, this restructure helps a lot, I have just a couple more notes for how we should test this to make it super clear we covered all the bases

view this post on Zulip Wasmtime GitHub notifications bot (Oct 27 2026 at 18:47):

:speech_balloon: pchickey created PR review comment:

Feels off to require a byte slice argument but return a String, it could return a Vec<u8> and the comparisons can still be .starts_with(b"HTTP ...").

view this post on Zulip Wasmtime GitHub notifications bot (Oct 27 2026 at 18:47):

:speech_balloon: pchickey created PR review comment:

I don't see any reason to check for the HTML contents in these these tests, the starts with status line is sufficient

view this post on Zulip Wasmtime GitHub notifications bot (Oct 27 2026 at 18:47):

:speech_balloon: pchickey created PR review comment:

This could be folded down to if let Err(e) = get_request_target(...) { let _ = tx.send(Err(e)); wasmtime::bail!(...) }

view this post on Zulip Wasmtime GitHub notifications bot (Oct 27 2026 at 18:47):

:speech_balloon: pchickey created PR review comment:

Lets restructure these tests: we want to test that, for a given set of requests that check one positive case and various negative cases for the the host headers, both the p2 and p3 implementations of wasmtime-wasi-http handle it properly. So, lets make a single function async fn test_rfc_9112_sec_3_2(&WasmtimeServe) -> Result<()> that sends all the requests and asserts the responses are what we want, and then invoke it in just two #[tokio::test] async fn pN_cli_serve_rfc_9112_sec_3_2() -> Result<()> { ... } , one of which uses P2_CLI_SERVE_HELLO_WORLD_COMPONENT and the other uses the P3_ one.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 10:47):

xia-chao updated PR #14988.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 10:49):

xia-chao commented on PR #14988:

Done.Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 10:57):

xia-chao edited a comment on PR #14988:

Done. Thanks!
#14983 and #14993 have been folded into this PR.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 11:15):

xia-chao updated PR #14988.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 11:20):

xia-chao deleted a comment on PR #14988:

Done. Thanks!
#14983 and #14993 have been folded into this PR.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 11:20):

xia-chao commented on PR #14988:

Done. Thanks!
#14983 and #14993 have been folded into this PR, which already covers #15023.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 16:06):

xia-chao updated PR #14988.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 16:10):

xia-chao commented on PR #14988:

Rebased onto the merged #14908: HTTP/2 requests carry the authority in :authority and often omit Host, so a missing Host is now only an error for HTTP/1.1. Added h2 coverage for missing, duplicate, invalid, and disagreeing Host values.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 16:24):

xia-chao updated PR #14988.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 17:43):

xia-chao updated PR #14988.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 17:58):

:thumbs_up: pchickey submitted PR review:

Thanks for seeing this through

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 17:58):

pchickey added PR #14988 wasi-http: validate the Host header in one place to the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 18:22):

:check: pchickey merged PR #14988.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 28 2026 at 18:22):

pchickey removed PR #14988 wasi-http: validate the Host header in one place from the merge queue.

Last updated: Oct 31 2026 at 23:03 UTC