Skip to content

Conversation

agrawroh
Copy link
Member

Description

This PR ntroduces a new ip_address_parsing_lib which provides helper methods to parse IPv4 and IPv6 addresses that use getaddrinfo() with AI_NUMERICHOST|AI_NUMERICSERV for numeric-only parsing. We have refactored parseInternetAddressNoThrow() and parseInternetAddressAndPortNoThrow() to use these newly added helpers.

Note: For IPv4, we enforce strict dotted-quad by round-tripping via inet_ntop to avoid platform quirks and keep the behavior consistent.

Fix #23952


Commit Message: network: centralize IP parsing and unify on getaddrinfo() to avoid circular deps
Additional Description: Refactor IP parsing logic and consolidate the scattered logic with a single low-level implementation.
Risk Level: Low
Testing: Added Unit Tests
Docs Changes: N/A
Release Notes: N/A

Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #40737 was opened by agrawroh.

see: more, trace.

@agrawroh agrawroh requested a review from yanavlasov August 15, 2025 16:54
@agrawroh agrawroh marked this pull request as ready for review August 15, 2025 16:55
Copy link
Contributor

@yanavlasov yanavlasov left a comment

Choose a reason for hiding this comment

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

/wait-any

sockaddr_in sa4 = *reinterpret_cast<sockaddr_in*>(res->ai_addr);
os_sys_calls.freeaddrinfo(res);
sa4.sin_port = htons(port);
// Enforce strict dotted-quad by round-tripping via inet_ntop and requiring equality.
Copy link
Contributor

Choose a reason for hiding this comment

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

What errors will this catch? Can we just keep using inet_pton as before?
Converting back into string and comparing can be expensive for configs with a lot of addresses.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was trying to unify on getaddrinfo() but indeed I can just use inet_pton as before and save the unnecessary overhead.

@yanavlasov yanavlasov merged commit 35e4745 into envoyproxy:main Aug 27, 2025
25 checks passed
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.

Cleanup IP parsing logic
2 participants