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

Fixes #850: apply max-doc-properties on indexed properties only, not all #852

Merged
merged 2 commits into from
Feb 1, 2024
Merged
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
2 changes: 1 addition & 1 deletion CONFIGURATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ Here are some Stargate-relevant property groups that are necessary for correct s
| `stargate.jsonapi.document.limits.max-depth` | `int` | `16` | The maximum document depth (nesting). |
| `stargate.jsonapi.document.limits.max-property-path-length` | `int` | `250` | The maximum length of property paths in a document (segments and separating periods) |
| `stargate.jsonapi.document.limits.max-object-properties` | `int` | `1000` | The maximum number of properties any single indexable object in a document can contain. |
| `stargate.jsonapi.document.limits.max-document-properties` | `int` | `2000` | The maximum total number of properties all objects in a document can contain. |
| `stargate.jsonapi.document.limits.max-document-properties` | `int` | `2000` | The maximum number of total indexed properties a document can contain. |
| `stargate.jsonapi.document.limits.max-number-length` | `int` | `100` | The maximum length (in characters) of a single number value in a document. |
| `stargate.jsonapi.document.limits.max-string-length-in-bytes` | `int` | `8000` | The maximum length (in bytes) of a single indexable string value in a document. |
| `stargate.jsonapi.document.limits.max-array-length` | `int` | `1000` | The maximum length (in elements) of a single indexable array in a document. |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,19 @@ public interface DocumentLimitsConfig {
int DEFAULT_MAX_DOCUMENT_DEPTH = 16;

/**
* Defines the default maximum length (in elements) of a single indexable Array in JSON document
* can contain.
* Defines the default maximum length (in elements) of a single indexable Array in JSON document.
*/
int DEFAULT_MAX_ARRAY_LENGTH = 1_000;

/**
* Defines the default maximum number of properties any single indexable Object in JSON document
* can contain.
* Defines the default maximum number of properties for any single indexable Object in JSON
* document.
*/
int DEFAULT_MAX_OBJECT_PROPERTIES = 1000;

/**
* Defines the default maximum number of properties the whole JSON document can contain (including
* Object- and Array-valued properties).
* Defines the default maximum number of indexable properties (properties in indexable Objects)
* for the whole JSON document (including Object- and Array-valued properties).
*/
int DEFAULT_MAX_DOC_PROPERTIES = 2000;

Expand Down Expand Up @@ -72,8 +71,7 @@ public interface DocumentLimitsConfig {
/**
* @return Defines the maximum length of paths to properties in JSON documents, defaults to {@code
* 250 characters}. Note that this is the total length of the path (sequence of one or more
* individual property names separated by comma): individual property name lengths are limited
* by {@link #maxPropertyNameLength()}.
* individual property names separated by comma).
*/
@Positive
@WithDefault("" + DEFAULT_MAX_PROPERTY_PATH_LENGTH)
Expand Down Expand Up @@ -106,16 +104,12 @@ public interface DocumentLimitsConfig {
@WithDefault("" + DEFAULT_MAX_STRING_LENGTH_IN_BYTES)
int maxStringLengthInBytes();

/**
* @return Maximum length of an indexable Array in document, defaults to {@code 1,000} elements.
*/
/** @return Maximum length of an indexable Array in document (in elements). */
@Positive
@WithDefault("" + DEFAULT_MAX_ARRAY_LENGTH)
int maxArrayLength();

/**
* @return Maximum length of Vector ($vector) array allowed, defaults to {@code 4096} elements.
*/
/** @return Maximum length of Vector ($vector) array allowed. */
@Positive
@WithDefault("" + DEFAULT_MAX_VECTOR_EMBEDDING_LENGTH)
int maxVectorEmbeddingLength();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,25 +235,20 @@ private void validateDocumentSize(DocumentLimitsConfig limits, String docJson) {

/**
* Validator applied to the full document, before removing non-indexable fields. Used to ensure
* that it does not violate overall structural limits such as maximum nesting depth.
* that the full document does not violate overall structural limits such as total length or
* maximum nesting depth, or invalid property names. Most checks are done at a later point with
* {@link IndexableValueValidator}.
*/
static class FullDocValidator {
final DocumentLimitsConfig limits;
final AtomicInteger totalProperties;

public FullDocValidator(DocumentLimitsConfig limits) {
this.limits = limits;
totalProperties = new AtomicInteger(0);
}

public void validate(ObjectNode doc) {
// Second: traverse to check for other constraints
validateObjectValue(null, doc, 0, 0);
if (totalProperties.get() > limits.maxDocumentProperties()) {
throw ErrorCode.SHRED_DOC_LIMIT_VIOLATION.toApiException(
"total number of properties (%d) in document exceeds maximum allowed (%d)",
totalProperties.get(), limits.maxDocumentProperties());
}
}

private void validateValue(
Expand Down Expand Up @@ -282,14 +277,13 @@ private void validateObjectValue(
validateDocDepth(limits, depth);

final int propCount = objectValue.size();
totalProperties.addAndGet(propCount);

var it = objectValue.fields();
while (it.hasNext()) {
var entry = it.next();
final String key = entry.getKey();
validateObjectKey(key, entry.getValue(), depth, parentPathLength);
// Path through property consists of segements separated by comma:
// Path through property consists of segments separated by comma:
final int propPathLength = parentPathLength + 1 + key.length();
validateValue(key, entry.getValue(), depth, propPathLength);
}
Expand Down Expand Up @@ -337,12 +331,20 @@ private void validateDocDepth(DocumentLimitsConfig limits, int depth) {
static class IndexableValueValidator {
final DocumentLimitsConfig limits;

final AtomicInteger totalProperties;

public IndexableValueValidator(DocumentLimitsConfig limits) {
this.limits = limits;
totalProperties = new AtomicInteger(0);
}

public void validate(ObjectNode doc) {
validateObjectValue(null, doc);
if (totalProperties.get() > limits.maxDocumentProperties()) {
throw ErrorCode.SHRED_DOC_LIMIT_VIOLATION.toApiException(
"total number of indexed properties (%d) in document exceeds maximum allowed (%d)",
totalProperties.get(), limits.maxDocumentProperties());
}
}

private void validateValue(String referringPropertyName, JsonNode value) {
Expand Down Expand Up @@ -383,6 +385,7 @@ private void validateObjectValue(String referringPropertyName, JsonNode objectVa
"number of properties an indexable Object ('%s') has (%d) exceeds maximum allowed (%s)",
referringPropertyName, objectValue.size(), limits.maxObjectProperties());
}
totalProperties.addAndGet(propCount);

for (Map.Entry<String, JsonNode> entry : objectValue.properties()) {
validateValue(entry.getKey(), entry.getValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,7 @@ public void tryInsertDocWithTooManyProps() {
.body("errors[0].errorCode", is("SHRED_DOC_LIMIT_VIOLATION"))
.body(
"errors[0].message",
startsWith("Document size limitation violated: total number of properties ("))
startsWith("Document size limitation violated: total number of indexed properties ("))
.body("errors[0].message", endsWith(" in document exceeds maximum allowed (2000)"));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public void catchTooManyDocProps() {
.hasFieldOrPropertyWithValue("errorCode", ErrorCode.SHRED_DOC_LIMIT_VIOLATION)
.hasMessageStartingWith(ErrorCode.SHRED_DOC_LIMIT_VIOLATION.getMessage())
.hasMessageEndingWith(
" total number of properties (2101) in document exceeds maximum allowed ("
" total number of indexed properties (2101) in document exceeds maximum allowed ("
+ docLimits.maxDocumentProperties()
+ ")");
}
Expand Down
Loading