kpatelatwork commented on a change in pull request #10530:
URL: https://github.com/apache/kafka/pull/10530#discussion_r612800672
##########
File path:
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/RestServer.java
##########
@@ -369,9 +370,31 @@ else if (serverConnector != null &&
serverConnector.getHost() != null && serverC
else if (serverConnector != null && serverConnector.getPort() > 0)
builder.port(serverConnector.getPort());
- log.info("Advertised URI: {}", builder.build());
+ URI uri = builder.build();
+ validateUriHost(uri);
+ log.info("Advertised URI: {}", uri);
- return builder.build();
+ return uri;
+ }
+
+ /**
+ * Parses the uri and throws a more definitive error
+ * when the internal node to node communication can't happen due to an
invalid host name.
+ */
+ private void validateUriHost(URI uri) {
+ if (uri.getHost() == null) {
+ String host = Utils.getHost(uri.getAuthority());
+ String errorMsg = "Invalid host=" + host + ", in url=" +
uri.toString();
Review comment:
The new check in the code happens only when uri.getHost is null so its
not worse than what we have in trunk but I do agree Apache HttpClient may fix
this. I had thought about using Apache HttpClient but isn't this a big library
change so do we want to do that in a separate ticket as I wasn't sure about out
library rules as I didn't found Apache HttpClient being used anywhere in our
code except in 1 test case.
##########
File path:
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/RestServer.java
##########
@@ -369,9 +370,31 @@ else if (serverConnector != null &&
serverConnector.getHost() != null && serverC
else if (serverConnector != null && serverConnector.getPort() > 0)
builder.port(serverConnector.getPort());
- log.info("Advertised URI: {}", builder.build());
+ URI uri = builder.build();
+ validateUriHost(uri);
+ log.info("Advertised URI: {}", uri);
- return builder.build();
+ return uri;
+ }
+
+ /**
+ * Parses the uri and throws a more definitive error
+ * when the internal node to node communication can't happen due to an
invalid host name.
+ */
+ private void validateUriHost(URI uri) {
+ if (uri.getHost() == null) {
+ String host = Utils.getHost(uri.getAuthority());
+ String errorMsg = "Invalid host=" + host + ", in url=" +
uri.toString();
Review comment:
@C0urante The new check in the code happens only when uri.getHost is
null so its not worse than what we have in trunk but I do agree Apache
HttpClient may fix this. I had thought about using Apache HttpClient but isn't
this a big library change so do we want to do that in a separate ticket as I
wasn't sure about out library rules as I didn't found Apache HttpClient being
used anywhere in our code except in 1 test case.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
