Stream: git-wasmtime

Topic: wasmtime / PR #11292 allow configurable forbidden header


view this post on Zulip Wasmtime GitHub notifications bot (Jul 22 2025 at 01:22):

xofyarg requested wasmtime-wasi-reviewers for a review on PR #11292.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 22 2025 at 01:22):

xofyarg opened PR #11292 from xofyarg:wasi-http-configurable-forbidden-header to bytecodealliance:main:

When the HTTP library is not used as a proxy, removing of the forbidden headers by default may not make sense. This change makes it configurable via the WasiHttpView, which keeps the default behavior.

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Jul 22 2025 at 02:31):

xofyarg updated PR #11292.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 22 2025 at 02:35):

xofyarg commented on PR #11292:

Not sure which is better: 3c9f4c1 looks clean to me, but it changes previous behavior if user has implemented is_forbidden_header; d183eff adds an extra method.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 22 2025 at 07:42):

rvolosatovs submitted PR review:

In p3 I've fixed this in the following manner: https://github.com/bytecodealliance/wasip3-prototyping/blob/main/crates%2Fwasi-http%2Fsrc%2Fp3%2Fmod.rs#L381-L387

This way users can choose the headers to deny with no breaking changes to the API

That said, IMO this diff is already an improvement

view this post on Zulip Wasmtime GitHub notifications bot (Jul 22 2025 at 15:40):

xofyarg commented on PR #11292:

Should I adjust the change close to p3 then? There are two commits in the PR, I can either drop the 2nd, or squash them together. Please let me know what you think. Thanks.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 22 2025 at 16:01):

xofyarg edited a comment on PR #11292:

Should I adjust the change close to p3 then? There are two commits in the PR, I can either drop the 2nd, or squash them together. Please let me know what you think. Thanks.

Edit:

choose the headers to deny with no breaking changes to the API

The way it works in p2 is, when user override this trait method, the new customized headers as well as the default forbidden headers are denied: https://github.com/bytecodealliance/wasmtime/blob/v34.0.2/crates/wasi-http/src/types.rs#L287

I prefer the way how p3 is implemented, but it means a break change to the current API. Otherwise we have to introduce something new, which will then be dropped when the user upgrade to p3, which also not nice.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 23 2025 at 02:25):

xofyarg updated PR #11292.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 23 2025 at 02:45):

xofyarg commented on PR #11292:

I updated the PR to follow the p3 API, that should give the users a smooth path forward. Although it has a break change for people who override the trait method. The PR is ready to be merged.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 23 2025 at 09:28):

rvolosatovs submitted PR review:

LGTM, thank you!
As you correctly point out, this is actually is a breaking change for users providing an implementation of the method. Should we record this in release notes? I'm not actually sure about the expectations/procedure here
cc @bytecodealliance/wasmtime-wasi-reviewers

view this post on Zulip Wasmtime GitHub notifications bot (Jul 23 2025 at 14:48):

alexcrichton commented on PR #11292:

Breaking changes are fine yeah, and if you're able to write a note in the release notes that'd be appreciated, but it's not required. When I go back and review merged PRs for the release notes I try to catch issues like this and write them in.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 23 2025 at 15:09):

xofyarg requested wasmtime-default-reviewers for a review on PR #11292.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 23 2025 at 15:09):

xofyarg updated PR #11292.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 23 2025 at 15:09):

xofyarg requested fitzgen for a review on PR #11292.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 23 2025 at 15:10):

rvolosatovs submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 23 2025 at 15:10):

rvolosatovs has enabled auto merge for PR #11292.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 23 2025 at 15:10):

xofyarg commented on PR #11292:

I put a note in the RELEASES.md file, please take another look.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 23 2025 at 15:47):

rvolosatovs merged PR #11292.


Last updated: Dec 06 2025 at 06:05 UTC