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

Reply via email to