From 0cfddbf48921eec618d1dadc69bf585faa7cae9f Mon Sep 17 00:00:00 2001 From: Heemin Kim Date: Thu, 27 Jul 2023 15:28:50 -0700 Subject: [PATCH] Added input parameter validation with few small changes (#371) 1. Added datasource name validation in delete/update api 2. Added datasource name validation in processor creation method 3. Changed index name delimiter from `.` to `-` to be consistent with others Signed-off-by: Heemin Kim --- .../action/DeleteDatasourceRequest.java | 6 +- .../ip2geo/action/PutDatasourceRequest.java | 53 ++----------- .../action/UpdateDatasourceRequest.java | 6 ++ .../ip2geo/common/ParameterValidator.java | 58 ++++++++++++++ .../ip2geo/jobscheduler/Datasource.java | 2 +- .../jobscheduler/DatasourceExtension.java | 2 +- .../ip2geo/processor/Ip2GeoProcessor.java | 22 +++++- .../action/DeleteDatasourceRequestTests.java | 12 +++ .../action/PutDatasourceRequestTests.java | 78 +++++-------------- .../action/UpdateDatasourceRequestTests.java | 14 ++++ .../common/InputFormatValidatorTests.java | 71 +++++++++++++++++ .../processor/Ip2GeoProcessorTests.java | 20 ++++- 12 files changed, 232 insertions(+), 112 deletions(-) create mode 100644 src/main/java/org/opensearch/geospatial/ip2geo/common/ParameterValidator.java create mode 100644 src/test/java/org/opensearch/geospatial/ip2geo/common/InputFormatValidatorTests.java diff --git a/src/main/java/org/opensearch/geospatial/ip2geo/action/DeleteDatasourceRequest.java b/src/main/java/org/opensearch/geospatial/ip2geo/action/DeleteDatasourceRequest.java index 60bc3f0c..80a8ad65 100644 --- a/src/main/java/org/opensearch/geospatial/ip2geo/action/DeleteDatasourceRequest.java +++ b/src/main/java/org/opensearch/geospatial/ip2geo/action/DeleteDatasourceRequest.java @@ -15,6 +15,7 @@ import org.opensearch.action.ActionRequestValidationException; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.geospatial.ip2geo.common.ParameterValidator; /** * GeoIP datasource delete request @@ -23,6 +24,7 @@ @Setter @AllArgsConstructor public class DeleteDatasourceRequest extends ActionRequest { + private static final ParameterValidator VALIDATOR = new ParameterValidator(); /** * @param name the datasource name * @return the datasource name @@ -43,9 +45,9 @@ public DeleteDatasourceRequest(final StreamInput in) throws IOException { @Override public ActionRequestValidationException validate() { ActionRequestValidationException errors = null; - if (name == null || name.isBlank()) { + if (VALIDATOR.validateDatasourceName(name).isEmpty() == false) { errors = new ActionRequestValidationException(); - errors.addValidationError("Datasource name should not be empty"); + errors.addValidationError("no such datasource exist"); } return errors; } diff --git a/src/main/java/org/opensearch/geospatial/ip2geo/action/PutDatasourceRequest.java b/src/main/java/org/opensearch/geospatial/ip2geo/action/PutDatasourceRequest.java index e764f6c4..ddc83942 100644 --- a/src/main/java/org/opensearch/geospatial/ip2geo/action/PutDatasourceRequest.java +++ b/src/main/java/org/opensearch/geospatial/ip2geo/action/PutDatasourceRequest.java @@ -6,27 +6,25 @@ package org.opensearch.geospatial.ip2geo.action; import java.io.IOException; -import java.io.UnsupportedEncodingException; import java.net.MalformedURLException; import java.net.URISyntaxException; import java.net.URL; +import java.util.List; import java.util.Locale; -import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.Setter; import lombok.extern.log4j.Log4j2; -import org.opensearch.OpenSearchException; import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionRequestValidationException; -import org.opensearch.common.Strings; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.ParseField; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.xcontent.ObjectParser; import org.opensearch.geospatial.ip2geo.common.DatasourceManifest; +import org.opensearch.geospatial.ip2geo.common.ParameterValidator; /** * Ip2Geo datasource creation request @@ -34,11 +32,11 @@ @Getter @Setter @Log4j2 -@EqualsAndHashCode(callSuper = false) public class PutDatasourceRequest extends ActionRequest { - private static final int MAX_DATASOURCE_NAME_BYTES = 255; public static final ParseField ENDPOINT_FIELD = new ParseField("endpoint"); public static final ParseField UPDATE_INTERVAL_IN_DAYS_FIELD = new ParseField("update_interval_in_days"); + private static final ParameterValidator VALIDATOR = new ParameterValidator(); + /** * @param name the datasource name * @return the datasource name @@ -96,50 +94,15 @@ public void writeTo(final StreamOutput out) throws IOException { @Override public ActionRequestValidationException validate() { ActionRequestValidationException errors = new ActionRequestValidationException(); - validateDatasourceName(errors); + List errorMsgs = VALIDATOR.validateDatasourceName(name); + if (errorMsgs.isEmpty() == false) { + errorMsgs.stream().forEach(msg -> errors.addValidationError(msg)); + } validateEndpoint(errors); validateUpdateInterval(errors); return errors.validationErrors().isEmpty() ? null : errors; } - private void validateDatasourceName(final ActionRequestValidationException errors) { - if (!Strings.validFileName(name)) { - errors.addValidationError("Datasource name must not contain the following characters " + Strings.INVALID_FILENAME_CHARS); - return; - } - if (name.isEmpty()) { - errors.addValidationError("Datasource name must not be empty"); - return; - } - if (name.contains("#")) { - errors.addValidationError("Datasource name must not contain '#'"); - return; - } - if (name.contains(":")) { - errors.addValidationError("Datasource name must not contain ':'"); - return; - } - if (name.charAt(0) == '_' || name.charAt(0) == '-' || name.charAt(0) == '+') { - errors.addValidationError("Datasource name must not start with '_', '-', or '+'"); - return; - } - int byteCount = 0; - try { - byteCount = name.getBytes("UTF-8").length; - } catch (UnsupportedEncodingException e) { - // UTF-8 should always be supported, but rethrow this if it is not for some reason - throw new OpenSearchException("Unable to determine length of datasource name", e); - } - if (byteCount > MAX_DATASOURCE_NAME_BYTES) { - errors.addValidationError("Datasource name is too long, (" + byteCount + " > " + MAX_DATASOURCE_NAME_BYTES + ")"); - return; - } - if (name.equals(".") || name.equals("..")) { - errors.addValidationError("Datasource name must not be '.' or '..'"); - return; - } - } - /** * Conduct following validation on endpoint * 1. endpoint format complies with RFC-2396 diff --git a/src/main/java/org/opensearch/geospatial/ip2geo/action/UpdateDatasourceRequest.java b/src/main/java/org/opensearch/geospatial/ip2geo/action/UpdateDatasourceRequest.java index 2114d6c1..1d71bbbd 100644 --- a/src/main/java/org/opensearch/geospatial/ip2geo/action/UpdateDatasourceRequest.java +++ b/src/main/java/org/opensearch/geospatial/ip2geo/action/UpdateDatasourceRequest.java @@ -24,6 +24,7 @@ import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.xcontent.ObjectParser; import org.opensearch.geospatial.ip2geo.common.DatasourceManifest; +import org.opensearch.geospatial.ip2geo.common.ParameterValidator; /** * Ip2Geo datasource update request @@ -36,6 +37,8 @@ public class UpdateDatasourceRequest extends ActionRequest { public static final ParseField ENDPOINT_FIELD = new ParseField("endpoint"); public static final ParseField UPDATE_INTERVAL_IN_DAYS_FIELD = new ParseField("update_interval_in_days"); private static final int MAX_DATASOURCE_NAME_BYTES = 255; + private static final ParameterValidator VALIDATOR = new ParameterValidator(); + /** * @param name the datasource name * @return the datasource name @@ -93,6 +96,9 @@ public void writeTo(final StreamOutput out) throws IOException { @Override public ActionRequestValidationException validate() { ActionRequestValidationException errors = new ActionRequestValidationException(); + if (VALIDATOR.validateDatasourceName(name).isEmpty() == false) { + errors.addValidationError("no such datasource exist"); + } if (endpoint == null && updateInterval == null) { errors.addValidationError("no values to update"); } diff --git a/src/main/java/org/opensearch/geospatial/ip2geo/common/ParameterValidator.java b/src/main/java/org/opensearch/geospatial/ip2geo/common/ParameterValidator.java new file mode 100644 index 00000000..56a2ea1d --- /dev/null +++ b/src/main/java/org/opensearch/geospatial/ip2geo/common/ParameterValidator.java @@ -0,0 +1,58 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.geospatial.ip2geo.common; + +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; + +import org.apache.commons.lang3.StringUtils; +import org.opensearch.common.Strings; + +/** + * Parameter validator for IP2Geo APIs + */ +public class ParameterValidator { + private static final int MAX_DATASOURCE_NAME_BYTES = 127; + + /** + * Validate datasource name and return list of error messages + * + * @param datasourceName datasource name + * @return Error messages. Empty list if there is no violation. + */ + public List validateDatasourceName(final String datasourceName) { + List errorMsgs = new ArrayList<>(); + if (StringUtils.isBlank(datasourceName)) { + errorMsgs.add("datasource name must not be empty"); + return errorMsgs; + } + + if (!Strings.validFileName(datasourceName)) { + errorMsgs.add( + String.format(Locale.ROOT, "datasource name must not contain the following characters %s", Strings.INVALID_FILENAME_CHARS) + ); + } + if (datasourceName.contains("#")) { + errorMsgs.add("datasource name must not contain '#'"); + } + if (datasourceName.contains(":")) { + errorMsgs.add("datasource name must not contain ':'"); + } + if (datasourceName.charAt(0) == '_' || datasourceName.charAt(0) == '-' || datasourceName.charAt(0) == '+') { + errorMsgs.add("datasource name must not start with '_', '-', or '+'"); + } + int byteCount = datasourceName.getBytes(StandardCharsets.UTF_8).length; + if (byteCount > MAX_DATASOURCE_NAME_BYTES) { + errorMsgs.add(String.format(Locale.ROOT, "datasource name is too long, (%d > %d)", byteCount, MAX_DATASOURCE_NAME_BYTES)); + } + if (datasourceName.equals(".") || datasourceName.equals("..")) { + errorMsgs.add("datasource name must not be '.' or '..'"); + } + return errorMsgs; + } +} diff --git a/src/main/java/org/opensearch/geospatial/ip2geo/jobscheduler/Datasource.java b/src/main/java/org/opensearch/geospatial/ip2geo/jobscheduler/Datasource.java index a256aa27..551c2a79 100644 --- a/src/main/java/org/opensearch/geospatial/ip2geo/jobscheduler/Datasource.java +++ b/src/main/java/org/opensearch/geospatial/ip2geo/jobscheduler/Datasource.java @@ -50,7 +50,7 @@ public class Datasource implements Writeable, ScheduledJobParameter { /** * Prefix of indices having Ip2Geo data */ - public static final String IP2GEO_DATA_INDEX_NAME_PREFIX = ".geospatial.ip2geo.data"; + public static final String IP2GEO_DATA_INDEX_NAME_PREFIX = ".geospatial-ip2geo-data"; /** * Default fields for job scheduling diff --git a/src/main/java/org/opensearch/geospatial/ip2geo/jobscheduler/DatasourceExtension.java b/src/main/java/org/opensearch/geospatial/ip2geo/jobscheduler/DatasourceExtension.java index ce610ebe..3ba32bbb 100644 --- a/src/main/java/org/opensearch/geospatial/ip2geo/jobscheduler/DatasourceExtension.java +++ b/src/main/java/org/opensearch/geospatial/ip2geo/jobscheduler/DatasourceExtension.java @@ -26,7 +26,7 @@ public class DatasourceExtension implements JobSchedulerExtension { /** * Job index name for a datasource */ - public static final String JOB_INDEX_NAME = ".scheduler_geospatial_ip2geo_datasource"; + public static final String JOB_INDEX_NAME = ".scheduler-geospatial-ip2geo-datasource"; /** * Job index setting * diff --git a/src/main/java/org/opensearch/geospatial/ip2geo/processor/Ip2GeoProcessor.java b/src/main/java/org/opensearch/geospatial/ip2geo/processor/Ip2GeoProcessor.java index 56100c0b..72f07db0 100644 --- a/src/main/java/org/opensearch/geospatial/ip2geo/processor/Ip2GeoProcessor.java +++ b/src/main/java/org/opensearch/geospatial/ip2geo/processor/Ip2GeoProcessor.java @@ -4,6 +4,7 @@ */ package org.opensearch.geospatial.ip2geo.processor; +import static org.opensearch.ingest.ConfigurationUtils.newConfigurationException; import static org.opensearch.ingest.ConfigurationUtils.readBooleanProperty; import static org.opensearch.ingest.ConfigurationUtils.readOptionalList; import static org.opensearch.ingest.ConfigurationUtils.readStringProperty; @@ -17,12 +18,12 @@ import java.util.function.BiConsumer; import java.util.stream.Collectors; -import lombok.AllArgsConstructor; import lombok.Getter; import lombok.extern.log4j.Log4j2; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.geospatial.ip2geo.common.DatasourceState; +import org.opensearch.geospatial.ip2geo.common.ParameterValidator; import org.opensearch.geospatial.ip2geo.dao.DatasourceDao; import org.opensearch.geospatial.ip2geo.dao.GeoIpDataDao; import org.opensearch.geospatial.ip2geo.dao.Ip2GeoCachedDao; @@ -232,13 +233,25 @@ public String getType() { /** * Ip2Geo processor factory */ - @AllArgsConstructor public static final class Factory implements Processor.Factory { + private static final ParameterValidator VALIDATOR = new ParameterValidator(); private final IngestService ingestService; private final DatasourceDao datasourceDao; private final GeoIpDataDao geoIpDataDao; private final Ip2GeoCachedDao ip2GeoCachedDao; + public Factory( + final IngestService ingestService, + final DatasourceDao datasourceDao, + final GeoIpDataDao geoIpDataDao, + final Ip2GeoCachedDao ip2GeoCachedDao + ) { + this.ingestService = ingestService; + this.datasourceDao = datasourceDao; + this.geoIpDataDao = geoIpDataDao; + this.ip2GeoCachedDao = ip2GeoCachedDao; + } + /** * Within this method, blocking request cannot be called because this method is executed in a transport thread. * This means, validation using data in an index won't work. @@ -256,6 +269,11 @@ public Ip2GeoProcessor create( List propertyNames = readOptionalList(TYPE, processorTag, config, CONFIG_PROPERTIES); boolean ignoreMissing = readBooleanProperty(TYPE, processorTag, config, CONFIG_IGNORE_MISSING, false); + List error = VALIDATOR.validateDatasourceName(datasourceName); + if (error.isEmpty() == false) { + throw newConfigurationException(TYPE, processorTag, "datasource", error.get(0)); + } + return new Ip2GeoProcessor( processorTag, description, diff --git a/src/test/java/org/opensearch/geospatial/ip2geo/action/DeleteDatasourceRequestTests.java b/src/test/java/org/opensearch/geospatial/ip2geo/action/DeleteDatasourceRequestTests.java index a3bc17fa..8f53472a 100644 --- a/src/test/java/org/opensearch/geospatial/ip2geo/action/DeleteDatasourceRequestTests.java +++ b/src/test/java/org/opensearch/geospatial/ip2geo/action/DeleteDatasourceRequestTests.java @@ -50,4 +50,16 @@ public void testValidate_whenBlank_thenError() { assertNotNull(error.validationErrors()); assertFalse(error.validationErrors().isEmpty()); } + + public void testValidate_whenInvalidDatasourceName_thenFails() { + String invalidName = "_" + GeospatialTestHelper.randomLowerCaseString(); + DeleteDatasourceRequest request = new DeleteDatasourceRequest(invalidName); + + // Run + ActionRequestValidationException exception = request.validate(); + + // Verify + assertEquals(1, exception.validationErrors().size()); + assertTrue(exception.validationErrors().get(0).contains("no such datasource")); + } } diff --git a/src/test/java/org/opensearch/geospatial/ip2geo/action/PutDatasourceRequestTests.java b/src/test/java/org/opensearch/geospatial/ip2geo/action/PutDatasourceRequestTests.java index b182b3c1..b06651f2 100644 --- a/src/test/java/org/opensearch/geospatial/ip2geo/action/PutDatasourceRequestTests.java +++ b/src/test/java/org/opensearch/geospatial/ip2geo/action/PutDatasourceRequestTests.java @@ -5,13 +5,10 @@ package org.opensearch.geospatial.ip2geo.action; -import java.util.Arrays; import java.util.Locale; -import java.util.Map; import org.opensearch.action.ActionRequestValidationException; import org.opensearch.common.Randomness; -import org.opensearch.common.Strings; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.common.io.stream.BytesStreamInput; @@ -45,14 +42,29 @@ public void testValidate_whenInvalidManifestFile_thenFails() { assertTrue(exception.validationErrors().get(0).contains("Error occurred while reading a file")); } - public void testValidate_whenValidInput_thenSucceed() throws Exception { + public void testValidate_whenValidInput_thenSucceed() { String datasourceName = GeospatialTestHelper.randomLowerCaseString(); PutDatasourceRequest request = new PutDatasourceRequest(datasourceName); request.setEndpoint(sampleManifestUrl()); request.setUpdateInterval(TimeValue.timeValueDays(1)); + assertNull(request.validate()); } + public void testValidate_whenInvalidDatasourceName_thenFails() { + String invalidName = "_" + GeospatialTestHelper.randomLowerCaseString(); + PutDatasourceRequest request = new PutDatasourceRequest(invalidName); + request.setEndpoint(sampleManifestUrl()); + request.setUpdateInterval(TimeValue.timeValueDays(1)); + + // Run + ActionRequestValidationException exception = request.validate(); + + // Verify + assertEquals(1, exception.validationErrors().size()); + assertTrue(exception.validationErrors().get(0).contains("must not")); + } + public void testValidate_whenZeroUpdateInterval_thenFails() throws Exception { String datasourceName = GeospatialTestHelper.randomLowerCaseString(); PutDatasourceRequest request = new PutDatasourceRequest(datasourceName); @@ -98,60 +110,6 @@ public void testValidate_whenInvalidUrlInsideManifest_thenFail() throws Exceptio assertTrue(exception.validationErrors().get(0).contains("Invalid URL format")); } - public void testValidate_whenInvalidDatasourceNames_thenFails() throws Exception { - String validDatasourceName = GeospatialTestHelper.randomLowerCaseString(); - String domain = GeospatialTestHelper.randomLowerCaseString(); - PutDatasourceRequest request = new PutDatasourceRequest(validDatasourceName); - request.setEndpoint(sampleManifestUrl()); - request.setUpdateInterval(TimeValue.timeValueDays(Randomness.get().nextInt(10) + 1)); - - // Run - ActionRequestValidationException exception = request.validate(); - - // Verify - assertNull(exception); - - String fileNameChar = validDatasourceName + Strings.INVALID_FILENAME_CHARS.stream() - .skip(Randomness.get().nextInt(Strings.INVALID_FILENAME_CHARS.size() - 1)) - .findFirst(); - String startsWith = Arrays.asList("_", "-", "+").get(Randomness.get().nextInt(3)) + validDatasourceName; - String empty = ""; - String hash = validDatasourceName + "#"; - String colon = validDatasourceName + ":"; - StringBuilder longName = new StringBuilder(); - while (longName.length() < 256) { - longName.append(GeospatialTestHelper.randomLowerCaseString()); - } - String point = Arrays.asList(".", "..").get(Randomness.get().nextInt(2)); - Map nameToError = Map.of( - fileNameChar, - "not contain the following characters", - empty, - "must not be empty", - hash, - "must not contain '#'", - colon, - "must not contain ':'", - startsWith, - "must not start with", - longName.toString(), - "name is too long", - point, - "must not be '.' or '..'" - ); - - for (Map.Entry entry : nameToError.entrySet()) { - request.setName(entry.getKey()); - - // Run - exception = request.validate(); - - // Verify - assertEquals(1, exception.validationErrors().size()); - assertTrue(exception.validationErrors().get(0).contains(entry.getValue())); - } - } - public void testStreamInOut_whenValidInput_thenSucceed() throws Exception { String datasourceName = GeospatialTestHelper.randomLowerCaseString(); String domain = GeospatialTestHelper.randomLowerCaseString(); @@ -166,6 +124,8 @@ public void testStreamInOut_whenValidInput_thenSucceed() throws Exception { PutDatasourceRequest copiedRequest = new PutDatasourceRequest(input); // Verify - assertEquals(request, copiedRequest); + assertEquals(request.getName(), copiedRequest.getName()); + assertEquals(request.getUpdateInterval(), copiedRequest.getUpdateInterval()); + assertEquals(request.getEndpoint(), copiedRequest.getEndpoint()); } } diff --git a/src/test/java/org/opensearch/geospatial/ip2geo/action/UpdateDatasourceRequestTests.java b/src/test/java/org/opensearch/geospatial/ip2geo/action/UpdateDatasourceRequestTests.java index c37b6830..115c42b3 100644 --- a/src/test/java/org/opensearch/geospatial/ip2geo/action/UpdateDatasourceRequestTests.java +++ b/src/test/java/org/opensearch/geospatial/ip2geo/action/UpdateDatasourceRequestTests.java @@ -69,6 +69,20 @@ public void testValidate_whenValidInput_thenSucceed() { assertNull(request.validate()); } + public void testValidate_whenInvalidDatasourceName_thenFails() { + String invalidName = "_" + GeospatialTestHelper.randomLowerCaseString(); + UpdateDatasourceRequest request = new UpdateDatasourceRequest(invalidName); + request.setEndpoint(sampleManifestUrl()); + request.setUpdateInterval(TimeValue.timeValueDays(1)); + + // Run + ActionRequestValidationException exception = request.validate(); + + // Verify + assertEquals(1, exception.validationErrors().size()); + assertTrue(exception.validationErrors().get(0).contains("no such datasource")); + } + @SneakyThrows public void testValidate_whenZeroUpdateInterval_thenFails() { String datasourceName = GeospatialTestHelper.randomLowerCaseString(); diff --git a/src/test/java/org/opensearch/geospatial/ip2geo/common/InputFormatValidatorTests.java b/src/test/java/org/opensearch/geospatial/ip2geo/common/InputFormatValidatorTests.java new file mode 100644 index 00000000..b9139a84 --- /dev/null +++ b/src/test/java/org/opensearch/geospatial/ip2geo/common/InputFormatValidatorTests.java @@ -0,0 +1,71 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.geospatial.ip2geo.common; + +import java.util.Arrays; +import java.util.List; +import java.util.Map; + +import org.opensearch.common.Randomness; +import org.opensearch.common.Strings; +import org.opensearch.geospatial.GeospatialTestHelper; +import org.opensearch.geospatial.ip2geo.Ip2GeoTestCase; + +public class InputFormatValidatorTests extends Ip2GeoTestCase { + public void testValidateDatasourceName_whenValidName_thenSucceed() { + ParameterValidator inputFormatValidator = new ParameterValidator(); + String validDatasourceName = GeospatialTestHelper.randomLowerCaseString(); + + // Run + List errorMsgs = inputFormatValidator.validateDatasourceName(validDatasourceName); + + // Verify + assertTrue(errorMsgs.isEmpty()); + } + + public void testValidate_whenInvalidDatasourceNames_thenFails() { + ParameterValidator inputFormatValidator = new ParameterValidator(); + String validDatasourceName = GeospatialTestHelper.randomLowerCaseString(); + String fileNameChar = validDatasourceName + Strings.INVALID_FILENAME_CHARS.stream() + .skip(Randomness.get().nextInt(Strings.INVALID_FILENAME_CHARS.size() - 1)) + .findFirst(); + String startsWith = Arrays.asList("_", "-", "+").get(Randomness.get().nextInt(3)) + validDatasourceName; + String empty = ""; + String hash = validDatasourceName + "#"; + String colon = validDatasourceName + ":"; + StringBuilder longName = new StringBuilder(); + while (longName.length() <= 127) { + longName.append(GeospatialTestHelper.randomLowerCaseString()); + } + String point = Arrays.asList(".", "..").get(Randomness.get().nextInt(2)); + Map nameToError = Map.of( + fileNameChar, + "not contain the following characters", + empty, + "must not be empty", + hash, + "must not contain '#'", + colon, + "must not contain ':'", + startsWith, + "must not start with", + longName.toString(), + "name is too long", + point, + "must not be '.' or '..'" + ); + + for (Map.Entry entry : nameToError.entrySet()) { + + // Run + List errorMsgs = inputFormatValidator.validateDatasourceName(entry.getKey()); + + // Verify + assertFalse(errorMsgs.isEmpty()); + assertTrue(errorMsgs.get(0).contains(entry.getValue())); + } + } +} diff --git a/src/test/java/org/opensearch/geospatial/ip2geo/processor/Ip2GeoProcessorTests.java b/src/test/java/org/opensearch/geospatial/ip2geo/processor/Ip2GeoProcessorTests.java index 82233c66..26cc78a0 100644 --- a/src/test/java/org/opensearch/geospatial/ip2geo/processor/Ip2GeoProcessorTests.java +++ b/src/test/java/org/opensearch/geospatial/ip2geo/processor/Ip2GeoProcessorTests.java @@ -24,10 +24,12 @@ import org.junit.Before; import org.mockito.ArgumentCaptor; +import org.opensearch.OpenSearchException; import org.opensearch.common.Randomness; import org.opensearch.geospatial.GeospatialTestHelper; import org.opensearch.geospatial.ip2geo.Ip2GeoTestCase; import org.opensearch.geospatial.ip2geo.common.DatasourceState; +import org.opensearch.geospatial.ip2geo.common.ParameterValidator; import org.opensearch.geospatial.ip2geo.jobscheduler.Datasource; import org.opensearch.ingest.IngestDocument; @@ -35,6 +37,7 @@ public class Ip2GeoProcessorTests extends Ip2GeoTestCase { private static final String DEFAULT_TARGET_FIELD = "ip2geo"; private static final List SUPPORTED_FIELDS = Arrays.asList("city", "country"); private Ip2GeoProcessor.Factory factory; + private ParameterValidator inputFormatValidator; @Before public void init() { @@ -238,7 +241,8 @@ public void testExecute_whenPropertiesSet_thenFilteredGeoIpDataIsAdded() { assertEquals(geoData.get("country"), addedValue.get("country")); } - public void testExecute_whenNoHandler_thenException() throws Exception { + @SneakyThrows + public void testExecute_whenNoHandler_thenException() { String datasourceName = GeospatialTestHelper.randomLowerCaseString(); Ip2GeoProcessor processor = createProcessor(datasourceName, Collections.emptyMap()); IngestDocument document = new IngestDocument(Collections.emptyMap(), Collections.emptyMap()); @@ -246,7 +250,8 @@ public void testExecute_whenNoHandler_thenException() throws Exception { assertTrue(e.getMessage().contains("Not implemented")); } - public void testExecute_whenContainsNonString_thenException() throws Exception { + @SneakyThrows + public void testExecute_whenContainsNonString_thenException() { String datasourceName = GeospatialTestHelper.randomLowerCaseString(); Ip2GeoProcessor processor = createProcessor(datasourceName, Collections.emptyMap()); List ips = Arrays.asList(randomIpAddress(), 1); @@ -264,6 +269,17 @@ public void testExecute_whenContainsNonString_thenException() throws Exception { assertTrue(captor.getValue().getMessage().contains("should only contain strings")); } + @SneakyThrows + public void testCreate_whenInvalidDatasourceName_thenFails() { + String invalidName = "_" + GeospatialTestHelper.randomLowerCaseString(); + + // Run + Exception e = expectThrows(OpenSearchException.class, () -> createProcessor(invalidName, Collections.emptyMap())); + + // Verify + assertTrue(e.getMessage().contains("must not")); + } + private Ip2GeoProcessor createProcessor(final String datasourceName, final Map config) throws Exception { Datasource datasource = new Datasource(); datasource.setName(datasourceName);