xofyarg requested wasmtime-wasi-reviewers for a review on PR #11292.
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:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
xofyarg updated PR #11292.
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.
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
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.
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.
xofyarg updated PR #11292.
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.
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
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.
xofyarg requested wasmtime-default-reviewers for a review on PR #11292.
xofyarg updated PR #11292.
xofyarg requested fitzgen for a review on PR #11292.
rvolosatovs submitted PR review.
rvolosatovs has enabled auto merge for PR #11292.
xofyarg commented on PR #11292:
I put a note in the RELEASES.md file, please take another look.
rvolosatovs merged PR #11292.
Last updated: Dec 06 2025 at 06:05 UTC