You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by GitBox <gi...@apache.org> on 2021/12/23 08:24:55 UTC
[GitHub] [jackrabbit-oak] thomasmueller commented on a change in pull request #446: OAK-9651: Protection against very large queries
thomasmueller commented on a change in pull request #446:
URL: https://github.com/apache/jackrabbit-oak/pull/446#discussion_r774398222
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java
##########
@@ -93,6 +93,22 @@
private String[] classNamesIgnoredInCallTrace = new String[] {};
+
+ private static final String OAK_QUERY_LENGTH_WARN_LIMIT = "oak.query.length.warn.limit";
+ private static final String OAK_QUERY_LENGTH_ERROR_LIMIT = "oak.query.length.error.limit";
+
+ private final long queryLengthWarnLimit = Long.getLong(OAK_QUERY_LENGTH_WARN_LIMIT, 102400); // 100 KB
Review comment:
So let's use "1024 * 1024" here (1 MiB)
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java
##########
@@ -151,12 +151,19 @@
} else {
LOG.debug("Parsing {} statement: {}", language, statement);
}
+ QueryEngineSettings settings = context.getSettings();
+ if (statement.length() > (settings.getQueryLengthErrorLimit())){
+ LOG.error("Too large query: " + statement);
+ throw new RuntimeException("Query length "+ statement.length() + " is larger than max supported query length: " + settings.getQueryLengthErrorLimit());
Review comment:
Maybe use ParseException instead of RuntimeException? We could use another exception as well, e.g. IllegalArgumentException, but we use ParseException below as well; I think it's better than to introduce a new exception type.
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java
##########
@@ -93,6 +93,22 @@
private String[] classNamesIgnoredInCallTrace = new String[] {};
+
+ private static final String OAK_QUERY_LENGTH_WARN_LIMIT = "oak.query.length.warn.limit";
+ private static final String OAK_QUERY_LENGTH_ERROR_LIMIT = "oak.query.length.error.limit";
+
+ private final long queryLengthWarnLimit = Long.getLong(OAK_QUERY_LENGTH_WARN_LIMIT, 102400); // 100 KB
+ private final long queryLengthErrorLimit = Long.getLong(OAK_QUERY_LENGTH_ERROR_LIMIT, 1048576); //1MB (1024KB)
Review comment:
We currently want to see if there are warnings, and not throw errors, so 100 * 1024 * 1024 would be better I think (100 MiB). BTW did you know that newer Java versions supports "_" in numbers, so 1_000_000 would be 1 million. But I think using 100 * 1024 is OK (serves the same purpose).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org