Change doc level query name validation (#630) (#638)

* change validation and tests



* change regex to 0-256 chars



* moved validation to common utils and fix test



* remove TODO



* ensure name is at lest 1 char



* change error message



---------


(cherry picked from commit 9beae87)

Signed-off-by: jowg-amazon <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

src/main/kotlin/org/opensearch/commons/alerting/model/DocLevelQuery.kt

@@ -22,9 +22,9 @@ data class DocLevelQuery(
 
     init {
         // Ensure the name and tags have valid characters
-        validateQuery(name)
+        validateQueryName(name)
         for (tag in tags) {
-            validateQuery(tag)
+            validateQueryTag(tag)
         }
     }
 
@@ -80,6 +80,7 @@ data class DocLevelQuery(
         const val QUERY_FIELD_NAMES_FIELD = "query_field_names"
         const val NO_ID = ""
         val INVALID_CHARACTERS: List<String> = listOf(" ", "[", "]", "{", "}", "(", ")")
+        val QUERY_NAME_REGEX = "^.{1,256}$".toRegex() // regex to restrict string length between 1 - 256 chars
 
         @JvmStatic
         @Throws(IOException::class)
@@ -100,7 +101,7 @@ data class DocLevelQuery(
                     QUERY_ID_FIELD -> id = xcp.text()
                     NAME_FIELD -> {
                         name = xcp.text()
-                        validateQuery(name)
+                        validateQueryName(name)
                     }
 
                     QUERY_FIELD -> query = xcp.text()
@@ -112,7 +113,7 @@ data class DocLevelQuery(
                         )
                         while (xcp.nextToken() != XContentParser.Token.END_ARRAY) {
                             val tag = xcp.text()
-                            validateQuery(tag)
+                            validateQueryTag(tag)
                             tags.add(tag)
                         }
                     }
@@ -159,16 +160,20 @@ data class DocLevelQuery(
             return DocLevelQuery(sin)
         }
 
-        // TODO: add test for this
-        private fun validateQuery(stringVal: String) {
+        private fun validateQueryTag(stringVal: String) {
             for (inValidChar in INVALID_CHARACTERS) {
                 if (stringVal.contains(inValidChar)) {
                     throw IllegalArgumentException(
-                        "They query name or tag, $stringVal, contains an invalid character: [' ','[',']','{','}','(',')']"
+                        "The query tag, $stringVal, contains an invalid character: [' ','[',']','{','}','(',')']"
                     )
                 }
             }
         }
+        private fun validateQueryName(stringVal: String) {
+            if (!stringVal.matches(QUERY_NAME_REGEX)) {
+                throw IllegalArgumentException("The query name, $stringVal, should be between 1 - 256 characters.")
+            }
+        }
     }
 
     // constructor for java plugins' convenience to optionally avoid passing empty list for 'fieldsBeingQueried' field

src/main/kotlin/org/opensearch/commons/utils/ValidationHelpers.kt

@@ -11,6 +11,9 @@ import java.util.regex.Pattern
 // Valid ID characters = (All Base64 chars + "_-") to support UUID format and Base64 encoded IDs
 private val VALID_ID_CHARS: Set<Char> = (('a'..'z') + ('A'..'Z') + ('0'..'9') + '+' + '/' + '_' + '-').toSet()
 
+// Invalid characters in a new name field: [* ? < > | #]
+private val INVALID_NAME_CHARS = "^\\*\\?<>|#"
+
 fun validateUrl(urlString: String) {
     require(isValidUrl(urlString)) { "Invalid URL or unsupported" }
 }
@@ -53,3 +56,15 @@ fun validateIamRoleArn(roleArn: String) {
     val roleArnRegex = Pattern.compile("^arn:aws(-[^:]+)?:iam::([0-9]{12}):([a-zA-Z_0-9+=,.@\\-_/]+)$")
     require(roleArnRegex.matcher(roleArn).find()) { "Invalid AWS role ARN: $roleArn " }
 }
+
+fun isValidName(name: String): Boolean {
+    // Regex to restrict string so that it cannot start with [_, -, +],
+    // contain two consecutive periods or contain invalid chars
+    val regex = Regex("""^(?![_\-\+])(?!.*\.\.)[^$INVALID_NAME_CHARS]+$""")
+
+    return name.matches(regex)
+}
+
+fun getInvalidNameChars(): String {
+    return INVALID_NAME_CHARS
+}

src/test/kotlin/org/opensearch/commons/alerting/model/DocLevelMonitorInputTests.kt

@@ -43,14 +43,33 @@ class DocLevelMonitorInputTests {
     }
 
     @Test
-    fun `test create Doc Level Query with invalid characters for name`() {
-        val badString = "query with space"
+    fun `test create Doc Level Query with invalid name length`() {
+        val stringBuilder = StringBuilder()
+
+        // test empty string
+        val emptyString = stringBuilder.toString()
+        try {
+            randomDocLevelQuery(name = emptyString)
+            Assertions.fail("Expecting an illegal argument exception")
+        } catch (e: IllegalArgumentException) {
+            Assertions.assertEquals(
+                "The query name, $emptyString, should be between 1 - 256 characters.",
+                e.message
+            )
+        }
+
+        // test string with 257 chars
+        repeat(257) {
+            stringBuilder.append("a")
+        }
+        val badString = stringBuilder.toString()
+
         try {
             randomDocLevelQuery(name = badString)
             Assertions.fail("Expecting an illegal argument exception")
         } catch (e: IllegalArgumentException) {
             Assertions.assertEquals(
-                "They query name or tag, $badString, contains an invalid character: [' ','[',']','{','}','(',')']",
+                "The query name, $badString, should be between 1 - 256 characters.",
                 e.message
             )
         }
@@ -65,7 +84,7 @@ class DocLevelMonitorInputTests {
             Assertions.fail("Expecting an illegal argument exception")
         } catch (e: IllegalArgumentException) {
             Assertions.assertEquals(
-                "They query name or tag, $badString, contains an invalid character: [' ','[',']','{','}','(',')']",
+                "The query tag, $badString, contains an invalid character: [' ','[',']','{','}','(',')']",
                 e.message
             )
         }