Skip to content

Add support for the host being an IP address. #118

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Frostie314159
Copy link
Contributor

@Frostie314159 Frostie314159 commented Mar 27, 2025

Hi,
as discussed in #116 this PR adds support for the URL host to be an IP address. When the PR for nourl is merged, I'll point nourl back to crates.io and mark this as ready for review.
I also added a check to skip the DNS lookup all together, if the host is an IP address, which should provide better performance, than going through a mock DNS resolver first.
Greetings
Simon

@rmja
Copy link
Member

rmja commented Mar 28, 2025

nourl 0.1.3 is released now which have ipv6 host support.

@Frostie314159
Copy link
Contributor Author

@lulf Would you be fine, with me adding an extra function to specify an endpoint manually? I agree, that having proper IPv6 handing for URLs is useful for generic applications, but for applications that exclusively use manually specified endpoints, it seems a little wasteful to first format a URL, just for it to be immediately deserialized again.

@Frostie314159 Frostie314159 marked this pull request as ready for review March 28, 2025 08:18
@Frostie314159
Copy link
Contributor Author

I just noticed, that I messed up with nourl and defmt support.

@Frostie314159
Copy link
Contributor Author

It's fixed now.

@Frostie314159
Copy link
Contributor Author

I've temporarily pointed embedded-tls to git, since that resolves an issue, which caused errors with TLS. Once the relevant PR gets merged and a new version released, I'll point this back to crates.io.

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