Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.11] Change doc level query name validation #642

Open
wants to merge 1 commit into
base: 2.11
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions src/main/kotlin/org/opensearch/commons/utils/ValidationHelpers.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
}
Expand Down Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
}
Expand All @@ -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
)
}
Expand Down
Loading