Change doc level query name validation (#630)

* change validation and tests Signed-off-by: jowg-amazon <[email protected]> * change regex to 0-256 chars Signed-off-by: jowg-amazon <[email protected]> * moved validation to common utils and fix test Signed-off-by: jowg-amazon <[email protected]> * remove TODO Signed-off-by: jowg-amazon <[email protected]> * ensure name is at lest 1 char Signed-off-by: jowg-amazon <[email protected]> * change error message Signed-off-by: jowg-amazon <[email protected]> --------- Signed-off-by: jowg-amazon <[email protected]> (cherry picked from commit 9beae87) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Apr 19, 2024 · db2255c · db2255c
1 parent bbe9cf2
commit db2255c
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 11 deletions.
diff --git a/src/main/kotlin/org/opensearch/commons/alerting/model/DocLevelQuery.kt b/src/main/kotlin/org/opensearch/commons/alerting/model/DocLevelQuery.kt
@@ -21,9 +21,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)
         }
     }
 
@@ -74,6 +74,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)
@@ -93,7 +94,7 @@ data class DocLevelQuery(
                     QUERY_ID_FIELD -> id = xcp.text()
                     NAME_FIELD -> {
                         name = xcp.text()
-                        validateQuery(name)
+                        validateQueryName(name)
                     }
 
                     QUERY_FIELD -> query = xcp.text()
@@ -105,7 +106,7 @@ data class DocLevelQuery(
                         )
                         while (xcp.nextToken() != XContentParser.Token.END_ARRAY) {
                             val tag = xcp.text()
-                            validateQuery(tag)
+                            validateQueryTag(tag)
                             tags.add(tag)
                         }
                     }
@@ -139,16 +140,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

diff --git a/src/main/kotlin/org/opensearch/commons/utils/ValidationHelpers.kt b/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
+}
diff --git a/src/test/kotlin/org/opensearch/commons/alerting/model/DocLevelMonitorInputTests.kt b/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
             )
         }