diff --git a/src/main/java/io/stargate/sgv2/jsonapi/config/constants/DocumentConstants.java b/src/main/java/io/stargate/sgv2/jsonapi/config/constants/DocumentConstants.java index 97ab3145d7..49c3fd7189 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/config/constants/DocumentConstants.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/config/constants/DocumentConstants.java @@ -1,12 +1,18 @@ package io.stargate.sgv2.jsonapi.config.constants; import io.stargate.sgv2.jsonapi.api.model.command.clause.filter.JsonType; +import java.util.regex.Pattern; public interface DocumentConstants { /** Names of "special" fields in Documents */ interface Fields { /** Primary key for Documents stored; has special handling for many operations. */ String DOC_ID = "_id"; + + // Current definition of valid JSON API names: note that this only validates + // characters, not length limits (nor empty nor "too long" allowed but validated + // separately) + Pattern VALID_NAME_PATTERN = Pattern.compile("[a-zA-Z0-9_]*"); } interface KeyTypeId { diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java index 2e3663d151..c958d7b2de 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java @@ -37,6 +37,8 @@ public enum ErrorCode { SHRED_DOC_LIMIT_VIOLATION("Document size limitation violated"), + SHRED_DOC_KEY_NAME_VIOLATION("Document key name constraints violated"), + SHRED_BAD_EJSON_VALUE("Bad EJSON value"), INVALID_FILTER_EXPRESSION("Invalid filter expression"), diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java index 026cd563d2..e2fbb3117f 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/shredding/Shredder.java @@ -11,6 +11,7 @@ import io.stargate.sgv2.jsonapi.service.shredding.model.DocValueHasher; import io.stargate.sgv2.jsonapi.service.shredding.model.DocumentId; import io.stargate.sgv2.jsonapi.service.shredding.model.WritableShreddedDocument; +import io.stargate.sgv2.jsonapi.util.JsonUtil; import java.io.IOException; import java.util.Iterator; import java.util.Map; @@ -229,17 +230,33 @@ private void validateObjectValue(DocumentLimitsConfig limits, JsonNode objectVal var it = objectValue.fields(); while (it.hasNext()) { var entry = it.next(); - final String key = entry.getKey(); - if (key.length() > documentLimits.maxPropertyNameLength()) { + validateObjectKey(limits, entry.getKey(), entry.getValue()); + validateDocValue(limits, entry.getValue(), depth); + } + } + + private void validateObjectKey(DocumentLimitsConfig limits, String key, JsonNode value) { + if (key.length() > documentLimits.maxPropertyNameLength()) { + throw new JsonApiException( + ErrorCode.SHRED_DOC_LIMIT_VIOLATION, + String.format( + "%s: Property name length (%d) exceeds maximum allowed (%s)", + ErrorCode.SHRED_DOC_LIMIT_VIOLATION.getMessage(), + key.length(), + limits.maxPropertyNameLength())); + } + if (!DocumentConstants.Fields.VALID_NAME_PATTERN.matcher(key).matches()) { + // Special names are accepted in some cases: for now only one such case for + // Date values -- if more needed, will refactor. But for now inline: + if (JsonUtil.EJSON_VALUE_KEY_DATE.equals(key) && value.isValueNode()) { + ; // Fine, looks like legit Date value + } else { throw new JsonApiException( - ErrorCode.SHRED_DOC_LIMIT_VIOLATION, + ErrorCode.SHRED_DOC_KEY_NAME_VIOLATION, String.format( - "%s: Property name length (%d) exceeds maximum allowed (%s)", - ErrorCode.SHRED_DOC_LIMIT_VIOLATION.getMessage(), - key.length(), - limits.maxPropertyNameLength())); + "%s: Property name ('%s') contains character(s) not allowed", + ErrorCode.SHRED_DOC_KEY_NAME_VIOLATION.getMessage(), key)); } - validateDocValue(limits, entry.getValue(), depth); } } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderDocLimitsTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderDocLimitsTest.java index dc4426802a..63998e26a0 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderDocLimitsTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderDocLimitsTest.java @@ -16,6 +16,8 @@ import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; @QuarkusTest @TestProfile(NoGlobalResourcesTestProfile.Impl.class) @@ -178,7 +180,7 @@ class ValidationDocAtomicSizeViolations { public void allowNotTooLongNames() { final ObjectNode doc = objectMapper.createObjectNode(); doc.put("_id", 123); - doc.put("prop-123456789-123456789-123456789-123456789", true); + doc.put("prop_123456789_123456789_123456789_123456789", true); assertThat(shredder.shred(doc)).isNotNull(); } @@ -188,7 +190,7 @@ public void catchTooLongNames() { doc.put("_id", 123); ObjectNode ob = doc.putObject("subdoc"); final String propName = - "property-with-way-too-long-name-123456789-123456789-123456789-123456789"; + "property_with_way_too_long_name_123456789_123456789_123456789_123456789"; ob.put(propName, true); Exception e = catchException(() -> shredder.shred(doc)); @@ -205,11 +207,30 @@ public void catchTooLongNames() { + ")"); } + @ParameterizedTest + @ValueSource(strings = {"$function", "dot.ted", "index[1]"}) + public void catchInvalidFieldName(String invalidName) { + final ObjectNode doc = objectMapper.createObjectNode(); + doc.put("_id", 123); + doc.put(invalidName, 123456); + + Exception e = catchException(() -> shredder.shred(doc)); + assertThat(e) + .isNotNull() + .isInstanceOf(JsonApiException.class) + .hasFieldOrPropertyWithValue("errorCode", ErrorCode.SHRED_DOC_KEY_NAME_VIOLATION) + .hasMessageStartingWith(ErrorCode.SHRED_DOC_KEY_NAME_VIOLATION.getMessage()) + .hasMessageEndingWith( + "Document key name constraints violated: Property name ('" + + invalidName + + "') contains character(s) not allowed"); + } + @Test public void allowNotTooLongStringValues() { final ObjectNode doc = objectMapper.createObjectNode(); doc.put("_id", 123); - // Max is 16_000 so do bit less + // Max is 16_000 so do a bit less doc.put("text", RandomStringUtils.randomAscii(12_000)); assertThat(shredder.shred(doc)).isNotNull(); } diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderTest.java index bb99386ffd..744564a6e2 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/shredding/ShredderTest.java @@ -43,7 +43,7 @@ public void shredSimpleWithId() throws Exception { { "_id" : "abc", "name" : "Bob", "values" : [ 1, 2 ], - "[extra.stuff]" : true, + "extra_stuff" : true, "nullable" : null } """; @@ -57,7 +57,7 @@ public void shredSimpleWithId() throws Exception { JsonPath.from("values"), JsonPath.from("values.0", true), JsonPath.from("values.1", true), - JsonPath.from("[extra.stuff]"), + JsonPath.from("extra_stuff"), JsonPath.from("nullable")); // First verify paths @@ -76,7 +76,7 @@ public void shredSimpleWithId() throws Exception { "name SBob", "values N1", "values N2", - "[extra.stuff] B1", + "extra_stuff B1", "nullable Z", "values.0 N1", "values.1 N2"); @@ -90,7 +90,7 @@ public void shredSimpleWithId() throws Exception { // Then atomic value containers assertThat(doc.queryBoolValues()) - .isEqualTo(Collections.singletonMap(JsonPath.from("[extra.stuff]"), Boolean.TRUE)); + .isEqualTo(Collections.singletonMap(JsonPath.from("extra_stuff"), Boolean.TRUE)); Map expNums = new LinkedHashMap<>(); expNums.put(JsonPath.from("values.0", true), BigDecimal.valueOf(1)); expNums.put(JsonPath.from("values.1", true), BigDecimal.valueOf(2)); @@ -271,8 +271,8 @@ public void badEJSONUnrecognized() { assertThat(t) .isNotNull() - .hasMessage("Bad EJSON value: unrecognized type '$unknownType' (path 'value')") - .hasFieldOrPropertyWithValue("errorCode", ErrorCode.SHRED_BAD_EJSON_VALUE); + .hasMessageStartingWith("Document key name constraints violated") + .hasFieldOrPropertyWithValue("errorCode", ErrorCode.SHRED_DOC_KEY_NAME_VIOLATION); } }