Skip to content

Conversation

@tbrockman
Copy link
Contributor

Discussion: #wasi > wasi-http Request and pub(crate) Body @ 💬

Largely copies the implementation from Response::into_http as suggested, contains one basic test asserting it works in the happy path.

@tbrockman tbrockman requested a review from a team as a code owner October 13, 2025 03:36
Copy link
Contributor

@dicej dicej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this; LGTM.

Please run cargo fmt when you have a chance, which should address the CI failure.

@alexcrichton
Copy link
Member

Would it also be possible to update the default_send_request to use this new code path as well? I would naively expect the two to be pretty similar in what they're doing.

@tbrockman
Copy link
Contributor Author

tbrockman commented Oct 13, 2025

Thanks for doing this; LGTM.

Please run cargo fmt when you have a chance, which should address the CI failure.

No problem! Seems a fair exchange for the past/future questions I'll have about wasmtime 😬

Would it also be possible to update the default_send_request to use this new code path as well? I would naively expect the two to be pretty similar in what they're doing.

I looked into this, but I think I might not quite understand the ask as the req parameter to default_send_request already appears to be an http::Request?

Edit: Ah, guessing you meant this code which eventually calls default_send_request, which does indeed have some overlap here (if you can confirm that this is what you meant I can dig more into it).

@alexcrichton
Copy link
Member

Ah yes indeed! That eventually bottoms out here and I think yeah you're right that what you've added here is more suitable for replacing/augmenting the implementation of handle as opposed to default_send_request itself (which is already working at the abstraction level of Hyper types)

@tbrockman
Copy link
Contributor Author

@alexcrichton So I looked into it a bit, and given my limited understanding of handle and how it uses various channels (and perhaps channels, pinning, and tasks in Rust in general), it's not immediately straightforward to me how to integrate Request::into_http in a way that doesn't require adding too much complexity to Request::into_http, without prompting workarounds/redundant code to preserve existing functionality in handle 🤔

Might need a bit of hand holding here 😅

@alexcrichton
Copy link
Member

Ah ok no worries, I can try to take a look after this merges

@alexcrichton alexcrichton added this pull request to the merge queue Oct 14, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 14, 2025
@tbrockman
Copy link
Contributor Author

Looks like it leaked some memory during testing as well? 😞

=================================================================
==18584==ERROR: LeakSanitizer: detected memory leaks

Indirect leak of 72 byte(s) in 1 object(s) allocated from:
    #0 0x563417655224  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x14cb224) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #1 0x563417756761  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x15cc761) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #2 0x5634176fc8f9  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x15728f9) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #3 0x5634177a923d  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x161f23d) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #4 0x5634176ae4da  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x15244da) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #5 0x56341771965c  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x158f65c) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #6 0x56341775d61d  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x15d361d) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #7 0x5634176ad96d  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x152396d) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #8 0x5634177a894d  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x161e94d) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #9 0x5634177c1c98  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x1637c98) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #10 0x5634177ffef4  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x1675ef4) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #11 0x5634177d68e3  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x164c8e3) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #12 0x5634177da219  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x1650219) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #13 0x563419c47c6e  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x3abdc6e) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #14 0x563417652b96  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x14c8b96) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)

Indirect leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x563417655224  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x14cb224) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #1 0x5634176fdb34  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x1573b34) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #2 0x5634177a95f7  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x161f5f7) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #3 0x5634176ae4da  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x15244da) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #4 0x56341771965c  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x158f65c) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #5 0x56341775d61d  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x15d361d) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #6 0x5634176ad96d  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x152396d) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #7 0x5634177a894d  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x161e94d) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #8 0x5634177c1c98  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x1637c98) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #9 0x5634177ffef4  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x1675ef4) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #10 0x5634177d68e3  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x164c8e3) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #11 0x5634177da219  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x1650219) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #12 0x563419c47c6e  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x3abdc6e) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)
    #13 0x563417652b96  (/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521+0x14c8b96) (BuildId: ebb66c38ee1e4fe09240bde1dc91b9090cdf87d4)

SUMMARY: AddressSanitizer: 104 byte(s) leaked in 2 allocation(s).
error: test failed, to rerun pass `-p wasmtime-wasi-http --lib`

Caused by:
  process didn't exit successfully: `/home/runner/work/wasmtime/wasmtime/target/x86_64-unknown-linux-gnu/debug/deps/wasmtime_wasi_http-e2c9f65d23950521` (exit status: 1)
note: test exited abnormally; to see the full output pass --no-capture to the harness.
Error: Process completed with exit code 1.

@alexcrichton
Copy link
Member

As for the leak, it looks like that's caused by the test added here so I don't think it's spurious. The error message isn't great, however, but if you try running locally it might provide a better error message when llvm-symbolizer is in your $PATH

@tbrockman
Copy link
Contributor Author

Thanks for the detailed review and guidance @rvolosatovs! I'll incorporate the changes when I get a chance.

@rvolosatovs rvolosatovs added this pull request to the merge queue Oct 31, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 31, 2025
@rvolosatovs rvolosatovs enabled auto-merge October 31, 2025 16:17
@rvolosatovs rvolosatovs added this pull request to the merge queue Oct 31, 2025
Merged via the queue into bytecodealliance:main with commit 5d97bc3 Oct 31, 2025
45 checks passed
@alexcrichton
Copy link
Member

Question for y'all: the new method here looks to more-or-less be a copy/paste of this functionality. Would it be possible to unify the two, and if not, how come? The handling of Body::Host for example is quite different between the two and I naively am not entirely sure why.

@rvolosatovs
Copy link
Member

Question for y'all: the new method here looks to more-or-less be a copy/paste of this functionality. Would it be possible to unify the two, and if not, how come? The handling of Body::Host for example is quite different between the two and I naively am not entirely sure why.

I'm working on a PR, which will align the two

@rvolosatovs
Copy link
Member

See #11970

@alexcrichton
Copy link
Member

Makes sense, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants