Accept RFC 1123 hostnames in EndpointUtil.validateEndpoint - #8746
Accept RFC 1123 hostnames in EndpointUtil.validateEndpoint#8746kalayciburak wants to merge 1 commit into
Conversation
URI.getHost() returns null for some valid DNS names (JDK-8188305), such as a label that starts with a digit. Fall back to URL.getHost() so those endpoints are accepted, matching OtlpConfigUtil. Fixes open-telemetry#8745 Signed-off-by: Burak KALAYCI <kalayciburak1996@gmail.com>
|
|
Pull request dashboard statusWaiting on the author · refreshed 2026-09-02 14:35 UTC Respond to 1 review item (e.g. link a commit, explain why not, ask a follow-up):
Status above doesn't look right?
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69f1520108
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "Invalid endpoint, must start with http:// or https://: " + uri); | ||
| } | ||
| if (uri.getHost() == null) { | ||
| if (!hasHost(uri, endpoint)) { |
There was a problem hiding this comment.
When this fallback is taken, including for the three new test cases, the method still returns the original URI, whose getHost() is null and whose getPort() is -1. Supported consumers cannot use that representation: JdkHttpSender.java:198 passes it to HttpRequest.Builder.uri, which throws IllegalArgumentException: unsupported URI, while UpstreamGrpcSenderProvider.java:71 passes the null host and invalid port to ManagedChannelBuilder.forAddress. Consequently, these newly accepted endpoints still fail whenever the JDK HTTP or managed-channel gRPC sender is selected; the parsed host and port must be preserved or those consumers must use a parser that supports these names.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (57.14%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #8746 +/- ##
============================================
- Coverage 91.29% 91.28% -0.01%
- Complexity 10498 10501 +3
============================================
Files 1006 1006
Lines 28338 28344 +6
Branches 3581 3582 +1
============================================
+ Hits 25870 25873 +3
- Misses 1675 1677 +2
- Partials 793 794 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Hi @kalayciburak — just a friendly reminder that this pull request is waiting on you. The dashboard status comment has the open items and is kept current.
|
1 participant
Description
EndpointUtil.validateEndpointrejects hostnames thatjava.net.URI#getHost()cannot parse even when they are valid DNS names. That is JDK-8188305: RFC 2396 host parsing returnsnullfor labels that start with a digit, which is common for Kubernetes namespaces such asotlp.1234-k8s-namespace.The existing
uri.getHost() == nullcheck was added in #8489 to reject truly host-less URIs (http:localhost:4317,https:/foo). That check is still needed, but it is too strict whengetHost()isnulland a host is present.This falls back to
URL.getHost(), the same parser used byOtlpConfigUtil.validateEndpoint. Host-less URIs still fail; RFC 1123 names thatURI.getHost()misses are accepted.Fixes #8745
Testing done
./gradlew :exporters:common:test --tests io.opentelemetry.exporter.internal.EndpointUtilTest— 12 tests, 0 failedURI.getHost() == nullbefore the change./gradlew :exporters:common:check— passed (tests, checkstyle, spotless, japicmp, animalsniffer)