Add parameter to limit the size of a query string #4320
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fmassot
reviewed
Dec 23, 2023
| @@ -187,6 +188,7 @@ impl Default for SearcherConfig { | |||
| max_num_concurrent_split_searches: 100, | |||
| aggregation_memory_limit: ByteSize::mb(500), | |||
| aggregation_bucket_limit: 65000, | |||
| query_string_size_limit: ByteSize::b(1024), | |||
There was a problem hiding this comment.
This limit is too low. For Jaeger, we need to make queries with at least 100 trace ID terms or even bigger ones. A trace ID is 128 bits, we represent it in hex string, so 2 characters for each byte, so we need 32 chars, and I'm not counting the field name size, which has 8 characters.
I feel like we should either put a limit on the number of terms, like 1K terms, or set a 100kb in terms of size.
@fulmicoton @guilload any thoughts?
There was a problem hiding this comment.
Can I go ahead with increasing the default size of the terms to 100kb @fulmicoton @guilload ?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
I added the parameter to limit the size of the query string in a user input. Further, I moved
validate_requestout of the for loop since many of the checks inside it did not require to be run for each index_metadata.An alternate approach for checking the query size could be to match the enums in the serialized query_ast value and check the size of the actual user input.
How was this PR tested?
New unit tests to check if the error is thrown.