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

Directly bind to interface name on supported platforms #359

Merged
merged 8 commits into from
Apr 12, 2024

Conversation

ducaale
Copy link
Owner

@ducaale ducaale commented Apr 1, 2024

The latest release of reqwest (v0.12) has added ClientBuilder::interface() which removes the need for translating interface names to IP addresses before binding to them.

However, this is only supported on limited platforms i.e linux, android and fuchsia so we still can't remove the network-interface crate/feature.

Depends on #357

Cargo.toml Show resolved Hide resolved
Copy link
Collaborator

@blyxxyz blyxxyz left a comment

Choose a reason for hiding this comment

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

xh --interface=lo example.org now hangs forever. (It used to give a connect error.) Any idea?

(EDIT: it does give a timeout eventually.)

Cargo.toml Show resolved Hide resolved
@ducaale
Copy link
Owner Author

ducaale commented Apr 12, 2024

xh --interface=lo example.org now hangs forever. (It used to give a connect error.) Any idea?

I'm not really sure. But if cURL is behaving the same, then it should be okay?

$ curl http://example.org/ --interface lo
curl: (28) Failed to connect to example.org port 80 after 131767 ms: Connection timed out

@blyxxyz
Copy link
Collaborator

blyxxyz commented Apr 12, 2024

Another difference is that my (disconnected) ethernet interfaces now also hang forever while the old implementation gave a "Couldn't bind" error (because NetworkInterface::show() returned it without an IP address).

The crux seems to be that reqwest uses SO_BINDTODEVICE to basically hand the interface name directly to the kernel instead of the IP address thing we were doing.

But yeah, it matches curl, good point. So it's probably for the best.

Copy link
Collaborator

@blyxxyz blyxxyz left a comment

Choose a reason for hiding this comment

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

Looks good! We'll have to watch out for other platforms getting supported.

@ducaale ducaale merged commit 40a0448 into master Apr 12, 2024
9 checks passed
@ducaale ducaale deleted the interface-name-bind branch April 12, 2024 21:19
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.

3 participants