TCP overrides by badeend · Pull Request #2 · rylev/wasmtime

badeend · 2024-01-11T19:24:12Z

For TCP sockets and IP name lookup, I've moved all the WASI interop code into the crates/wasi/src/preview2/host/ folder. The files directly in crates/wasi/src/preview2/ should now be as "idiomatic" Rust as I could get it. Part of this change is that I updated Network::resolve_addresses to return a plain Box<dyn Future> and effectively demoted ResolveAddressStream to be an implementation detail of the WASI interop.

In the end I did get the regular AsyncRead and AsyncWrite to work, thanks to the existing AsyncReadStream and AsyncWriteStream implementations in pipe.rs and write_stream.rs

I've combined the two distinct permission mechanisms (AllowedNetworkUses & SocketAddrCheck) into one.

In UdpSocket, I've removed the Option<> wrapper around the SocketAddrCheck.

Because the System*** types are expected to be wrapped inside other implementations, I've added non-boxing variants of methods that would otherwise require a box because of the dyn Trait restrictions. This eliminates unnecessary intermediate boxes in places where the inner type is statically known. For example, the inner field of DefaultTcpSocket can be of type SystemTcpSocket instead of Box<dyn TcpSocket>. Places where this is relevant:

SystemNetwork::resolve_addresses
SystemNetwork::new_tcp_socket
SystemTcpSocket::connect
SystemTcpSocket::poll_accept

…ency on cap_net_ext

…Socket. Also updated some overly strict test cases.

…okio::net::TcpListener::poll_accept

…layer

…-allocating variant wherever possible.

- BoxSyncFuture: for public facing signatures. This is just a regular `Pin<Box<dyn Future ...>>` - Preview2Future: utility type for use within the WASI wrapper code

- Remove AllowedNetworkUses in favor of just socket_addr_checks - Let all permission checks go through self.ctx(), instead of NetworkHandle

rylev

This looks great! I only have a few nits that I don't think should block us from merging this and starting on UDP.

rylev · 2024-01-15T13:38:39Z

-    fn resolve_addresses(&mut self, name: String) -> ResolveAddressStream;
+    ///
+    /// Unicode domain names are automatically converted to ASCII using IDNA encoding.
+    /// If the input is an IP address string, the address is parsed and returned


Nit: these docs are written as if resolve_addresses was a concrete function. We should phrase them so that it's clear that these are expectations the implementor must uphold.

For a large part I (mindlessly) copied these docs from the WIT files :)
That being said, don't think it's that weird to write the documentation for the consumer instead of the implementor. There is precedent

rylev · 2024-01-15T13:46:05Z

+    /// Non-boxing variant of [Network::resolve_addresses]
+    pub fn resolve_addresses(
+        &mut self,
+        name: String,
+    ) -> impl Future<Output = io::Result<Vec<IpAddr>>> + Send + Sync + 'static {
+        async move { resolve_addresses(&name).await }
+    }
+
+    /// Non-boxing variant of [Network::new_tcp_socket]
+    pub fn new_tcp_socket(&mut self, family: SocketAddrFamily) -> io::Result<SystemTcpSocket> {
+        SystemTcpSocket::new(family)
+    }


It's unclear to me why we need this functions instead of inlining their bodies everywhere they're called.

Because the functions they call are internal to the crate. We could open those up, but I think it makes sense (at least conceptually) that consumers can't fabricate a SystemTcpSocket without going through SystemNetwork.

Co-authored-by: Ryan Levick <me@ryanlevick.com>

…to tcp-override

rylev · 2024-01-16T08:54:35Z

Amazing stuff! I'll get going on UDP!

…dealliance#7029) * Rename `Host*` things to avoid name conflicts with bindings. * Update to the latest resource-enabled wit files. * Adapting the code to the new bindings. * Update wasi-http to the resource-enabled wit deps. * Start adapting the wasi-http code to the new bindings. * Make `get_directories` always return new owned handles. * Simplify the `poll_one` implementation. * Update the wasi-preview1-component-adapter. FIXME: temporarily disable wasi-http tests. Add logging to the cli world, since stderr is now a reseource that can only be claimed once. * Work around a bug hit by poll-list, fix a bug in poll-one. * Comment out `test_fd_readwrite_invalid_fd`, which panics now. * Fix a few FIXMEs. * Use `.as_ref().trapping_unwrap()` instead of `TrappingUnwrapRef`. * Use `drop_in_place`. * Remove `State::with_mut`. * Remove the `RefCell` around the `State`. * Update to wit-bindgen 0.12. * Update wasi-http to use resources for poll and I/O. This required making incoming-body and outgoing-body resourrces too, to work with `push_input_stream_child` and `push_output_stream_child`. * Re-enable disabled tests, remove logging from the worlds. * Remove the `poll_list` workarounds that are no longer needed. * Remove logging from the adapter. That said, there is no replacement yet, so add a FIXME comment. * Reenable a test that now passes. * Remove `.descriptors_mut` and use `with_descriptors_mut` instead. Replace `.descriptors()` and `.descriptors_mut()` with functions that take closures, which limits their scope, to prevent them from invalid aliasing. * Implement dynamic borrow checking for descriptors. * Add a cargo-vet audit for wasmtime-wmemcheck. * Update cargo vet for wit-bindgen 0.12. * Cut down on duplicate sync/async resource types (#1) * Allow calling `get-directories` more than once (#2) For now `Clone` the directories into new descriptor slots as needed. * Start to lift restriction of stdio only once (#3) * Start to lift restriction of stdio only once This commit adds new `{Stdin,Stdout}Stream` traits which take over the job of the stdio streams in `WasiCtxBuilder` and `WasiCtx`. These traits bake in the ability to create a stream at any time to satisfy the API of `wasi:cli`. The TTY functionality is folded into them as while I was at it. The implementation for stdin is relatively trivial since the stdin implementation already handles multiple streams reading it. Built-in impls of the `StdinStream` trait are also provided for helper types in `preview2::pipe` which resulted in the implementation of `MemoryInputPipe` being updated to support `Clone` where all clones read the same original data. * Get tests building * Un-ignore now-passing test * Remove unneeded argument from `WasiCtxBuilder::build` * Fix tests * Remove some workarounds Stdio functions can now be called multiple times. * If `poll_oneoff` fails part-way through, clean up properly. Fix the `Drop` implementation for pollables to only drop the pollables that have been successfully added to the list. This fixes the poll_oneoff_files failure and removes a FIXME. --------- Co-authored-by: Alex Crichton <alex@alexcrichton.com>

badeend added 17 commits January 10, 2024 10:31

Reorder code blocks

d41494b

Move tcp_create_socket impl

a59482c

Move preview2/tcp.rs into preview2/host/tcp.rs

69c1132

Rename types to include *Wrapper suffix

66e0ade

Migrate to a AsyncRead+AsyncWrite based implementation

9ac3c4f

Define our own SocketAddrFamily so that we don't have a public depend…

4e92f84

…ency on cap_net_ext

Use std::io::Result for the validation utils

9e64490

Introduce SystemTcpSocket type with bare-bones implementation

a44ed1e

Move the low hanging fruit out of the WASI wrapper and into SystemTcp…

5241f02

…Socket. Also updated some overly strict test cases.

Move implementation of listen & listen_backlog_size into SystemTcpSocket

452c746

Explicitly specify which states are awaitable

d7c8e53

Extract out the async polling mechanism behind accept. Similar to t…

1fb5f0f

…okio::net::TcpListener::poll_accept

Separate IP name lookup implementation out from the WASI translation …

3056818

…layer

Introduce DynFuture

e4227e3

Refactor IP name lookup to use DynFuture, taking advantage of the non…

775021e

…-allocating variant wherever possible.

Move connect into SystemTcpSocket

d6087b9

Move bind into SystemTcpSocket

663c62e

badeend force-pushed the tcp-override branch from b8ae1cf to 663c62e Compare January 11, 2024 22:32

badeend added 6 commits January 12, 2024 00:02

Inline TCP-specific utilities

6053b7f

Return the streams from connect&accept

ae90b6e

Refactor DynFuture away into two types:

6343c9e

- BoxSyncFuture: for public facing signatures. This is just a regular `Pin<Box<dyn Future ...>>` - Preview2Future: utility type for use within the WASI wrapper code

Introduce TcpSocket trait

a2ee9a9

Refactor:

31ea773

- Remove AllowedNetworkUses in favor of just socket_addr_checks - Let all permission checks go through self.ctx(), instead of NetworkHandle

Implement DefaultTcpSocket

196a69a

badeend marked this pull request as ready for review January 14, 2024 13:16

badeend added 4 commits January 14, 2024 21:39

Fix error in MacOS-only code

3877aee

Resolve ambiguity between SocketAddrFamily and SocketAddressFamily

d3039a5

Update comments

9a5ebdb

Document why these public methods exist

0aaaf66

rylev approved these changes Jan 15, 2024

View reviewed changes

badeend and others added 6 commits January 15, 2024 19:40

Explicitly call it deny

484e777

Update crates/wasi/src/preview2/host/network.rs

bebc91f

Co-authored-by: Ryan Levick <me@ryanlevick.com>

Update crates/wasi/src/preview2/host/network.rs

78a48bd

Co-authored-by: Ryan Levick <me@ryanlevick.com>

Merge branch 'tcp-override' of https://github.com/badeend/wasmtime in…

36e8190

…to tcp-override

Give all WASI wrappers the same suffix

19f6ec4

Eliminate fall-throughs from resolve_next_address

7b3fbbf

badeend requested a review from rylev January 15, 2024 19:50

rylev merged commit afcaae1 into rylev:lookup-override Jan 16, 2024

rylev mentioned this pull request Jan 22, 2024

UDP override #4

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TCP overrides#2

TCP overrides#2
rylev merged 33 commits into
rylev:lookup-overridefrom
badeend:tcp-override

badeend commented Jan 11, 2024 •

edited

Loading

Uh oh!

rylev left a comment

Uh oh!

Uh oh!

Uh oh!

rylev Jan 15, 2024

Uh oh!

badeend Jan 15, 2024

Uh oh!

rylev Jan 15, 2024

Uh oh!

badeend Jan 15, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rylev commented Jan 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

badeend commented Jan 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rylev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rylev Jan 15, 2024

Choose a reason for hiding this comment

Uh oh!

badeend Jan 15, 2024

Choose a reason for hiding this comment

Uh oh!

rylev Jan 15, 2024

Choose a reason for hiding this comment

Uh oh!

badeend Jan 15, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rylev commented Jan 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

badeend commented Jan 11, 2024 •

edited

Loading