[
https://issues.apache.org/jira/browse/METRON-976?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16039328#comment-16039328
]
ASF GitHub Bot commented on METRON-976:
---------------------------------------
Github user mattf-horton commented on a diff in the pull request:
https://github.com/apache/metron/pull/600#discussion_r120429871
--- Diff:
metron-platform/metron-common/src/main/java/org/apache/metron/common/utils/KafkaUtils.java
---
@@ -68,12 +68,14 @@
return ret;
}
- public List<String> fromEndpoint(String url) throws URISyntaxException {
+ public List<String> fromEndpoint(String url){
List<String> ret = new ArrayList<>();
if(url != null) {
- URI uri = new URI(url);
- int port = uri.getPort();
- ret.add(uri.getHost() + ((port > 0)?(":" + port):""));
+ Iterable<String> splits = Splitter.on("//").split(url);
+ if(Iterables.size(splits) == 2) {
+ String hostPort = Iterables.getLast(splits);
+ ret.add(hostPort);
--- End diff --
Thanks, @justinleet , and sorry if I'm being a PITA. By inspection, I have
the following concerns:
1. The URI solution always delivers exactly "host[:port]" (or fails). The
split solution delivers the entire url except the protocol, ie
"host[:port]/path/extension/if/any...", unless there's something upstream
cutting off the path extension (if any). Is there? I didn't see anything, it
looks like endpoint urls are simply read raw from a config JSON structure. So
I would have expected trimming the proposed split at both ends. Unless
endpoint URLs are supposed to allow path extensions, in which case the URI
approach is clearly wrong.
2. The split code only returns a result if the split size is == 2. This
prevents it from accepting:
* Endpoint urls without the protocol prefix
* Endpoint urls with path extensions that accidentally have double
slashes in them (a common problem with machine-generated URLs, and accepted by
most URL path processors)
It also does not provide any log feedback on why it silently ignores
"bad" endpoint urls.
None of these issues would show up in testing unless the unit tests include
thorough negative testcases for invalid configured endpoint URLs.
> KafkaUtils doesn't handle SASL_PLAINTEXT
> ----------------------------------------
>
> Key: METRON-976
> URL: https://issues.apache.org/jira/browse/METRON-976
> Project: Metron
> Issue Type: Bug
> Reporter: Justin Leet
> Assignee: Justin Leet
>
> Java's URI class throws an error because '_' isn't allowed to be in a URI.
> We'll need to move to a manually split approach.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
