From 325c80052a12119cf8e2461812895054cfc7a56b Mon Sep 17 00:00:00 2001 From: Jing Zhang Date: Tue, 20 May 2025 13:06:47 -0700 Subject: [PATCH] fix bug 3860 for no class issue Signed-off-by: Jing Zhang --- .../connector/MLCreateConnectorRequest.java | 2 +- .../connector/MLUpdateConnectorRequest.java | 2 +- .../transport/model/MLUpdateModelRequest.java | 2 +- .../MLRegisterModelGroupRequest.java | 2 +- .../MLUpdateModelGroupRequest.java | 2 +- .../register/MLRegisterModelRequest.java | 2 +- .../ml/common/utils/StringUtils.java | 55 ---------------- .../opensearch/ml/common/utils/Validator.java | 66 +++++++++++++++++++ .../MLCreateConnectorRequestTests.java | 2 +- .../MLUpdateConnectorRequestTests.java | 2 +- .../model/MLUpdateModelRequestTest.java | 2 +- .../MLRegisterModelGroupRequestTest.java | 2 +- .../MLUpdateModelGroupRequestTest.java | 2 +- .../register/MLRegisterModelRequestTest.java | 2 +- .../ml/common/utils/StringUtilsTest.java | 64 +++++++++--------- 15 files changed, 110 insertions(+), 99 deletions(-) create mode 100644 common/src/main/java/org/opensearch/ml/common/utils/Validator.java diff --git a/common/src/main/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorRequest.java b/common/src/main/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorRequest.java index 72a5acccb8..793d1563a9 100644 --- a/common/src/main/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorRequest.java +++ b/common/src/main/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorRequest.java @@ -6,7 +6,7 @@ package org.opensearch.ml.common.transport.connector; import static org.opensearch.action.ValidateActions.addValidationError; -import static org.opensearch.ml.common.utils.StringUtils.validateFields; +import static org.opensearch.ml.common.utils.Validator.validateFields; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; diff --git a/common/src/main/java/org/opensearch/ml/common/transport/connector/MLUpdateConnectorRequest.java b/common/src/main/java/org/opensearch/ml/common/transport/connector/MLUpdateConnectorRequest.java index e1fe1db68a..6988ef395c 100644 --- a/common/src/main/java/org/opensearch/ml/common/transport/connector/MLUpdateConnectorRequest.java +++ b/common/src/main/java/org/opensearch/ml/common/transport/connector/MLUpdateConnectorRequest.java @@ -6,7 +6,7 @@ package org.opensearch.ml.common.transport.connector; import static org.opensearch.action.ValidateActions.addValidationError; -import static org.opensearch.ml.common.utils.StringUtils.validateFields; +import static org.opensearch.ml.common.utils.Validator.validateFields; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; diff --git a/common/src/main/java/org/opensearch/ml/common/transport/model/MLUpdateModelRequest.java b/common/src/main/java/org/opensearch/ml/common/transport/model/MLUpdateModelRequest.java index 48389763e8..a3d5d59f1f 100644 --- a/common/src/main/java/org/opensearch/ml/common/transport/model/MLUpdateModelRequest.java +++ b/common/src/main/java/org/opensearch/ml/common/transport/model/MLUpdateModelRequest.java @@ -6,7 +6,7 @@ package org.opensearch.ml.common.transport.model; import static org.opensearch.action.ValidateActions.addValidationError; -import static org.opensearch.ml.common.utils.StringUtils.validateFields; +import static org.opensearch.ml.common.utils.Validator.validateFields; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; diff --git a/common/src/main/java/org/opensearch/ml/common/transport/model_group/MLRegisterModelGroupRequest.java b/common/src/main/java/org/opensearch/ml/common/transport/model_group/MLRegisterModelGroupRequest.java index 5fd56292ee..0908e12838 100644 --- a/common/src/main/java/org/opensearch/ml/common/transport/model_group/MLRegisterModelGroupRequest.java +++ b/common/src/main/java/org/opensearch/ml/common/transport/model_group/MLRegisterModelGroupRequest.java @@ -6,7 +6,7 @@ package org.opensearch.ml.common.transport.model_group; import static org.opensearch.action.ValidateActions.addValidationError; -import static org.opensearch.ml.common.utils.StringUtils.validateFields; +import static org.opensearch.ml.common.utils.Validator.validateFields; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; diff --git a/common/src/main/java/org/opensearch/ml/common/transport/model_group/MLUpdateModelGroupRequest.java b/common/src/main/java/org/opensearch/ml/common/transport/model_group/MLUpdateModelGroupRequest.java index e130975c71..cf82472321 100644 --- a/common/src/main/java/org/opensearch/ml/common/transport/model_group/MLUpdateModelGroupRequest.java +++ b/common/src/main/java/org/opensearch/ml/common/transport/model_group/MLUpdateModelGroupRequest.java @@ -6,7 +6,7 @@ package org.opensearch.ml.common.transport.model_group; import static org.opensearch.action.ValidateActions.addValidationError; -import static org.opensearch.ml.common.utils.StringUtils.validateFields; +import static org.opensearch.ml.common.utils.Validator.validateFields; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; diff --git a/common/src/main/java/org/opensearch/ml/common/transport/register/MLRegisterModelRequest.java b/common/src/main/java/org/opensearch/ml/common/transport/register/MLRegisterModelRequest.java index 4dcfaea971..2221f91c9c 100644 --- a/common/src/main/java/org/opensearch/ml/common/transport/register/MLRegisterModelRequest.java +++ b/common/src/main/java/org/opensearch/ml/common/transport/register/MLRegisterModelRequest.java @@ -6,7 +6,7 @@ package org.opensearch.ml.common.transport.register; import static org.opensearch.action.ValidateActions.addValidationError; -import static org.opensearch.ml.common.utils.StringUtils.validateFields; +import static org.opensearch.ml.common.utils.Validator.validateFields; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; diff --git a/common/src/main/java/org/opensearch/ml/common/utils/StringUtils.java b/common/src/main/java/org/opensearch/ml/common/utils/StringUtils.java index a11f5db440..4fd9332519 100644 --- a/common/src/main/java/org/opensearch/ml/common/utils/StringUtils.java +++ b/common/src/main/java/org/opensearch/ml/common/utils/StringUtils.java @@ -5,8 +5,6 @@ package org.opensearch.ml.common.utils; -import static org.opensearch.action.ValidateActions.addValidationError; - import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.security.AccessController; @@ -30,7 +28,6 @@ import org.json.JSONException; import org.json.JSONObject; import org.opensearch.OpenSearchParseException; -import org.opensearch.action.ActionRequestValidationException; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; @@ -63,11 +60,6 @@ public class StringUtils { + " return input;" + "\n }\n"; - // Regex allows letters, digits, spaces, hyphens, underscores, and dots. - private static final Pattern SAFE_INPUT_PATTERN = Pattern.compile("^[\\p{L}\\p{N}\\s.,!?():@\\-_/'\"]*$"); - - public static final String SAFE_INPUT_DESCRIPTION = "can only contain letters, numbers, spaces, and basic punctuation (.,!?():@-_'/\")"; - public static final Gson gson; static { @@ -505,51 +497,4 @@ public static String hashString(String input) { } } - /** - * Validates a map of fields to ensure that their values only contain allowed characters. - *

- * Allowed characters are: letters, digits, spaces, underscores (_), hyphens (-), dots (.), and colons (:). - * If a value does not comply, a validation error is added. - * - * @param fields A map where the key is the field name (used for error messages) and the value is the text to validate. - * @return An {@link ActionRequestValidationException} containing all validation errors, or {@code null} if all fields are valid. - */ - public static ActionRequestValidationException validateFields(Map fields) { - ActionRequestValidationException exception = null; - - for (Map.Entry entry : fields.entrySet()) { - String key = entry.getKey(); - FieldDescriptor descriptor = entry.getValue(); - String value = descriptor.getValue(); - - if (descriptor.isRequired()) { - if (!isSafeText(value)) { - String reason = (value == null || value.isBlank()) ? "is required and cannot be null or blank" : SAFE_INPUT_DESCRIPTION; - exception = addValidationError(key + " " + reason, exception); - } - } else { - if (value != null && !value.isBlank() && !matchesSafePattern(value)) { - exception = addValidationError(key + " " + SAFE_INPUT_DESCRIPTION, exception); - } - } - } - - return exception; - } - - /** - * Checks if the input is safe (non-null, non-blank, matches safe character set). - * - * @param value The input string to validate - * @return true if input is safe, false otherwise - */ - public static boolean isSafeText(String value) { - return value != null && !value.isBlank() && matchesSafePattern(value); - } - - // Just checks pattern - public static boolean matchesSafePattern(String value) { - return SAFE_INPUT_PATTERN.matcher(value).matches(); - } - } diff --git a/common/src/main/java/org/opensearch/ml/common/utils/Validator.java b/common/src/main/java/org/opensearch/ml/common/utils/Validator.java new file mode 100644 index 0000000000..c19af98585 --- /dev/null +++ b/common/src/main/java/org/opensearch/ml/common/utils/Validator.java @@ -0,0 +1,66 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.ml.common.utils; + +import static org.opensearch.action.ValidateActions.addValidationError; + +import java.util.Map; +import java.util.regex.Pattern; + +import org.opensearch.action.ActionRequestValidationException; + +public class Validator { + public static final String SAFE_INPUT_DESCRIPTION = "can only contain letters, numbers, spaces, and basic punctuation (.,!?():@-_'/\")"; + // Regex allows letters, digits, spaces, hyphens, underscores, and dots. + static final Pattern SAFE_INPUT_PATTERN = Pattern.compile("^[\\p{L}\\p{N}\\s.,!?():@\\-_/'\"]*$"); + + /** + * Validates a map of fields to ensure that their values only contain allowed characters. + *

+ * Allowed characters are: letters, digits, spaces, underscores (_), hyphens (-), dots (.), and colons (:). + * If a value does not comply, a validation error is added. + * + * @param fields A map where the key is the field name (used for error messages) and the value is the text to validate. + * @return An {@link ActionRequestValidationException} containing all validation errors, or {@code null} if all fields are valid. + */ + public static ActionRequestValidationException validateFields(Map fields) { + ActionRequestValidationException exception = null; + + for (Map.Entry entry : fields.entrySet()) { + String key = entry.getKey(); + FieldDescriptor descriptor = entry.getValue(); + String value = descriptor.getValue(); + + if (descriptor.isRequired()) { + if (!isSafeText(value)) { + String reason = (value == null || value.isBlank()) ? "is required and cannot be null or blank" : SAFE_INPUT_DESCRIPTION; + exception = addValidationError(key + " " + reason, exception); + } + } else { + if (value != null && !value.isBlank() && !matchesSafePattern(value)) { + exception = addValidationError(key + " " + SAFE_INPUT_DESCRIPTION, exception); + } + } + } + + return exception; + } + + /** + * Checks if the input is safe (non-null, non-blank, matches safe character set). + * + * @param value The input string to validate + * @return true if input is safe, false otherwise + */ + public static boolean isSafeText(String value) { + return value != null && !value.isBlank() && matchesSafePattern(value); + } + + // Just checks pattern + public static boolean matchesSafePattern(String value) { + return SAFE_INPUT_PATTERN.matcher(value).matches(); + } +} diff --git a/common/src/test/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorRequestTests.java b/common/src/test/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorRequestTests.java index 1fb2069863..e7f4cb9921 100644 --- a/common/src/test/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorRequestTests.java +++ b/common/src/test/java/org/opensearch/ml/common/transport/connector/MLCreateConnectorRequestTests.java @@ -10,7 +10,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; -import static org.opensearch.ml.common.utils.StringUtils.SAFE_INPUT_DESCRIPTION; +import static org.opensearch.ml.common.utils.Validator.SAFE_INPUT_DESCRIPTION; import java.io.IOException; import java.io.UncheckedIOException; diff --git a/common/src/test/java/org/opensearch/ml/common/transport/connector/MLUpdateConnectorRequestTests.java b/common/src/test/java/org/opensearch/ml/common/transport/connector/MLUpdateConnectorRequestTests.java index 5c89ebbd78..8340f12248 100644 --- a/common/src/test/java/org/opensearch/ml/common/transport/connector/MLUpdateConnectorRequestTests.java +++ b/common/src/test/java/org/opensearch/ml/common/transport/connector/MLUpdateConnectorRequestTests.java @@ -10,7 +10,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; -import static org.opensearch.ml.common.utils.StringUtils.SAFE_INPUT_DESCRIPTION; +import static org.opensearch.ml.common.utils.Validator.SAFE_INPUT_DESCRIPTION; import java.io.IOException; import java.io.UncheckedIOException; diff --git a/common/src/test/java/org/opensearch/ml/common/transport/model/MLUpdateModelRequestTest.java b/common/src/test/java/org/opensearch/ml/common/transport/model/MLUpdateModelRequestTest.java index 5085ddbae8..f785f6921f 100644 --- a/common/src/test/java/org/opensearch/ml/common/transport/model/MLUpdateModelRequestTest.java +++ b/common/src/test/java/org/opensearch/ml/common/transport/model/MLUpdateModelRequestTest.java @@ -9,7 +9,7 @@ import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; -import static org.opensearch.ml.common.utils.StringUtils.SAFE_INPUT_DESCRIPTION; +import static org.opensearch.ml.common.utils.Validator.SAFE_INPUT_DESCRIPTION; import java.io.IOException; import java.io.UncheckedIOException; diff --git a/common/src/test/java/org/opensearch/ml/common/transport/model_group/MLRegisterModelGroupRequestTest.java b/common/src/test/java/org/opensearch/ml/common/transport/model_group/MLRegisterModelGroupRequestTest.java index 0b2b1b66da..75f33dc726 100644 --- a/common/src/test/java/org/opensearch/ml/common/transport/model_group/MLRegisterModelGroupRequestTest.java +++ b/common/src/test/java/org/opensearch/ml/common/transport/model_group/MLRegisterModelGroupRequestTest.java @@ -5,7 +5,7 @@ import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; -import static org.opensearch.ml.common.utils.StringUtils.SAFE_INPUT_DESCRIPTION; +import static org.opensearch.ml.common.utils.Validator.SAFE_INPUT_DESCRIPTION; import java.io.IOException; import java.io.UncheckedIOException; diff --git a/common/src/test/java/org/opensearch/ml/common/transport/model_group/MLUpdateModelGroupRequestTest.java b/common/src/test/java/org/opensearch/ml/common/transport/model_group/MLUpdateModelGroupRequestTest.java index 7e9d8fbe3c..a7a9166bea 100644 --- a/common/src/test/java/org/opensearch/ml/common/transport/model_group/MLUpdateModelGroupRequestTest.java +++ b/common/src/test/java/org/opensearch/ml/common/transport/model_group/MLUpdateModelGroupRequestTest.java @@ -4,7 +4,7 @@ import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; -import static org.opensearch.ml.common.utils.StringUtils.SAFE_INPUT_DESCRIPTION; +import static org.opensearch.ml.common.utils.Validator.SAFE_INPUT_DESCRIPTION; import java.io.IOException; import java.io.UncheckedIOException; diff --git a/common/src/test/java/org/opensearch/ml/common/transport/register/MLRegisterModelRequestTest.java b/common/src/test/java/org/opensearch/ml/common/transport/register/MLRegisterModelRequestTest.java index 3917566c57..83e8d4bc97 100644 --- a/common/src/test/java/org/opensearch/ml/common/transport/register/MLRegisterModelRequestTest.java +++ b/common/src/test/java/org/opensearch/ml/common/transport/register/MLRegisterModelRequestTest.java @@ -1,7 +1,7 @@ package org.opensearch.ml.common.transport.register; import static org.junit.Assert.*; -import static org.opensearch.ml.common.utils.StringUtils.SAFE_INPUT_DESCRIPTION; +import static org.opensearch.ml.common.utils.Validator.SAFE_INPUT_DESCRIPTION; import java.io.IOException; import java.io.UncheckedIOException; diff --git a/common/src/test/java/org/opensearch/ml/common/utils/StringUtilsTest.java b/common/src/test/java/org/opensearch/ml/common/utils/StringUtilsTest.java index 693caa7d7a..ea842a402c 100644 --- a/common/src/test/java/org/opensearch/ml/common/utils/StringUtilsTest.java +++ b/common/src/test/java/org/opensearch/ml/common/utils/StringUtilsTest.java @@ -754,16 +754,16 @@ public void testValidateSchema() throws IOException { @Test public void testIsSafeText_ValidInputs() { - assertTrue(StringUtils.isSafeText("Model-Name_1.0")); - assertTrue(StringUtils.isSafeText("This is a description:")); - assertTrue(StringUtils.isSafeText("Name_with-dots.and:colons")); + assertTrue(Validator.isSafeText("Model-Name_1.0")); + assertTrue(Validator.isSafeText("This is a description:")); + assertTrue(Validator.isSafeText("Name_with-dots.and:colons")); } @Test public void testValidateFields_AllValid() { Map fields = Map .of("Field1", new FieldDescriptor("Valid Name 1", true), "Field2", new FieldDescriptor("Another_Valid-Field.Name:Here", true)); - assertNull(StringUtils.validateFields(fields)); + assertNull(Validator.validateFields(fields)); } @Test @@ -777,13 +777,13 @@ public void testValidateFields_OptionalFieldsValidWhenBlank() { "OptionalField3", new FieldDescriptor(null, false) ); - assertNull(StringUtils.validateFields(fields)); + assertNull(Validator.validateFields(fields)); } @Test public void testValidateFields_OptionalFieldInvalidPattern() { Map fields = Map.of("OptionalField1", new FieldDescriptor("Bad@Value$", false)); - ActionRequestValidationException exception = StringUtils.validateFields(fields); + ActionRequestValidationException exception = Validator.validateFields(fields); assertNotNull(exception); assertTrue(exception.getMessage().contains("OptionalField1")); } @@ -791,33 +791,33 @@ public void testValidateFields_OptionalFieldInvalidPattern() { @Test public void testIsSafeText_AdvancedValidInputs() { // Testing all allowed characters - assertTrue(StringUtils.isSafeText("Hello World")); // spaces - assertTrue(StringUtils.isSafeText("Hello.World")); // period - assertTrue(StringUtils.isSafeText("Hello,World")); // comma - assertTrue(StringUtils.isSafeText("Hello!World")); // exclamation - assertTrue(StringUtils.isSafeText("Hello?World")); // question mark - assertTrue(StringUtils.isSafeText("Hello(World)")); // parentheses - assertTrue(StringUtils.isSafeText("Hello:World")); // colon - assertTrue(StringUtils.isSafeText("Hello@World")); // at sign - assertTrue(StringUtils.isSafeText("Hello-World")); // hyphen - assertTrue(StringUtils.isSafeText("Hello_World")); // underscore - assertTrue(StringUtils.isSafeText("Hello'World")); // single quote - assertTrue(StringUtils.isSafeText("Hello\"World")); // double quote + assertTrue(Validator.isSafeText("Hello World")); // spaces + assertTrue(Validator.isSafeText("Hello.World")); // period + assertTrue(Validator.isSafeText("Hello,World")); // comma + assertTrue(Validator.isSafeText("Hello!World")); // exclamation + assertTrue(Validator.isSafeText("Hello?World")); // question mark + assertTrue(Validator.isSafeText("Hello(World)")); // parentheses + assertTrue(Validator.isSafeText("Hello:World")); // colon + assertTrue(Validator.isSafeText("Hello@World")); // at sign + assertTrue(Validator.isSafeText("Hello-World")); // hyphen + assertTrue(Validator.isSafeText("Hello_World")); // underscore + assertTrue(Validator.isSafeText("Hello'World")); // single quote + assertTrue(Validator.isSafeText("Hello\"World")); // double quote } @Test public void testIsSafeText_AdvancedInvalidInputs() { // Testing specifically excluded characters - assertFalse(StringUtils.isSafeText("HelloWorld")); // greater than - assertTrue(StringUtils.isSafeText("Hello/World")); // forward slash - assertFalse(StringUtils.isSafeText("Hello\\World")); // backslash - assertFalse(StringUtils.isSafeText("Hello&World")); // ampersand - assertFalse(StringUtils.isSafeText("Hello+World")); // plus - assertFalse(StringUtils.isSafeText("Hello=World")); // equals - assertFalse(StringUtils.isSafeText("Hello;World")); // semicolon - assertFalse(StringUtils.isSafeText("Hello|World")); // pipe - assertFalse(StringUtils.isSafeText("Hello*World")); // asterisk + assertFalse(Validator.isSafeText("HelloWorld")); // greater than + assertTrue(Validator.isSafeText("Hello/World")); // forward slash + assertFalse(Validator.isSafeText("Hello\\World")); // backslash + assertFalse(Validator.isSafeText("Hello&World")); // ampersand + assertFalse(Validator.isSafeText("Hello+World")); // plus + assertFalse(Validator.isSafeText("Hello=World")); // equals + assertFalse(Validator.isSafeText("Hello;World")); // semicolon + assertFalse(Validator.isSafeText("Hello|World")); // pipe + assertFalse(Validator.isSafeText("Hello*World")); // asterisk } @Test @@ -828,7 +828,7 @@ public void testValidateFields_RequiredFields_MissingOrInvalid() { fields.put("RequiredField3", new FieldDescriptor("Bad@#Char$", true)); fields.put("RequiredField4", new FieldDescriptor(null, true)); - ActionRequestValidationException exception = StringUtils.validateFields(fields); + ActionRequestValidationException exception = Validator.validateFields(fields); assertNotNull(exception); String message = exception.getMessage(); assertTrue(message.contains("RequiredField1")); @@ -840,20 +840,20 @@ public void testValidateFields_RequiredFields_MissingOrInvalid() { @Test public void testValidateFields_EmptyMap() { Map fields = new HashMap<>(); - assertNull(StringUtils.validateFields(fields)); + assertNull(Validator.validateFields(fields)); } @Test public void testValidateFields_UnicodeLettersAndNumbers() { Map fields = Map .of("field1", new FieldDescriptor("Hello世界123", true), "field2", new FieldDescriptor("Café42", true)); - assertNull(StringUtils.validateFields(fields)); + assertNull(Validator.validateFields(fields)); } @Test public void testValidateFields_InvalidCharacterSet() { Map fields = Map.of("Field1", new FieldDescriptor("Bad#Value$With^Weird*Chars", true)); - ActionRequestValidationException exception = StringUtils.validateFields(fields); + ActionRequestValidationException exception = Validator.validateFields(fields); assertNotNull(exception); assertTrue(exception.getMessage().contains("Field1")); }