Index: oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/QueryEngineSettingsMBean.java =================================================================== --- oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/QueryEngineSettingsMBean.java (revision 1858534) +++ oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/QueryEngineSettingsMBean.java (working copy) @@ -21,57 +21,57 @@ @ProviderType public interface QueryEngineSettingsMBean { String TYPE = "QueryEngineSettings"; - + /** * Get the limit on how many nodes a query may read at most into memory, for * "order by" and "distinct" queries. If this limit is exceeded, the query * throws an exception. - * + * * @return the limit */ @Description("Get the limit on how many nodes a query may read at most into memory, for " + - "\"order by\" and \"distinct\" queries. If this limit is exceeded, the query throws an exception.") + "\"order by\" and \"distinct\" queries. If this limit is exceeded, the query throws an exception.") long getLimitInMemory(); - + /** * Change the limit. - * + * * @param limitInMemory the new limit */ void setLimitInMemory(long limitInMemory); - + /** * Get the limit on how many nodes a query may read at most (raw read * operations, including skipped nodes). If this limit is exceeded, the * query throws an exception. - * + * * @return the limit */ @Description("Get the limit on how many nodes a query may read at most (raw read " + "operations, including skipped nodes). If this limit is exceeded, the " + - "query throws an exception.") + "query throws an exception.") long getLimitReads(); - + /** * Change the limit. - * + * * @param limitReads the new limit */ void setLimitReads(long limitReads); - + /** * Whether queries that don't use an index will fail (throw an exception). * The default is false. - * + * * @return true if they fail */ @Description("Whether queries that don't use an index will fail (throw an exception). " + - "The default is false.") + "The default is false.") boolean getFailTraversal(); /** * Set whether queries that don't use an index will fail (throw an exception). - * + * * @param failTraversal the new value for this setting */ void setFailTraversal(boolean failTraversal); @@ -81,9 +81,37 @@ * * @return true if enabled */ - @Description("Whether the query result size should return an estimation for large queries.") + @Description("Whether the query result size should return an estimation for large queries.") boolean isFastQuerySize(); void setFastQuerySize(boolean fastQuerySize); + /** + * Set or remove a query validator pattern. + * + * @param key the key + * @param pattern the regular expression pattern (empty to remove the + * pattern) + * @param comment a comment + * @param failQuery whether matching queries should fail (true) or just log + * a warning (false) + */ + @Description("Set or remove a query validator pattern.") + void setQueryValidatorPattern( + @Description("the key") + @Name("key") + String key, + @Description("the regular expression pattern (empty to remove the pattern)") + @Name("pattern") + String pattern, + @Description("a comment") + @Name("comment") + String comment, + @Description("whether matching queries should fail (true) or just log a warning (false)") + @Name("failQuery") + boolean failQuery); + + @Description("Get the query validator data as a JSON string.") + String getQueryValidatorJson(); + } Index: oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/package-info.java =================================================================== --- oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/package-info.java (revision 1858534) +++ oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/package-info.java (working copy) @@ -15,7 +15,7 @@ * limitations under the License. */ -@Version("4.7.1") +@Version("4.8.0") package org.apache.jackrabbit.oak.api.jmx; import org.osgi.annotation.versioning.Version; Index: oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java (revision 1858534) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java (working copy) @@ -756,6 +756,8 @@ regs.add(registerMBean(whiteboard, QueryStatsMBean.class, queryEngineSettings.getQueryStats(), QueryStatsMBean.TYPE, "Oak Query Statistics (Extended)")); + + queryEngineSettings.unwrap().getQueryValidator().init(store); // add index hooks later to prevent the OakInitializer to do excessive indexing commitHooks.add(new EditorHook(new IndexUpdateProvider(indexEditors, failOnMissingIndexProvider))); @@ -930,7 +932,17 @@ public void setFastQuerySize(boolean fastQuerySize) { settings.setFastQuerySize(fastQuerySize); } + + @Override + public void setQueryValidatorPattern(String key, String pattern, String comment, boolean failQuery) { + settings.getQueryValidator().setPattern(key, pattern, comment, failQuery); + } + @Override + public String getQueryValidatorJson() { + return settings.getQueryValidator().getJson(); + } + public QueryStatsMBean getQueryStats() { return settings.getQueryStats(); } Index: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java (revision 1858534) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java (working copy) @@ -157,6 +157,8 @@ NodeTypeInfoProvider nodeTypes = context.getNodeTypeInfoProvider(); QueryEngineSettings settings = context.getSettings(); + settings.getQueryValidator().checkStatement(statement); + QueryExecutionStats stats = settings.getQueryStatsReporter().getQueryExecution(statement, language); SQL2Parser parser = new SQL2Parser(mapper, nodeTypes, settings, stats); Index: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java (revision 1858534) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java (working copy) @@ -18,6 +18,8 @@ */ package org.apache.jackrabbit.oak.query; +import org.apache.jackrabbit.oak.api.jmx.Description; +import org.apache.jackrabbit.oak.api.jmx.Name; import org.apache.jackrabbit.oak.api.jmx.QueryEngineSettingsMBean; import org.apache.jackrabbit.oak.query.stats.QueryStatsMBean; import org.apache.jackrabbit.oak.query.stats.QueryStatsMBeanImpl; @@ -82,6 +84,8 @@ * StatisticsProvider used to record query side metrics. */ private final StatisticsProvider statisticsProvider; + + private final QueryValidator queryValidator = new QueryValidator(); public QueryEngineSettings() { statisticsProvider = StatisticsProvider.NOOP; @@ -155,8 +159,22 @@ public StatisticsProvider getStatisticsProvider() { return statisticsProvider; } + + @Override + public void setQueryValidatorPattern(String key, String pattern, String comment, boolean failQuery) { + queryValidator.setPattern(key, pattern, comment, failQuery); + } @Override + public String getQueryValidatorJson() { + return queryValidator.getJson(); + } + + public QueryValidator getQueryValidator() { + return queryValidator; + } + + @Override public String toString() { return "QueryEngineSettings{" + "limitInMemory=" + limitInMemory + Index: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryValidator.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryValidator.java (nonexistent) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryValidator.java (working copy) @@ -0,0 +1,170 @@ +/* + * 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.jackrabbit.oak.query; + +import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME; + +import java.text.ParseException; +import java.util.concurrent.ConcurrentSkipListMap; +import java.util.regex.Pattern; + +import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.commons.json.JsopBuilder; +import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry; +import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.apache.jackrabbit.oak.spi.state.NodeStore; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * A validator for query. Invalid queries either log a warning, or throw an + * exception when trying to execute. + */ +public class QueryValidator { + + private static final Logger LOG = LoggerFactory.getLogger(QueryValidator.class); + + private static final String QUERY_VALIDATOR = "queryValidator"; + + /** + * The next time to log a warning for a query, in milliseconds. + */ + private static final int NEXT_LOG_MILLIS = 10 * 1000; + + /** + * The set of invalid queries. + */ + private ConcurrentSkipListMap map = new ConcurrentSkipListMap<>(); + + /** + * Add a pattern. + * + * @param key the key + * @param pattern the pattern regular expression - if empty, the entry is removed + * @param comment the comment + * @param failQuery - if true, trying to run such a query will fail; + * otherwise the queries that will work, but will log a warning. + * A warning is logged at most once every 10 seconds. + */ + public void setPattern(String key, String pattern, String comment, boolean failQuery) { + LOG.debug("set pattern key={} pattern={} comment={} failQuery={}", key, pattern, comment, failQuery); + if (pattern.isEmpty()) { + map.remove(key); + } else { + ProblematicQueryPattern p = new ProblematicQueryPattern(key, pattern, comment, failQuery); + map.put(key, p); + } + } + + /** + * Get the current set of pattern data. + * + * @return the json representation + */ + public String getJson() { + JsopBuilder b = new JsopBuilder().array(); + for (ProblematicQueryPattern p : map.values()) { + b.newline().encodedValue(p.getJson()); + } + return b.endArray().toString(); + } + + /** + * Check if a query is valid. It is either valid, logs a warning, or throws a exception if invalid. + * + * @param statement the query statement + * @throws ParseException if it is invalid + */ + public void checkStatement(String statement) throws ParseException { + if (map.isEmpty()) { + // the normal case: no patterns defined + return; + } + for (ProblematicQueryPattern p : map.values()) { + p.checkStatement(statement); + } + } + + public void init(NodeStore store) { + NodeState def = store.getRoot().getChildNode(INDEX_DEFINITIONS_NAME). + getChildNode(QUERY_VALIDATOR); + if (!def.exists()) { + return; + } + for(ChildNodeEntry e : def.getChildNodeEntries()) { + String key = e.getName(); + String pattern = e.getNodeState().getProperty("pattern").getValue(Type.STRING); + String comment = e.getNodeState().getProperty("comment").getValue(Type.STRING); + boolean failQuery = e.getNodeState().getProperty("failQuery").getValue(Type.BOOLEAN); + if (pattern != null && comment != null) { + setPattern(key, pattern, comment, failQuery); + } + } + } + + private static class ProblematicQueryPattern { + + private final String key; + private final String pattern; + private final String comment; + private final Pattern compiledPattern; + private final boolean failQuery; + private long executedLast; + private long executedCount; + + ProblematicQueryPattern(String key, String pattern, String comment, boolean failQuery) { + this.key = key; + this.pattern = pattern; + this.comment = comment; + this.compiledPattern = Pattern.compile(pattern); + this.failQuery = failQuery; + } + + void checkStatement(String statement) throws ParseException { + if (!compiledPattern.matcher(statement).matches()) { + return; + } + executedCount++; + long previousExecuted = executedLast; + long now = System.currentTimeMillis(); + executedLast = now; + if (failQuery) { + String message = "Query is blacklisted: statement=" + statement + " pattern=" + pattern; + ParseException p = new ParseException(message, 0); + LOG.warn(message, p); + throw p; + } else { + String message = "Query is questionable, but executed: statement=" + statement + " pattern=" + pattern; + if (previousExecuted + NEXT_LOG_MILLIS < now) { + LOG.warn(message, new Exception("QueryValidator")); + } else { + LOG.debug(message, new Exception("QueryValidator")); + } + } + } + + public String getJson() { + return new JsopBuilder().object().newline(). + key("key").value(key).newline(). + key("pattern").value(pattern).newline(). + key("comment").value(comment).newline(). + key("failQuery").value(failQuery).newline(). + key("executedLast").value( + executedLast == 0 ? "" : new java.sql.Timestamp(executedLast).toString()).newline(). + key("executedCount").value(executedCount).newline(). + endObject().toString(); + } + } + +} Index: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryValidatorTest.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryValidatorTest.java (nonexistent) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryValidatorTest.java (working copy) @@ -0,0 +1,98 @@ +/* + * 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.jackrabbit.oak.query; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.text.ParseException; +import java.util.List; + +import org.apache.jackrabbit.oak.commons.junit.LogCustomizer; +import org.junit.Test; +import org.slf4j.event.Level; + +/** + * Tests the query validator. + */ +public class QueryValidatorTest { + + @Test + public void empty() throws ParseException { + QueryValidator v = new QueryValidator(); + // expected to be very fast + v.checkStatement("x"); + v.setPattern("x", "x.*", "all", false); + v.setPattern("x", "", "", false); + v.checkStatement("x"); + } + + @Test + public void warning() throws ParseException { + QueryValidator v = new QueryValidator(); + v.setPattern("x", "x.*", "all", false); + assertEquals("[\n" + + "{\n" + + "\"key\":\"x\"\n" + + ",\"pattern\":\"x.*\"\n" + + ",\"comment\":\"all\"\n" + + ",\"failQuery\":false\n" + + ",\"executedLast\":\"\"\n" + + ",\"executedCount\":0\n" + + "}]", v.getJson()); + LogCustomizer customLogs = LogCustomizer.forLogger(QueryValidator.class.getName()).enable(Level.WARN).create(); + try { + customLogs.starting(); + v.checkStatement("x1"); + v.checkStatement("x2"); + v.checkStatement("y"); + List logs = customLogs.getLogs(); + assertEquals("[Query is questionable, but executed: statement=x1 pattern=x.*]", logs.toString()); + } finally { + customLogs.finished(); + } + } + + @Test + public void error() throws ParseException { + QueryValidator v = new QueryValidator(); + v.setPattern("x", "x.*", "all", true); + try { + v.checkStatement("x1"); + fail(); + } catch (ParseException e) { + // expected + } + v.checkStatement("y"); + assertTrue(v.getJson().startsWith("[\n" + + "{\n" + + "\"key\":\"x\"\n" + + ",\"pattern\":\"x.*\"\n" + + ",\"comment\":\"all\"\n" + + ",\"failQuery\":true\n")); + assertTrue(v.getJson().indexOf("\"executedCount\":1") >= 0); + v.checkStatement("y"); + try { + v.checkStatement("x2"); + fail(); + } catch (ParseException e) { + // expected + } + assertTrue(v.getJson().indexOf("\"executedCount\":2") >= 0); + } +}