This is an automated email from the ASF dual-hosted git repository.
bdelacretaz pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-graphql-core.git
The following commit(s) were added to refs/heads/master by this push:
new 08fe036 SLING-12198 - make maxQueryTokens and maxWhitespaceTokens
configurable (#39)
08fe036 is described below
commit 08fe0362b60a24a67c8ef84f0c3d05dd80b4ebc5
Author: Lenard Palko (Lenny) <[email protected]>
AuthorDate: Fri Dec 22 17:43:51 2023 +0100
SLING-12198 - make maxQueryTokens and maxWhitespaceTokens configurable (#39)
---
.../graphql/core/engine/DefaultQueryExecutor.java | 35 ++++++
.../graphql/core/engine/MaxQueryTokensTest.java | 129 +++++++++++++++++++++
.../graphql/core/engine/ResourceQueryTestBase.java | 6 +-
3 files changed, 169 insertions(+), 1 deletion(-)
diff --git
a/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java
b/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java
index 0c092f1..aa5a975 100644
---
a/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java
+++
b/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java
@@ -27,9 +27,12 @@ import java.util.Optional;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.function.Consumer;
import javax.script.ScriptException;
+import graphql.GraphQLContext;
+import graphql.parser.ParserOptions;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.graphql.api.SchemaProvider;
import org.apache.sling.graphql.api.SlingDataFetcher;
@@ -114,6 +117,10 @@ public class DefaultQueryExecutor implements QueryExecutor
{
private final Lock writeLock = readWriteLock.writeLock();
private final SchemaGenerator schemaGenerator = new SchemaGenerator();
+ private int maxQueryTokens;
+
+ private int maxWhitespaceTokens;
+
@Reference
private RankedSchemaProviders schemaProvider;
@@ -136,6 +143,20 @@ public class DefaultQueryExecutor implements QueryExecutor
{
" cached and reused, rather than parsed by the engine
all the time. The cache is a LRU and will store up to this number of schemas."
)
int schemaCacheSize() default 128;
+
+ @AttributeDefinition(
+ name = "Max Query Tokens",
+ description = "The number of GraphQL query tokens to parse.
This is a safety measure to avoid denial of service attacks." +
+ " Change ONLY if you know exactly what you are doing."
+ )
+ int maxQueryTokens() default 15000;
+
+ @AttributeDefinition(
+ name = "Max Whitespace Tokens",
+ description = "The number of GraphQL query whitespace tokens
to parse. This is a safety measure to avoid denial of service attacks." +
+ " Change ONLY if you know exactly what you are doing."
+ )
+ int maxWhitespaceTokens() default 200000;
}
private class ExecutionContext {
@@ -155,9 +176,20 @@ public class DefaultQueryExecutor implements QueryExecutor
{
input = ExecutionInput.newExecutionInput()
.query(query)
.variables(variables)
+ .graphQLContext(getGraphQLContextBuilder())
.build();
}
+
+ private Consumer<GraphQLContext.Builder> getGraphQLContextBuilder() {
+ final ParserOptions parserOptions =
ParserOptions.getDefaultParserOptions().transform(builder ->
+ builder
+ .maxTokens(maxQueryTokens)
+ .maxWhitespaceTokens(maxWhitespaceTokens)
+ .build()
+ );
+ return builder -> builder.put(ParserOptions.class, parserOptions);
+ }
}
@Activate
@@ -166,6 +198,9 @@ public class DefaultQueryExecutor implements QueryExecutor {
if (schemaCacheSize < 0) {
schemaCacheSize = 0;
}
+ maxQueryTokens = config.maxQueryTokens();
+ maxWhitespaceTokens = config.maxWhitespaceTokens();
+
resourceToHashMap = new LRUCache<>(schemaCacheSize);
hashToSchemaMap = new LRUCache<>(schemaCacheSize);
}
diff --git
a/src/test/java/org/apache/sling/graphql/core/engine/MaxQueryTokensTest.java
b/src/test/java/org/apache/sling/graphql/core/engine/MaxQueryTokensTest.java
new file mode 100644
index 0000000..0c10ce0
--- /dev/null
+++ b/src/test/java/org/apache/sling/graphql/core/engine/MaxQueryTokensTest.java
@@ -0,0 +1,129 @@
+/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing,
+ ~ software distributed under the License is distributed on an
+ ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ ~ KIND, either express or implied. See the License for the
+ ~ specific language governing permissions and limitations
+ ~ under the License.
+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
+package org.apache.sling.graphql.core.engine;
+
+import org.apache.sling.graphql.core.mocks.CharacterTypeResolver;
+import org.apache.sling.graphql.core.mocks.EchoDataFetcher;
+import org.apache.sling.graphql.core.mocks.TestUtil;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+import static com.jayway.jsonpath.matchers.JsonPathMatchers.hasJsonPath;
+import static org.hamcrest.Matchers.equalTo;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.hamcrest.Matchers.containsString;
+
+/** Test the SLING-12198 configurable max tokens */
+@RunWith(Parameterized.class)
+public class MaxQueryTokensTest extends ResourceQueryTestBase {
+ private static final int DEFAULT_MAX_QUERY_TOKENS = 15000;
+ private static final String INVALID = "invalid ";
+ private static final String WHITESPACE = " {\t";
+ private final Integer maxQueryTokens;
+ private final Integer maxWhitespaceTokens;
+ private final int nOk;
+ private final int nTokensFailure;
+ private final int nWhitespaceFailure;
+
+ @Parameters(name = "{0}")
+ public static Collection<Object[]> data() {
+ final List<Object[]> result = new ArrayList<>();
+ result.add(new Object[] { "default values", null, null,
DEFAULT_MAX_QUERY_TOKENS - 1, DEFAULT_MAX_QUERY_TOKENS, -1 });
+ result.add(new Object[] { "same values", 100, 100, 99, 100, 100 });
+ result.add(new Object[] { "more whitespace", 100, 200, 99, 100, 150 });
+ return result;
+ }
+
+ public MaxQueryTokensTest(String name, Integer maxTokens, Integer
maxWhitespaceTokens, int nOk, int nTokensFailure,
+ int nWhitespaceFailure) {
+ this.maxQueryTokens = maxTokens;
+ this.maxWhitespaceTokens = maxWhitespaceTokens;
+ this.nOk = nOk;
+ this.nTokensFailure = nTokensFailure;
+ this.nWhitespaceFailure = nWhitespaceFailure;
+ }
+
+ protected Map<String, Object> getQueryExecutorProperties() {
+ final Map<String, Object> props = new HashMap<>();
+ if (maxQueryTokens != null) {
+ props.put("maxQueryTokens", maxQueryTokens);
+ }
+ if (maxWhitespaceTokens != null) {
+ props.put("maxWhitespaceTokens", maxWhitespaceTokens);
+ }
+ return props;
+ }
+
+ static String repeat(String what, int howMany) {
+ final StringBuffer sb = new StringBuffer();
+ for (int i = 0; i < howMany; i++) {
+ sb.append(what);
+ }
+ return sb.toString();
+ }
+
+ private void assertQueryFailure(String query, String expectedError) throws
Exception {
+ final String json = queryJSON(query);
+ assertThat(json, hasJsonPath("$.errors[0].extensions.classification",
is("InvalidSyntax")));
+ assertThat(json, hasJsonPath("$.errors[0].message",
containsString(expectedError)));
+ }
+
+ protected void setupAdditionalServices() {
+ TestUtil.registerSlingTypeResolver(context.bundleContext(),
"character/resolver", new CharacterTypeResolver());
+ TestUtil.registerSlingDataFetcher(context.bundleContext(), "echoNS/echo",
new EchoDataFetcher(null));
+ }
+
+ @Test
+ public void verifyQueriesWork() throws Exception {
+ final String json = queryJSON("{ currentResource { path resourceType } }");
+ assertThat(json, hasJsonPath("$.data.currentResource"));
+ assertThat(json, hasJsonPath("$.data.currentResource.path",
equalTo(resource.getPath())));
+ assertThat(json, hasJsonPath("$.data.currentResource.resourceType",
equalTo(resource.getResourceType())));
+ }
+
+ @Test
+ public void numberOfTokensOk() throws Exception {
+ assertQueryFailure(repeat(INVALID, nOk), "Invalid syntax with offending
token");
+ }
+
+ @Test
+ public void tooManyTokens() throws Exception {
+ assertQueryFailure(repeat(INVALID, nTokensFailure),
+ "'grammar' tokens have been presented. To prevent Denial Of Service");
+ }
+
+ @Test
+ public void tooManyWhitespaceTokens() throws Exception {
+ if (nWhitespaceFailure >= 0) {
+ assertQueryFailure(repeat(WHITESPACE, nWhitespaceFailure),
+ "'whitespace' tokens have been presented. To prevent Denial Of
Service");
+ }
+ }
+}
\ No newline at end of file
diff --git
a/src/test/java/org/apache/sling/graphql/core/engine/ResourceQueryTestBase.java
b/src/test/java/org/apache/sling/graphql/core/engine/ResourceQueryTestBase.java
index 0482e19..34c92fc 100644
---
a/src/test/java/org/apache/sling/graphql/core/engine/ResourceQueryTestBase.java
+++
b/src/test/java/org/apache/sling/graphql/core/engine/ResourceQueryTestBase.java
@@ -70,7 +70,11 @@ public abstract class ResourceQueryTestBase {
context.registerInjectActivateService(new SlingTypeResolverSelector());
context.registerInjectActivateService(new SlingScalarsProvider());
context.registerInjectActivateService(new RankedSchemaProviders());
- context.registerInjectActivateService(new DefaultQueryExecutor());
+ context.registerInjectActivateService(new DefaultQueryExecutor(),
getQueryExecutorProperties());
+ }
+
+ protected Map<String, Object> getQueryExecutorProperties() {
+ return null;
}
protected String queryJSON(String stmt) throws Exception {
