Copilot commented on code in PR #8819:
URL: https://github.com/apache/hadoop/pull/8819#discussion_r4256700119
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/KMSClientProvider.java:
##########
@@ -302,22 +312,51 @@ public KeyProvider createProvider(URI providerUri,
Configuration conf)
hostsPart = t[0];
}
KMSClientProvider[] providers =
- createProviders(conf, origUrl, port, hostsPart);
+ createProviders(conf, origUrl.getProtocol(), origUrl.getPath(),
port, hostsPart);
return new LoadBalancingKMSClientProvider(providerUri, providers,
conf);
}
return null;
}
- private KMSClientProvider[] createProviders(Configuration conf,
- URL origUrl, int port, String hostsPart) throws IOException {
+ /**
+ * Handles an authority that has a bracketed IPv6 literal, which the colon
split above cannot
+ * parse and, for more than one host, neither {@link Path} nor {@link URL}
can hold.
+ */
+ private KMSClientProvider[] createBracketedProviders(Configuration conf,
URI providerUri,
+ String kmsAuthority) throws IOException {
+ String[] parts = kmsAuthority.split("@", 2);
+ if (parts.length != 2) {
+ throw new IOException("No valid authority in kms uri [" + providerUri
+ "]");
+ }
+ int port = -1;
+ String hostsPart = parts[1];
+ // A colon after the last closing bracket delimits the port, others
belong to a literal
+ int portIdx = hostsPart.lastIndexOf(':');
+ if (portIdx > hostsPart.lastIndexOf(']')) {
+ try {
+ port = Integer.parseInt(hostsPart.substring(portIdx + 1));
+ } catch (NumberFormatException e) {
+ throw new IOException("Could not parse port in kms uri [" +
providerUri + "]");
+ }
+ hostsPart = hostsPart.substring(0, portIdx);
+ }
+ return createProviders(conf, parts[0], providerUri.getPath(), port,
hostsPart);
+ }
+
+ private KMSClientProvider[] createProviders(Configuration conf, String
protocol, String path,
+ int port, String hostsPart) throws IOException {
String[] hosts = hostsPart.split(";");
KMSClientProvider[] providers = new KMSClientProvider[hosts.length];
for (int i = 0; i < hosts.length; i++) {
+ // URI would add the brackets itself, which leaves the end of the
address ambiguous
+ if (hosts[i].contains(":") && !hosts[i].startsWith("[")) {
+ throw new IOException("Could not parse host [" + hosts[i]
+ + "] in kms uri. An IPv6 literal must be enclosed in brackets.");
+ }
try {
providers[i] =
new KMSClientProvider(
- new URI("kms", origUrl.getProtocol(), hosts[i], port,
- origUrl.getPath(), null, null), conf);
+ new URI("kms", protocol, hosts[i], port, path, null, null),
conf);
} catch (URISyntaxException e) {
Review Comment:
The host validation is currently too permissive for bracketed inputs: any
host token starting with `[` bypasses the colon check, which allows invalid
forms like `[ipv6]:port` (port-per-host) to flow into `new URI(..., host,
...)`, relying on `URISyntaxException` rather than producing a deterministic,
targeted `IOException`. Consider explicitly validating bracketed tokens (e.g.,
require they end with `]` and contain no trailing characters), rejecting `]:`
(per-host port) with a specific error, and normalizing bracketed IPv6 literals
before constructing the URI (e.g., pass the IPv6 address without brackets to
the `URI` host component) so both valid and invalid cases behave predictably.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
