ctubbsii commented on code in PR #5729: URL: https://github.com/apache/accumulo/pull/5729#discussion_r2199322090
########## server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java: ########## @@ -453,6 +454,13 @@ private GCStatus fetchGcStatus() { public void run() { ServerContext context = getContext(); int[] ports = getConfiguration().getPort(Property.MONITOR_PORT); + String rootContext = getConfiguration().get(Property.MONITOR_ROOT_CONTEXT); + // Needs leading slash in order to property create rest endpoint requests + Preconditions.checkArgument(rootContext.startsWith("/"), + "Root context: \"%s\" does not have a leading '/'", rootContext); + // Needed to support the existing zk monitor address format + Preconditions.checkArgument(rootContext.endsWith("/"), + "Root context: \"%s\" does not have a trailing '/'", rootContext); Review Comment: Forcing it to end with slash makes the path assumptions in the code a little simpler, probably, but it's probably annoying to use. I should be able to put "/path" instead of "/path/" in the config. The logic here should be able to handle that case pretty easily. It might also need to validate that the path doesn't contain non-URL characters like whitespace, or try to do anything weird with path traversal using `../` -- 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: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org