honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Makes freezing QueryShardContext safer by stopping overrides (#20800) 

The `QueryShardContext.failIfFrozen()` and `QueryShardContext.freezeContext()`
methods should be final so that overriding/bypassing the freezing of
`QueryShardContext` is not possible. This is important so that we can
trust when the `QueryShardContext` says a request is cacheable.

This change also makes the methods that call `QueryShardContext.failIfFrozen()`
`final` so they cannot be overridden to bypass setting the request as not
cacheable.
This commit is contained in:
Colin Goodheart-Smithe 2016-10-07 14:37:29 +01:00 committed by GitHub
parent 7a403f640b
commit 4981f2fd51
2 changed files with 26 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java
View File

@ -334,7 +334,7 @@ public class QueryShardContext extends QueryRewriteContext {
* Compiles (or retrieves from cache) and binds the parameters to the
* provided script
*/
public SearchScript getSearchScript(Script script, ScriptContext context, Map<String, String> params) {
public final SearchScript getSearchScript(Script script, ScriptContext context, Map<String, String> params) {
failIfFrozen();
return scriptService.search(lookup(), script, context, params);
}
@ -342,7 +342,7 @@ public class QueryShardContext extends QueryRewriteContext {
* Returns a lazily created {@link SearchScript} that is compiled immediately but can be pulled later once all
* parameters are available.
*/
public Function<Map<String, Object>, SearchScript> getLazySearchScript(Script script, ScriptContext context,
public final Function<Map<String, Object>, SearchScript> getLazySearchScript(Script script, ScriptContext context,
Map<String, String> params) {
failIfFrozen();
CompiledScript compile = scriptService.compile(script, context, params);
@ -353,7 +353,7 @@ public class QueryShardContext extends QueryRewriteContext {
* Compiles (or retrieves from cache) and binds the parameters to the
* provided script
*/
public ExecutableScript getExecutableScript(Script script, ScriptContext context, Map<String, String> params) {
public final ExecutableScript getExecutableScript(Script script, ScriptContext context, Map<String, String> params) {
failIfFrozen();
return scriptService.executable(script, context, params);
}
@ -362,7 +362,7 @@ public class QueryShardContext extends QueryRewriteContext {
* Returns a lazily created {@link ExecutableScript} that is compiled immediately but can be pulled later once all
* parameters are available.
*/
public Function<Map<String, Object>, ExecutableScript> getLazyExecutableScript(Script script, ScriptContext context,
public final Function<Map<String, Object>, ExecutableScript> getLazyExecutableScript(Script script, ScriptContext context,
Map<String, String> params) {
failIfFrozen();
CompiledScript executable = scriptService.compile(script, context, params);
@ -373,15 +373,20 @@ public class QueryShardContext extends QueryRewriteContext {
* if this method is called the query context will throw exception if methods are accessed
* that could yield different results across executions like {@link #getTemplateBytes(Script)}
*/
public void freezeContext() {
public final void freezeContext() {
this.frozen.set(Boolean.TRUE);
}
/**
* This method fails if {@link #freezeContext()} is called before on this context.
* This is used to <i>seal</i>
* This method fails if {@link #freezeContext()} is called before on this
* context. This is used to <i>seal</i>.
*
* This methods and all methods that call it should be final to ensure that
* setting the request as not cacheable and the freezing behaviour of this
* class cannot be bypassed. This is important so we can trust when this
* class says a request can be cached.
*/
protected void failIfFrozen() {
protected final void failIfFrozen() {
this.cachable = false;
if (frozen.get() == Boolean.TRUE) {
throw new IllegalArgumentException("features that prevent cachability are disabled on this context");
@ -391,7 +396,7 @@ public class QueryShardContext extends QueryRewriteContext {
}
@Override
public BytesReference getTemplateBytes(Script template) {
public final BytesReference getTemplateBytes(Script template) {
failIfFrozen();
return super.getTemplateBytes(template);
}
@ -399,7 +404,7 @@ public class QueryShardContext extends QueryRewriteContext {
/**
* Returns <code>true</code> iff the result of the processed search request is cachable. Otherwise <code>false</code>
*/
public boolean isCachable() {
public final boolean isCachable() {
return cachable;
}
@ -410,7 +415,8 @@ public class QueryShardContext extends QueryRewriteContext {
return shardId;
}
public long nowInMillis() {
@Override
public final long nowInMillis() {
failIfFrozen();
return super.nowInMillis();
}

12
core/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/ExtendedBoundsTests.java
View File

@ -19,23 +19,26 @@
package org.elasticsearch.search.aggregations.bucket.histogram;
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.ParseFieldMatcher;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.joda.FormatDateTimeFormatter;
import org.elasticsearch.common.joda.Joda;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.SearchParseException;
import org.elasticsearch.search.aggregations.bucket.histogram.ExtendedBounds;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.threadpool.ThreadPoolStats;
import org.joda.time.DateTimeZone;
import org.joda.time.Instant;
@ -93,10 +96,13 @@ public class ExtendedBoundsTests extends ESTestCase {
public void testParseAndValidate() {
long now = randomLong();
Settings indexSettings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1).put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 1).build();
SearchContext context = mock(SearchContext.class);
QueryShardContext qsc = mock(QueryShardContext.class);
QueryShardContext qsc = new QueryShardContext(0,
new IndexSettings(IndexMetaData.builder("foo").settings(indexSettings).build(), indexSettings), null, null, null, null,
null, null, null, null, null, () -> now);
when(context.getQueryShardContext()).thenReturn(qsc);
when(qsc.nowInMillis()).thenReturn(now);
FormatDateTimeFormatter formatter = Joda.forPattern("dateOptionalTime");
DocValueFormat format = new DocValueFormat.DateTime(formatter, DateTimeZone.UTC);