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

client: fix IPv6 parsing for client.servers block #20324

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

tgross
Copy link
Member

@tgross tgross commented Apr 8, 2024

When the client.servers block is parsed, we split the port from the address. This does not correctly handle IPv6 addresses when they are in URL format (wrapped in brackets), which we require to disambiguate the port and address.

Fix the parser to correctly split out the port and handle a missing port value for IPv6. Update the documentation to make the URL format requirement clear.

Fixes: #20310

When the `client.servers` block is parsed, we split the port from the
address. This does not correctly handle IPv6 addresses when they are in URL
format (wrapped in brackets), which we require to disambiguate the port and
address.

Fix the parser to correctly split out the port and handle a missing port value
for IPv6. Update the documentation to make the URL format requirement clear.

Fixes: #20310
@tgross tgross added backport/1.5.x backport to 1.5.x release line backport/1.6.x backport to 1.6.x release line backport/1.7.x backport to 1.7.x release line theme/docs Documentation issues and enhancements type/bug labels Apr 8, 2024
@tgross tgross force-pushed the 20310-ipv6-address-parsing branch from f6e32b5 to 42ed095 Compare April 8, 2024 18:15
@tgross tgross added this to the 1.7.x milestone Apr 8, 2024
@tgross tgross marked this pull request as ready for review April 8, 2024 18:39
@tgross tgross requested review from shoenig and gulducat April 8, 2024 18:39
Copy link
Member

@gulducat gulducat left a comment

Choose a reason for hiding this comment

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

LGTM!

the OP also pointed out that the log is somewhat hidden being log_level DEBUG -- perhaps we could also change that to Info or even Warn?

@tgross
Copy link
Member Author

tgross commented Apr 8, 2024

the OP also pointed out that the log is somewhat hidden being log_level DEBUG -- perhaps we could also change that to Info or even Warn?

I'm pretty sure the reason we don't debug at Info or Warn is that if it's a DNS address and you're having DNS resolution issues, it's going to create a lot of spam in the logs that's better handled in the caller (which gets the errors).

@tgross tgross merged commit 8eaf176 into main Apr 8, 2024
23 checks passed
@tgross tgross deleted the 20310-ipv6-address-parsing branch April 8, 2024 19:06
philrenaud pushed a commit that referenced this pull request Apr 18, 2024
When the `client.servers` block is parsed, we split the port from the
address. This does not correctly handle IPv6 addresses when they are in URL
format (wrapped in brackets), which we require to disambiguate the port and
address.

Fix the parser to correctly split out the port and handle a missing port value
for IPv6. Update the documentation to make the URL format requirement clear.

Fixes: #20310
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 13, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/1.5.x backport to 1.5.x release line backport/1.6.x backport to 1.6.x release line backport/1.7.x backport to 1.7.x release line theme/docs Documentation issues and enhancements theme/ipv6 type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

client: parsing of server IPv6 addresses is inconsistent with docs
2 participants