additional checks for path sanitizer (#905)

Improves handling of normal request parameters using
`?` rather than matrix arguments. In some cases these
may not get stripped off before calling the sanitizer.
Also explicitly checks the length of the sanitized
result.

spectator-ext-ipc/src/main/java/com/netflix/spectator/ipc/http/PathSanitizer.java

@@ -24,6 +24,8 @@
  */
 public final class PathSanitizer {
 
+  private static final int MAX_LENGTH = 120;
+
   private static final AsciiSet ALPHA_CHARS = AsciiSet.fromPattern("a-zA-Z");
 
   private static final AsciiSet DIGITS = AsciiSet.fromPattern("0-9");
@@ -35,11 +37,15 @@ private PathSanitizer() {
 
   /** Returns a sanitized path string for use as an endpoint tag value. */
   public static String sanitize(String path) {
-    return sanitizeSegments(removeMatixParameters(path));
+    return sanitizeSegments(removeParameters(path));
+  }
+
+  private static String removeParameters(String path) {
+    return removeParameters(removeParameters(path, '?'), ';');
   }
 
-  private static String removeMatixParameters(String path) {
-    int i = path.indexOf(';');
+  private static String removeParameters(String path, char c) {
+    int i = path.indexOf(c);
     return i > 0 ? path.substring(0, i) : path;
   }
 
@@ -62,9 +68,9 @@ private static String sanitizeSegments(String path) {
 
       if (!segment.isEmpty()) {
         if (shouldSuppressSegment(segment))
-          builder.append("_-");
+          appendIfSpaceAvailable(builder, "-");
         else
-          builder.append('_').append(segment);
+          appendIfSpaceAvailable(builder, segment);
         ++segmentsAdded;
       }
     }
@@ -102,4 +108,13 @@ private static boolean shouldSuppressSegment(String segment) {
 
     return !version && n == 2;
   }
+
+  private static void appendIfSpaceAvailable(StringBuilder builder, String segment) {
+    int spaceRemaining = MAX_LENGTH - builder.length() - 1;
+    if (segment.length() < spaceRemaining) {
+      builder.append('_').append(segment);
+    } else if (spaceRemaining >= 2) {
+      builder.append("_-");
+    }
+  }
 }

spectator-ext-ipc/src/test/java/com/netflix/spectator/ipc/http/PathSanitizerTest.java

@@ -47,6 +47,12 @@ public void matrixParameters() {
     Assertions.assertEquals("_api_v1_foo", sanitize(path));
   }
 
+  @Test
+  public void requestParameters() {
+    String path = "/api/v1/foo?user=bob&version=1";
+    Assertions.assertEquals("_api_v1_foo", sanitize(path));
+  }
+
   @Test
   public void decimalNumbers() {
     String path = "/api/v1/foo/1234567890/123";
@@ -240,4 +246,18 @@ public void allowMostWords() throws Exception {
           "suppressed " + suppressed + " of " + total);
     }
   }
+
+  @Test
+  public void restrictsLength() {
+    // Alternate consonants and vowels to avoid consonant run check
+    String pattern = "abecidofug";
+    StringBuilder path = new StringBuilder().append('/');
+    for (int i = 0; i < 100; ++i) {
+      path.append(pattern);
+    }
+    Assertions.assertEquals("_-", PathSanitizer.sanitize(path.toString()));
+
+    path.append("/foo/bar/baz");
+    Assertions.assertEquals("_-_foo_bar_baz", PathSanitizer.sanitize(path.toString()));
+  }
 }