DomGarguilo commented on code in PR #5966:
URL: https://github.com/apache/accumulo/pull/5966#discussion_r2561107810
##########
core/src/main/java/org/apache/accumulo/core/conf/PropertyType.java:
##########
@@ -423,43 +420,71 @@ public boolean test(final String input) {
}
- public static class PortRange extends Matches {
-
- public static final Range<Integer> VALID_RANGE = Range.of(1024, 65535);
-
- public PortRange(final String pattern) {
- super(pattern);
- }
+ private static class PortPredicate implements Predicate<String> {
@Override
public boolean test(final String input) {
- if (super.test(input)) {
+ if (input == null) {
+ return true;
+ }
+ final String trimmed = input.trim();
+ if (trimmed.isEmpty()) {
+ return false;
+ }
+ if ("0".equals(trimmed)) {
+ return true;
+ }
Review Comment:
There are several existing places in the code where we set "0" as the value
for the port:
https://github.com/apache/accumulo/blob/4c259d73f432f7b0796ed770fb35d71bb11645bc/minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloConfigImpl.java#L292-L296
This is used for all the
https://github.com/apache/accumulo/blob/4c259d73f432f7b0796ed770fb35d71bb11645bc/minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloConfigImpl.java#L186-L189
It looks like eventually the prop gets used in the server builder code to
create a `new InetSocketAddress(host, port)` whose javadoc says "A valid port
value is between 0 and 65535. A port number of zero will let the system pick up
an ephemeral port in a bind operation."
It would be a change in functionality to disallow "0".
There are several other places as well as tests and ITs that set 0 for the
port. Maybe we could document this better.
--
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]
