Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib/virtual-net: tokio::net::lookup_host requires host:port format #3900

Merged
merged 4 commits into from
May 24, 2023
Merged

lib/virtual-net: tokio::net::lookup_host requires host:port format #3900

merged 4 commits into from
May 24, 2023

Conversation

waynr
Copy link
Contributor

@waynr waynr commented May 23, 2023

This PR updates the call to tokio::net::lookup_host used by syscalls::wasix::wasi_resolve.

Prior to this PR when tokio::net::lookup_host is given a hostname without a port, name resolution fails with an Errno::Inval. When the port is added as shown in the tokio::net::lookup_host docs, it resolves successfully. This is kind of weird as it seems to me like the only thing necessary to look up a hostname should be the hostname itself -- the port shouldn't factor into it.

@waynr
Copy link
Contributor Author

waynr commented May 23, 2023

I verified this with a very simple reproduction of the problem i was seeing:

use tokio::net;

#[tokio::main]
async fn main() -> std::io::Result<()> {
    for addr in net::lookup_host("wasmer.io").await? {
        println!("socket address is {}", addr);
    }
    println!("Hello, world!");
    Ok(())
}

When I run this the output looks like

./target/debug/tokio-net-lookup-host
Error: Error { kind: InvalidInput, message: "invalid socket address" }

However, when I simply append a :0 to the host string it works:

use tokio::net;

#[tokio::main]
async fn main() -> std::io::Result<()> {
    for addr in net::lookup_host("wasmer.io:0").await? {
        println!("socket address is {}", addr);
    }
    println!("Hello, world!");
    Ok(())
}
./target/debug/tokio-net-lookup-host
socket address is 172.64.80.1:0
socket address is [2606:4700:3031::6815:16cc]:0
socket address is [2606:4700:3033::ac43:ceeb]:0
Hello, world!

@theduke
Copy link
Contributor

theduke commented May 24, 2023

@waynr

I think my intution was correct, our current Rust code just passes along the full string, while the regular implementation instead splits the string into port and hostname.

regular impl: https://github.com/wasix-org/rust/blob/43c52b0df5cad82411c989bf7228179585e4f494/library/std/src/sys_common/net.rs#L190

wasix impl: https://github.com/wasix-org/rust/blob/43c52b0df5cad82411c989bf7228179585e4f494/library/std/src/sys/wasix/net.rs#L1335

Note that the syscall impl in wasmer then replaces a 0 port with None, so it works out in the end.

Just need to fix the impl in Rust .

lib/virtual-net/src/host.rs Outdated Show resolved Hide resolved
lib/virtual-net/src/host.rs Outdated Show resolved Hide resolved
@waynr waynr enabled auto-merge May 24, 2023 18:15
@waynr waynr merged commit 42c2e2c into wasmerio:master May 24, 2023
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.

2 participants