You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by gu...@apache.org on 2019/03/12 15:57:28 UTC
[lucene-solr] branch branch_8x updated: SOLR-12891 MacroExpander
will no longer will expand URL parameters by default inside of the 'expr'
parameter,
add InjectionDefense class for safer handling of untrusted data in streaming
expressions and add -DStreamingExpressionMacros system property to revert
to legacy behavior
This is an automated email from the ASF dual-hosted git repository.
gus pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/branch_8x by this push:
new 4708131 SOLR-12891 MacroExpander will no longer will expand URL parameters by default inside of the 'expr' parameter, add InjectionDefense class for safer handling of untrusted data in streaming expressions and add -DStreamingExpressionMacros system property to revert to legacy behavior
4708131 is described below
commit 470813143d3b3a31232de2788a987b01a742c67e
Author: Gus Heck <gu...@apache.org>
AuthorDate: Tue Mar 12 10:46:30 2019 -0400
SOLR-12891 MacroExpander will no longer will expand URL parameters by
default inside of the 'expr' parameter, add InjectionDefense class
for safer handling of untrusted data in streaming expressions and add
-DStreamingExpressionMacros system property to revert to legacy behavior
(cherry picked from commit 9edc557f4526ffbbf35daea06972eb2c595e692b)
---
solr/CHANGES.txt | 41 +++--
.../apache/solr/request/macro/MacroExpander.java | 13 +-
.../solr/request/macro/TestMacroExpander.java | 25 +++
solr/solr-ref-guide/src/streaming-expressions.adoc | 12 +-
.../java/org/apache/solr/client/solrj/io/Lang.java | 2 +-
.../solr/client/solrj/io/stream/NoOpStream.java | 107 +++++++++++
.../stream/expr/InjectedExpressionException.java | 24 +++
.../solrj/io/stream/expr/InjectionDefense.java | 199 +++++++++++++++++++++
.../io/stream/expr/StreamExpressionParser.java | 116 ++++++------
.../org/apache/solr/client/solrj/io/TestLang.java | 2 +-
.../solrj/io/stream/StreamExpressionTest.java | 109 +++++------
.../solrj/io/stream/expr/InjectionDefenseTest.java | 115 ++++++++++++
12 files changed, 625 insertions(+), 140 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 4432922..4479659 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -30,10 +30,15 @@ Jetty 9.4.14.v20181114
Upgrade Notes
----------------------
-When requesting the status of an async request via REQUESTSTATUS collections API, the response will
-include the list of internal async requests (if any) in the "success" or "failed" keys (in addition
-to them being included outside those keys for backwards compatibility). See SOLR-12708 for more
-details
+* When requesting the status of an async request via REQUESTSTATUS collections API, the response will
+ include the list of internal async requests (if any) in the "success" or "failed" keys (in addition
+ to them being included outside those keys for backwards compatibility). See SOLR-12708 for more
+ details
+
+* SOLR-12891: MacroExpander will no longer will expand URL parameters inside of the 'expr' parameter (used by streaming
+ expressions) Additionally, users are advised to use the 'InjectionDefense' class when constructing streaming
+ expressions that include user supplied data to avoid risks similar to SQL injection. The legacy behavior of
+ expanding the 'expr' parameter can be reinstated with -DStreamingExpressionMacros=true passed to the JVM at startup.
New Features
----------------------
@@ -67,7 +72,7 @@ Bug Fixes
* SOLR-12708: Async collection actions should not hide internal failures (Mano Kovacs, Varun Thacker, Tomás Fernández Löbbe)
-* SOLR-11883: 500 code on functional query syntax errors and parameter dereferencing errors
+* SOLR-11883: 500 code on functional query syntax errors and parameter dereferencing errors
(Munendra S N via Mikhail Khludnev)
@@ -79,7 +84,7 @@ Bug Fixes
all the cores.
(Danyal Prout via shalin)
-* SOLR-9882: 500 error code on breaching timeAllowed by core and distributed (fsv) search,
+* SOLR-9882: 500 error code on breaching timeAllowed by core and distributed (fsv) search,
old and json facets (Mikhail Khludnev)
* SOLR-13295: Reproducible failure in TestDistributedGrouping (Erick Erickson)
@@ -87,7 +92,7 @@ Bug Fixes
* SOLR-13254: Correct message that is logged in solrj's ConnectionManager when an exception
occurred while reconnecting to ZooKeeper. (hu xiaodong via Christine Poerschke)
-* SOLR-13284: NullPointerException with 500 http status on omitted or wrong wt param.
+* SOLR-13284: NullPointerException with 500 http status on omitted or wrong wt param.
It's fixed by fallback to json (Munendra S N via Mikhail Khludnev)
Improvements
@@ -162,7 +167,7 @@ Upgrade Notes
it will send and can only be able to handle HTTP/1.1 requests. In case of using SSL Java 9 or latter versions are recommended.
* Custom AuthenticationPlugin must provide its own setup for Http2SolrClient through
- implementing HttpClientBuilderPlugin.setup, if not internal requests can't be authenticated.
+ implementing HttpClientBuilderPlugin.setup, if not internal requests can't be authenticated.
* LUCENE-7996: The 'func' query parser now returns scores that are equal to 0
when a negative value is produced. This change is due to the fact that
@@ -187,7 +192,7 @@ Upgrade Notes
* SOLR-12593: The "extraction" contrib (Solr Cell) no longer does any date parsing, and thus no longer has the
"date.formats" configuration. To ensure date strings are properly parsed, use ParseDateFieldUpdateProcessorFactory
- (an URP) commonly registered with the name "parse-date" in "schemaless mode". (David Smiley, Bar Rotstein)
+ (an URP) commonly registered with the name "parse-date" in "schemaless mode". (David Smiley, Bar Rotstein)
* SOLR-12643: Since Http2SolrClient does not support exposing connections related metrics. These metrics are no longer
available 'QUERY.httpShardHandler.{availableConnections, leasedConnections, maxConnections, pendingConnections}',
@@ -205,7 +210,7 @@ Upgrade Notes
* If you explicitly use BM25SimilarityFactory in your schema, the absolute scoring will be lower due to SOLR-13025.
But ordering of documents will not change in the normal case. Use LegacyBM25SimilarityFactory if you need to force
- the old 6.x/7.x scoring. Note that if you have not specified any similarity in schema or use the default
+ the old 6.x/7.x scoring. Note that if you have not specified any similarity in schema or use the default
SchemaSimilarityFactory, then LegacyBM25Similarity is automatically selected for 'luceneMatchVersion' < 8.0.0.
See also explanation in Reference Guide chapter "Other Schema Elements".
@@ -220,7 +225,7 @@ Upgrade Notes
This choice used to be toggleable with an internal/expert "anonChildDocs" parameter flag which is now gone.
(David Smiley)
-* SOLR-11774: In 'langid' contrib, the LanguageIdentifierUpdateProcessor base class changed some method signatures.
+* SOLR-11774: In 'langid' contrib, the LanguageIdentifierUpdateProcessor base class changed some method signatures.
If you have a custom language identifier implementation you will need to adapt your code.
* SOLR-9515: Hadoop dependencies have been upgraded to Hadoop 3.2.0 from 2.7.2. (Mark Miller, Kevin Risden)
@@ -264,7 +269,7 @@ New Features
* SOLR-12799: Allow Authentication Plugins to intercept internode requests on a per-request basis.
The BasicAuth plugin now supports a new parameter 'forwardCredentials', and when set to 'true',
- user's BasicAuth credentials will be used instead of PKI for client initiated internode requests. (janhoy, noble)
+ user's BasicAuth credentials will be used instead of PKI for client initiated internode requests. (janhoy, noble)
* SOLR-12791: Add Metrics reporting for AuthenticationPlugin (janhoy)
@@ -295,7 +300,7 @@ Bug Fixes
* SOLR-13058: Fix block that was synchronizing on the wrong collection in OverseerTaskProcessor (Gus Heck)
-* SOLR-11774: langid.map.individual now works together with langid.map.keepOrig. Also the detectLanguage() API
+* SOLR-11774: langid.map.individual now works together with langid.map.keepOrig. Also the detectLanguage() API
is changed to accept a Reader allowing for more memory efficient implementations (janhoy)
* SOLR-13126: Query boosts were not being combined correctly for documents where not all boost queries
@@ -336,7 +341,7 @@ Optimizations
* SOLR-12725: ParseDateFieldUpdateProcessorFactory should reuse ParsePosition. (ab)
* SOLR-13025: Due to LUCENE-8563, the BM25Similarity formula no longer includes the (k1+1) factor in the numerator
- This gives a lower absolute score but doesn't affect ordering, as this is a constant factor which is the same
+ This gives a lower absolute score but doesn't affect ordering, as this is a constant factor which is the same
for every document. Use LegacyBM25SimilarityFactory if you need the old 6.x/7.x scoring. See also upgrade notes (janhoy)
* SOLR-13130: during the ResponseBuilder.STAGE_GET_FIELDS directly copy string bytes and avoid creating String Objects (noble)
@@ -441,16 +446,16 @@ Upgrade Notes
New Features
----------------------
-* SOLR-12839: JSON 'terms' Faceting now supports a 'prelim_sort' option to use when initially selecting
+* SOLR-12839: JSON 'terms' Faceting now supports a 'prelim_sort' option to use when initially selecting
the top ranking buckets, prior to the final 'sort' option used after refinement. (hossman)
* SOLR-7896: Add a login page to Admin UI, with initial support for Basic Auth (janhoy)
-* SOLR-13116: Add Admin UI login support for Kerberos (janhoy, Jason Gerlowski)
+* SOLR-13116: Add Admin UI login support for Kerberos (janhoy, Jason Gerlowski)
* SOLR-11126: New Node-level health check handler at /admin/info/healthcheck and /node/health paths that
checks if the node is live, connected to zookeeper and not shutdown. (Anshum Gupta, Amrit Sarkar, shalin)
-
+
* SOLR-12770: Make it possible to configure a host whitelist for distributed search
(Christine Poerschke, janhoy, Erick Erickson, Tomás Fernández Löbbe)
@@ -469,7 +474,7 @@ Bug Fixes
* SOLR-12546: CVSResponseWriter omits useDocValuesAsStored=true field when fl=*
(Munendra S N via Mikhail Khludnev)
-
+
* SOLR-12933: Fix SolrCloud distributed commit. (Mark Miller)
* SOLR-13014: URI Too Long with large streaming expressions in SolrJ (janhoy)
diff --git a/solr/core/src/java/org/apache/solr/request/macro/MacroExpander.java b/solr/core/src/java/org/apache/solr/request/macro/MacroExpander.java
index 9d432fa..67219e5 100644
--- a/solr/core/src/java/org/apache/solr/request/macro/MacroExpander.java
+++ b/solr/core/src/java/org/apache/solr/request/macro/MacroExpander.java
@@ -35,8 +35,7 @@ public class MacroExpander {
private char escape = '\\';
private int level;
private final boolean failOnMissingParams;
-
-
+
public MacroExpander(Map<String,String[]> orig) {
this(orig, false);
}
@@ -58,8 +57,12 @@ public class MacroExpander {
boolean changed = false;
for (Map.Entry<String,String[]> entry : orig.entrySet()) {
String k = entry.getKey();
- String newK = expand(k);
String[] values = entry.getValue();
+ if (!isExpandingExpr() && "expr".equals(k) ) { // SOLR-12891
+ expanded.put(k,values);
+ continue;
+ }
+ String newK = expand(k);
List<String> newValues = null;
for (String v : values) {
String newV = expand(v);
@@ -92,6 +95,10 @@ public class MacroExpander {
return changed;
}
+ private Boolean isExpandingExpr() {
+ return Boolean.valueOf(System.getProperty("StreamingExpressionMacros", "false"));
+ }
+
public String expand(String val) {
level++;
try {
diff --git a/solr/core/src/test/org/apache/solr/request/macro/TestMacroExpander.java b/solr/core/src/test/org/apache/solr/request/macro/TestMacroExpander.java
index 8e3d273..733b960 100644
--- a/solr/core/src/test/org/apache/solr/request/macro/TestMacroExpander.java
+++ b/solr/core/src/test/org/apache/solr/request/macro/TestMacroExpander.java
@@ -118,6 +118,7 @@ public class TestMacroExpander extends SolrTestCase {
public void testMap() { // see SOLR-9740, the second fq param was being dropped.
final Map<String,String[]> request = new HashMap<>();
request.put("fq", new String[] {"zero", "${one_ref}", "two", "${three_ref}"});
+ request.put("expr", new String[] {"${one_ref}"}); // expr is for streaming expressions, no replacement by default
request.put("one_ref",new String[] {"one"});
request.put("three_ref",new String[] {"three"});
Map expanded = MacroExpander.expand(request);
@@ -125,6 +126,30 @@ public class TestMacroExpander extends SolrTestCase {
assertEquals("one", ((String[])expanded.get("fq"))[1]);
assertEquals("two", ((String[]) expanded.get("fq"))[2]);
assertEquals("three", ((String[]) expanded.get("fq"))[3]);
+
+ assertEquals("${one_ref}", ((String[])expanded.get("expr"))[0]);
}
+ @Test
+ public void testMapExprExpandOn() {
+ final Map<String,String[]> request = new HashMap<>();
+ request.put("fq", new String[] {"zero", "${one_ref}", "two", "${three_ref}"});
+ request.put("expr", new String[] {"${one_ref}"}); // expr is for streaming expressions, no replacement by default
+ request.put("one_ref",new String[] {"one"});
+ request.put("three_ref",new String[] {"three"});
+ // I believe that so long as this is sure to be reset before the end of the test we should
+ // be fine with respect to other tests.
+ String oldVal = System.getProperty("StreamingExpressionMacros","false");
+ System.setProperty("StreamingExpressionMacros", "true");
+ try {
+ Map expanded = MacroExpander.expand(request);
+ assertEquals("zero", ((String[])expanded.get("fq"))[0]);
+ assertEquals("one", ((String[])expanded.get("fq"))[1]);
+ assertEquals("two", ((String[]) expanded.get("fq"))[2]);
+ assertEquals("three", ((String[]) expanded.get("fq"))[3]);
+ assertEquals("one", ((String[])expanded.get("expr"))[0]);
+ } finally {
+ System.setProperty("StreamingExpressionMacros", oldVal);
+ }
+ }
}
diff --git a/solr/solr-ref-guide/src/streaming-expressions.adoc b/solr/solr-ref-guide/src/streaming-expressions.adoc
index 97eb297..c1c0404 100644
--- a/solr/solr-ref-guide/src/streaming-expressions.adoc
+++ b/solr/solr-ref-guide/src/streaming-expressions.adoc
@@ -99,11 +99,17 @@ The {solr-javadocs}/solr-solrj/org/apache/solr/client/solrj/io/package-summary.h
[source,java]
----
-StreamFactory streamFactory = new DefaultStreamFactory().withCollectionZkHost("collection1", zkServer.getZkAddress());
-
-ParallelStream pstream = (ParallelStream)streamFactory.constructStream("parallel(collection1, group(search(collection1, q=\"*:*\", fl=\"id,a_s,a_i,a_f\", sort=\"a_s asc,a_f asc\", partitionKeys=\"a_s\"), by=\"a_s asc\"), workers=\"2\", zkHost=\""+zkHost+"\", sort=\"a_s asc\")");
+ StreamFactory streamFactory = new DefaultStreamFactory().withCollectionZkHost("collection1", zkServer.getZkAddress());
+ InjectionDefense defense = new InjectionDefense("parallel(collection1, group(search(collection1, q=\"*:*\", fl=\"id,a_s,a_i,a_f\", sort=\"a_s asc,a_f asc\", partitionKeys=\"a_s\"), by=\"a_s asc\"), workers=\"2\", zkHost=\"?$?\", sort=\"a_s asc\")");
+ defense.addParameter(zkhost);
+ ParallelStream pstream = (ParallelStream)streamFactory.constructStream(defense.safeExpressionString());
----
+Note that InjectionDefense need only be used if the string being inserted could contain user supplied data. See the
+javadoc for `InjectionDefense` for usage details and SOLR-12891 for an example of the potential risks.
+Also note that for security reasons normal parameter substitution no longer applies to the expr parameter
+unless the jvm has been started with `-DStreamingExpressionMacros=true` (usually via `solr.in.sh`)
+
=== Data Requirements
Because streaming expressions relies on the `/export` handler, many of the field and field type requirements to use `/export` are also requirements for `/stream`, particularly for `sort` and `fl` parameters. Please see the section <<exporting-result-sets.adoc#exporting-result-sets,Exporting Result Sets>> for details.
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/io/Lang.java b/solr/solrj/src/java/org/apache/solr/client/solrj/io/Lang.java
index 0e3d2b8..ab14062 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/io/Lang.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/io/Lang.java
@@ -94,7 +94,7 @@ public class Lang {
.withFunctionName("plist", ParallelListStream.class)
.withFunctionName("zplot", ZplotStream.class)
.withFunctionName("hashRollup", HashRollupStream.class)
-
+ .withFunctionName("noop", NoOpStream.class)
// metrics
.withFunctionName("min", MinMetric.class)
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/NoOpStream.java b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/NoOpStream.java
new file mode 100644
index 0000000..8d55c31
--- /dev/null
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/NoOpStream.java
@@ -0,0 +1,107 @@
+/*
+ * 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.solr.client.solrj.io.stream;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+
+import org.apache.solr.client.solrj.io.Tuple;
+import org.apache.solr.client.solrj.io.comp.StreamComparator;
+import org.apache.solr.client.solrj.io.stream.expr.Explanation;
+import org.apache.solr.client.solrj.io.stream.expr.Explanation.ExpressionType;
+import org.apache.solr.client.solrj.io.stream.expr.Expressible;
+import org.apache.solr.client.solrj.io.stream.expr.StreamExplanation;
+import org.apache.solr.client.solrj.io.stream.expr.StreamExpression;
+import org.apache.solr.client.solrj.io.stream.expr.StreamFactory;
+
+/**
+ * A simple no-operation stream. Immediately returns eof. Mostly intended for use as
+ * a place holder in {@link org.apache.solr.client.solrj.io.stream.expr.InjectionDefense}.
+ *
+ * @since 8.0.0
+ */
+public class NoOpStream extends TupleStream implements Expressible {
+
+ private static final long serialVersionUID = 1;
+ private boolean finished;
+
+
+
+ public NoOpStream() throws IOException {
+ }
+
+ public NoOpStream(StreamExpression expression, StreamFactory factory) throws IOException {
+ }
+
+
+ @Override
+ public StreamExpression toExpression(StreamFactory factory) throws IOException{
+ return toExpression(factory, true);
+ }
+
+ private StreamExpression toExpression(StreamFactory factory, boolean includeStreams) throws IOException {
+ // function name
+ StreamExpression expression = new StreamExpression(factory.getFunctionName(this.getClass()));
+
+ return expression;
+ }
+
+ @Override
+ public Explanation toExplanation(StreamFactory factory) throws IOException {
+
+ return new StreamExplanation(getStreamNodeId().toString())
+ .withFunctionName(factory.getFunctionName(this.getClass()))
+ .withImplementingClass(this.getClass().getName())
+ .withExpressionType(ExpressionType.STREAM_DECORATOR)
+ .withExpression(toExpression(factory, false).toString());
+ }
+
+ public void setStreamContext(StreamContext context) {
+ }
+
+ public List<TupleStream> children() {
+ List<TupleStream> l = new ArrayList<TupleStream>();
+ return l;
+ }
+
+ public void open() throws IOException {
+
+ }
+
+ public void close() throws IOException {
+ }
+
+ public Tuple read() throws IOException {
+ HashMap m = new HashMap();
+ m.put("EOF", true);
+ Tuple tuple = new Tuple(m);
+ return tuple;
+ }
+
+ /** Return the stream sort - ie, the order in which records are returned */
+ public StreamComparator getStreamSort(){
+ return null;
+ }
+
+ public int getCost() {
+ return 0;
+ }
+
+
+}
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/InjectedExpressionException.java b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/InjectedExpressionException.java
new file mode 100644
index 0000000..82d05f3
--- /dev/null
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/InjectedExpressionException.java
@@ -0,0 +1,24 @@
+/*
+ * 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.solr.client.solrj.io.stream.expr;
+
+class InjectedExpressionException extends IllegalStateException {
+ InjectedExpressionException(String s) {
+ super(s);
+ }
+}
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/InjectionDefense.java b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/InjectionDefense.java
new file mode 100644
index 0000000..69c43c7
--- /dev/null
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/InjectionDefense.java
@@ -0,0 +1,199 @@
+/*
+ * 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.solr.client.solrj.io.stream.expr;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * A class with which to safely build a streaming expression. Three types of parameters
+ * (String, Numeric, Expression) are accepted and minimally type checked. All parameters
+ * are positional (unnamed) so the order in which parameters are added must correspond to
+ * the order of the parameters in the supplied expression string.<br><br>
+ *
+ * <p>Specifically, this class verifies that the parameter substitutions do not inject
+ * additional expressions, and that the parameters are strings, valid numbers or valid
+ * expressions producing the expected number of sub-expressions. The idea is not to provide
+ * full type safety but rather to heuristically prevent the injection of malicious
+ * expressions. The template expression and the parameters supplied must not contain
+ * comments since injection of comments could be used to hide one or more of the expected
+ * expressions. Use {@link #stripComments(String)} to remove comments.<br><br>
+ *
+ * <p>Valid patterns for parameters are:
+ * <ul>
+ * <li>?$? for strings</li>
+ * <li>?#? for numeric parameters in integer or decimal format (no exponents)</li>
+ * <li>?(n)? for expressions producing n sub-expressions (minimum n=1)</li>
+ * </ul>
+ *
+ * @since 8.0.0
+ */
+
+public class InjectionDefense {
+
+ private static final Pattern STRING_PARAM = Pattern.compile("\\?\\$\\?");
+ private static final Pattern NUMBER_PARAM = Pattern.compile("\\?#\\?");
+ private static final Pattern EXPRESSION_PARAM = Pattern.compile("\\?\\(\\d+\\)\\?");
+ private static final Pattern EXPRESSION_COUNT = Pattern.compile("\\d+");
+ private static final Pattern ANY_PARAM = Pattern.compile("\\?(?:[$#]|(?:\\(\\d+\\)))\\?");
+ private static final Pattern INT_OR_FLOAT = Pattern.compile("-?\\d+(?:\\.\\d+)?");
+
+ private String exprString;
+ private int expressionCount;
+ private List<String> params = new ArrayList<>();
+
+ @SuppressWarnings("WeakerAccess")
+ public InjectionDefense(String exprString) {
+ this.exprString = exprString;
+ checkExpression(exprString);
+ }
+
+ @SuppressWarnings("WeakerAccess")
+ public static String stripComments(String exprString) {
+ return StreamExpressionParser.stripComments(exprString);
+ }
+
+ public void addParameter(String param) {
+ params.add(param);
+ }
+
+ /**
+ * Provides an expression that is guaranteed to have the expected number of sub-expressions
+ *
+ * @return An expression object that should be safe from injection of additional expressions
+ */
+ @SuppressWarnings("WeakerAccess")
+ public StreamExpression safeExpression() {
+ String exprStr = buildExpression();
+ System.out.println(exprStr);
+ StreamExpression parsed = StreamExpressionParser.parse(exprStr);
+ int actual = countExpressions(parsed);
+ if (actual != expressionCount) {
+ throw new InjectedExpressionException("Expected Expression count ("+expressionCount+") does not match actual final " +
+ "expression count ("+actual+")! (possible injection attack?)");
+ } else {
+ return parsed;
+ }
+ }
+
+ /**
+ * Provides a string that is guaranteed to parse to a legal expression and to have the expected
+ * number of sub-expressions.
+ *
+ * @return A string that should be safe from injection of additional expressions.
+ */
+ @SuppressWarnings("WeakerAccess")
+ public String safeExpressionString() {
+ String exprStr = buildExpression();
+ StreamExpression parsed = StreamExpressionParser.parse(exprStr);
+ if (countExpressions(parsed) != expressionCount) {
+ throw new InjectedExpressionException("Expected Expression count does not match Actual final " +
+ "expression count! (possible injection attack?)");
+ } else {
+ return exprStr;
+ }
+
+ }
+
+ String buildExpression() {
+ Matcher anyParam = ANY_PARAM.matcher(exprString);
+ StringBuffer buff = new StringBuffer();
+ int pIdx = 0;
+ while (anyParam.find()) {
+ String found = anyParam.group();
+ String p = params.get(pIdx++);
+ if (found.contains("#")) {
+ if (!INT_OR_FLOAT.matcher(p).matches()) {
+ throw new NumberFormatException("Argument " + pIdx + " (" + p + ") is not numeric!");
+ }
+ }
+ anyParam.appendReplacement(buff, p);
+ }
+ anyParam.appendTail(buff);
+
+ // strip comments may add '\n' at the end so trim()
+ String result = buff.toString().trim();
+ String noComments = stripComments(result).trim();
+ if (!result.equals(noComments)) {
+ throw new IllegalStateException("Comments are not allowed in prepared expressions for security reasons " +
+ "please pre-process stripComments() first. If there were no comments, then they have been injected by " +
+ "a parameter value.");
+ }
+ return buff.toString().trim();
+ }
+
+ /**
+ * Perform some initial checks and establish the expected number of expressions
+ *
+ * @param exprString the expression to check.
+ */
+ private void checkExpression(String exprString) {
+ exprString = STRING_PARAM.matcher(exprString).replaceAll("foo");
+ exprString = NUMBER_PARAM.matcher(exprString).replaceAll("0");
+ Matcher eMatcher = EXPRESSION_PARAM.matcher(exprString);
+ StringBuffer temp = new StringBuffer();
+ while (eMatcher.find()) {
+ Matcher counter = EXPRESSION_COUNT.matcher(eMatcher.group());
+ eMatcher.appendReplacement(temp, "noop()");
+ if (counter.find()) {
+ Integer subExprCount = Integer.valueOf(counter.group());
+ if (subExprCount < 1) {
+ throw new IllegalStateException("Expression Param must contribute at least 1 expression!" +
+ " ?(1)? is the minimum allowed ");
+ }
+ expressionCount += (subExprCount - 1); // the noop() we insert will get counted later.
+ }
+ }
+ eMatcher.appendTail(temp);
+ exprString = temp.toString();
+
+ StreamExpression parsed = StreamExpressionParser.parse(exprString);
+ if (parsed != null) {
+ expressionCount += countExpressions(parsed);
+ } else {
+ throw new IllegalStateException("Invalid expression (parse returned null):" + exprString);
+ }
+ }
+
+ private int countExpressions(StreamExpression expression) {
+ int result = 0;
+ List<StreamExpressionParameter> exprToCheck = new ArrayList<>();
+ exprToCheck.add(expression);
+ while (exprToCheck.size() > 0) {
+ StreamExpressionParameter remove = exprToCheck.remove(0);
+ if (remove instanceof StreamExpressionNamedParameter) {
+ remove = ((StreamExpressionNamedParameter) remove).getParameter();
+ }
+ if (remove instanceof StreamExpression) {
+ result++;
+ for (StreamExpressionParameter parameter : ((StreamExpression) remove).getParameters()) {
+ if (parameter instanceof StreamExpressionNamedParameter) {
+ parameter = ((StreamExpressionNamedParameter) parameter).getParameter();
+ }
+ if (parameter instanceof StreamExpression) {
+ exprToCheck.add(parameter);
+ }
+ }
+ }
+ }
+ return result;
+ }
+
+}
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamExpressionParser.java b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamExpressionParser.java
index 7ae797f..45d5a80 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamExpressionParser.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamExpressionParser.java
@@ -41,43 +41,35 @@ public class StreamExpressionParser {
if(null != expr && expr instanceof StreamExpression){
return (StreamExpression)expr;
}
-
+
return null;
}
- private static String stripComments(String clause) throws RuntimeException {
+ static String stripComments(String clause) throws RuntimeException {
StringBuilder builder = new StringBuilder();
- BufferedReader reader = null;
- try {
- reader = new BufferedReader(new StringReader(clause));
- String line = null;
+ try (BufferedReader reader = new BufferedReader(new StringReader(clause))) {
+ String line;
while ((line = reader.readLine()) != null) {
- if(line.trim().startsWith("#")) {
- continue;
- } else {
- builder.append(line+'\n');
+ if (!line.trim().startsWith("#")) {
+ builder.append(line).append('\n');
}
}
- }catch(Exception e) {
+ } catch (Exception e) {
throw new RuntimeException(e);
- } finally{
- try {
- reader.close();
- } catch (Exception e) {}
}
return builder.toString();
}
-
- private static StreamExpressionParameter generateStreamExpression(String clause){
+
+ private static StreamExpressionParameter generateStreamExpression(String clause){
String working = clause.trim();
-
+
if(!isExpressionClause(working)){
throw new IllegalArgumentException(String.format(Locale.ROOT,"'%s' is not a proper expression clause", working));
}
-
+
// Get functionName
int firstOpenParen = findNextClear(working, 0, '(');
StreamExpression expression = new StreamExpression(working.substring(0, firstOpenParen).trim());
@@ -85,7 +77,7 @@ public class StreamExpressionParser {
// strip off functionName and ()
working = working.substring(firstOpenParen + 1,working.length() - 1).trim();
List<String> parts = splitOn(working,',');
-
+
for(int idx = 0; idx < parts.size(); ++idx){
String part = parts.get(idx).trim();
if(isExpressionClause(part)){
@@ -104,18 +96,18 @@ public class StreamExpressionParser {
expression.addParameter(new StreamExpressionValue(part));
}
}
-
+
return expression;
}
- private static StreamExpressionNamedParameter generateNamedParameterExpression(String clause){
+ private static StreamExpressionNamedParameter generateNamedParameterExpression(String clause){
String working = clause.trim();
-
+
// might be overkill as the only place this is called from does this check already
if(!isNamedParameterClause(working)){
throw new IllegalArgumentException(String.format(Locale.ROOT,"'%s' is not a proper named parameter clause", working));
}
-
+
// Get name
int firstOpenEquals = findNextClear(working, 0, '=');
StreamExpressionNamedParameter namedParameter = new StreamExpressionNamedParameter(working.substring(0, firstOpenEquals).trim());
@@ -133,7 +125,7 @@ public class StreamExpressionParser {
throw new IllegalArgumentException(String.format(Locale.ROOT,"'%s' is not a proper named parameter clause", working));
}
}
-
+
// if contain \" replace with "
if(parameter.contains("\\\"")){
parameter = parameter.replace("\\\"", "\"");
@@ -141,46 +133,46 @@ public class StreamExpressionParser {
throw new IllegalArgumentException(String.format(Locale.ROOT,"'%s' is not a proper named parameter clause", working));
}
}
-
+
namedParameter.setParameter(new StreamExpressionValue(parameter));
}
-
+
return namedParameter;
}
-
+
/* Returns true if the clause is a valid expression clause. This is defined to
* mean it begins with ( and ends with )
* Expects that the passed in clause has already been trimmed of leading and
* trailing spaces*/
private static boolean isExpressionClause(String clause){
// operator(.....something.....)
-
+
// must be balanced
if(!isBalanced(clause)){ return false; }
-
+
// find first (, then check from start to that location and only accept alphanumeric
int firstOpenParen = findNextClear(clause, 0, '(');
if(firstOpenParen <= 0 || firstOpenParen == clause.length() - 1){ return false; }
String functionName = clause.substring(0, firstOpenParen).trim();
if(!wordToken(functionName)){ return false; }
-
+
// Must end with )
return clause.endsWith(")");
}
-
+
private static boolean isNamedParameterClause(String clause){
// name=thing
-
+
// find first = then check from start to that location and only accept alphanumeric
int firstOpenEquals = findNextClear(clause, 0, '=');
if(firstOpenEquals <= 0 || firstOpenEquals == clause.length() - 1){ return false; }
String name = clause.substring(0, firstOpenEquals);
if(!wordToken(name.trim())){ return false; }
-
+
return true;
}
-
+
/* Finds index of the next char equal to findThis that is not within a quote or set of parens
* Does not work with the following values of findThis: " ' \ ) -- well, it might but wouldn't
* really give you what you want. Don't call with those characters */
@@ -189,22 +181,22 @@ public class StreamExpressionParser {
boolean isDoubleQuote = false;
boolean isSingleQuote = false;
boolean isEscaped = false;
-
+
for(int idx = startingIdx; idx < clause.length(); ++idx){
char c = clause.charAt(idx);
-
+
// if we're not in a non-escaped quote or paren state, then we've found the space we want
if(c == findThis && !isEscaped && !isSingleQuote && !isDoubleQuote && 0 == openParens){
return idx;
}
-
-
+
+
switch(c){
case '\\':
// We invert to support situations where \\ exists
isEscaped = !isEscaped;
break;
-
+
case '"':
// if we're not in a non-escaped single quote state, then invert the double quote state
if(!isEscaped && !isSingleQuote){
@@ -212,7 +204,7 @@ public class StreamExpressionParser {
}
isEscaped = false;
break;
-
+
case '\'':
// if we're not in a non-escaped double quote state, then invert the single quote state
if(!isEscaped && !isDoubleQuote){
@@ -220,7 +212,7 @@ public class StreamExpressionParser {
}
isEscaped = false;
break;
-
+
case '(':
// if we're not in a non-escaped quote state, then increment the # of open parens
if(!isEscaped && !isSingleQuote && !isDoubleQuote){
@@ -228,7 +220,7 @@ public class StreamExpressionParser {
}
isEscaped = false;
break;
-
+
case ')':
// if we're not in a non-escaped quote state, then decrement the # of open parens
if(!isEscaped && !isSingleQuote && !isDoubleQuote){
@@ -242,10 +234,10 @@ public class StreamExpressionParser {
}
// Not found
- return -1;
+ return -1;
}
-
+
/* Returns a list of the tokens found. Assumed to be of the form
* 'foo bar baz' and not of the for '(foo bar baz)'
* 'foo bar (baz jaz)' is ok and will return three tokens of
@@ -253,46 +245,46 @@ public class StreamExpressionParser {
*/
private static List<String> splitOn(String clause, char splitOnThis){
String working = clause.trim();
-
+
List<String> parts = new ArrayList<String>();
-
+
while(true){ // will break when next splitOnThis isn't found
int nextIdx = findNextClear(working, 0, splitOnThis);
-
+
if(nextIdx < 0){
parts.add(working);
break;
}
-
+
parts.add(working.substring(0, nextIdx));
-
+
// handle ending splitOnThis
if(nextIdx+1 == working.length()){
break;
}
-
- working = working.substring(nextIdx + 1).trim();
+
+ working = working.substring(nextIdx + 1).trim();
}
-
+
return parts;
}
-
+
/* Returns true if the clause has balanced parenthesis */
private static boolean isBalanced(String clause){
int openParens = 0;
boolean isDoubleQuote = false;
boolean isSingleQuote = false;
boolean isEscaped = false;
-
+
for(int idx = 0; idx < clause.length(); ++idx){
char c = clause.charAt(idx);
-
+
switch(c){
case '\\':
// We invert to support situations where \\ exists
isEscaped = !isEscaped;
break;
-
+
case '"':
// if we're not in a non-escaped single quote state, then invert the double quote state
if(!isEscaped && !isSingleQuote){
@@ -300,7 +292,7 @@ public class StreamExpressionParser {
}
isEscaped = false;
break;
-
+
case '\'':
// if we're not in a non-escaped double quote state, then invert the single quote state
if(!isEscaped && !isDoubleQuote){
@@ -308,7 +300,7 @@ public class StreamExpressionParser {
}
isEscaped = false;
break;
-
+
case '(':
// if we're not in a non-escaped quote state, then increment the # of open parens
if(!isEscaped && !isSingleQuote && !isDoubleQuote){
@@ -316,12 +308,12 @@ public class StreamExpressionParser {
}
isEscaped = false;
break;
-
+
case ')':
// if we're not in a non-escaped quote state, then decrement the # of open parens
if(!isEscaped && !isSingleQuote && !isDoubleQuote){
openParens -= 1;
-
+
// If we're ever < 0 then we know we're not balanced
if(openParens < 0){
return false;
@@ -329,7 +321,7 @@ public class StreamExpressionParser {
}
isEscaped = false;
break;
-
+
default:
isEscaped = false;
}
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/io/TestLang.java b/solr/solrj/src/test/org/apache/solr/client/solrj/io/TestLang.java
index da155fb..b43e963 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/io/TestLang.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/io/TestLang.java
@@ -75,7 +75,7 @@ public class TestLang extends SolrTestCase {
"convexHull", "getVertices", "getBaryCenter", "getArea", "getBoundarySize","oscillate",
"getAmplitude", "getPhase", "getAngularFrequency", "enclosingDisk", "getCenter", "getRadius",
"getSupportPoints", "pairSort", "log10", "plist", "recip", "pivot", "ltrim", "rtrim", "export",
- "zplot", "natural", "repeat", "movingMAD", "hashRollup"};
+ "zplot", "natural", "repeat", "movingMAD", "hashRollup", "noop"};
@Test
public void testLang() {
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionTest.java
index e271401..d4b6ce2 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionTest.java
@@ -412,51 +412,56 @@ public class StreamExpressionTest extends SolrCloudTestCase {
@Test
public void testParameterSubstitution() throws Exception {
+ String oldVal = System.getProperty("StreamingExpressionMacros", "false");
+ System.setProperty("StreamingExpressionMacros", "true");
+ try {
+ new UpdateRequest()
+ .add(id, "0", "a_s", "hello0", "a_i", "0", "a_f", "0")
+ .add(id, "2", "a_s", "hello2", "a_i", "2", "a_f", "0")
+ .add(id, "3", "a_s", "hello3", "a_i", "3", "a_f", "3")
+ .add(id, "4", "a_s", "hello4", "a_i", "4", "a_f", "4")
+ .add(id, "1", "a_s", "hello1", "a_i", "1", "a_f", "1")
+ .commit(cluster.getSolrClient(), COLLECTIONORALIAS);
- new UpdateRequest()
- .add(id, "0", "a_s", "hello0", "a_i", "0", "a_f", "0")
- .add(id, "2", "a_s", "hello2", "a_i", "2", "a_f", "0")
- .add(id, "3", "a_s", "hello3", "a_i", "3", "a_f", "3")
- .add(id, "4", "a_s", "hello4", "a_i", "4", "a_f", "4")
- .add(id, "1", "a_s", "hello1", "a_i", "1", "a_f", "1")
- .commit(cluster.getSolrClient(), COLLECTIONORALIAS);
-
- String url = cluster.getJettySolrRunners().get(0).getBaseUrl().toString() + "/" + COLLECTIONORALIAS;
- List<Tuple> tuples;
- TupleStream stream;
+ String url = cluster.getJettySolrRunners().get(0).getBaseUrl().toString() + "/" + COLLECTIONORALIAS;
+ List<Tuple> tuples;
+ TupleStream stream;
- // Basic test
- ModifiableSolrParams sParams = new ModifiableSolrParams();
- sParams.set("expr", "merge("
- + "${q1},"
- + "${q2},"
- + "on=${mySort})");
- sParams.set(CommonParams.QT, "/stream");
- sParams.set("q1", "search(" + COLLECTIONORALIAS + ", q=\"id:(0 3 4)\", fl=\"id,a_s,a_i,a_f\", sort=${mySort})");
- sParams.set("q2", "search(" + COLLECTIONORALIAS + ", q=\"id:(1)\", fl=\"id,a_s,a_i,a_f\", sort=${mySort})");
- sParams.set("mySort", "a_f asc");
- stream = new SolrStream(url, sParams);
- tuples = getTuples(stream);
+ // Basic test
+ ModifiableSolrParams sParams = new ModifiableSolrParams();
+ sParams.set("expr", "merge("
+ + "${q1},"
+ + "${q2},"
+ + "on=${mySort})");
+ sParams.set(CommonParams.QT, "/stream");
+ sParams.set("q1", "search(" + COLLECTIONORALIAS + ", q=\"id:(0 3 4)\", fl=\"id,a_s,a_i,a_f\", sort=${mySort})");
+ sParams.set("q2", "search(" + COLLECTIONORALIAS + ", q=\"id:(1)\", fl=\"id,a_s,a_i,a_f\", sort=${mySort})");
+ sParams.set("mySort", "a_f asc");
+ stream = new SolrStream(url, sParams);
+ tuples = getTuples(stream);
- assertEquals(4, tuples.size());
- assertOrder(tuples, 0,1,3,4);
+ assertEquals(4, tuples.size());
+ assertOrder(tuples, 0, 1, 3, 4);
- // Basic test desc
- sParams.set("mySort", "a_f desc");
- stream = new SolrStream(url, sParams);
- tuples = getTuples(stream);
+ // Basic test desc
+ sParams.set("mySort", "a_f desc");
+ stream = new SolrStream(url, sParams);
+ tuples = getTuples(stream);
- assertEquals(4, tuples.size());
- assertOrder(tuples, 4, 3, 1, 0);
+ assertEquals(4, tuples.size());
+ assertOrder(tuples, 4, 3, 1, 0);
- // Basic w/ multi comp
- sParams.set("q2", "search(" + COLLECTIONORALIAS + ", q=\"id:(1 2)\", fl=\"id,a_s,a_i,a_f\", sort=${mySort})");
- sParams.set("mySort", "\"a_f asc, a_s asc\"");
- stream = new SolrStream(url, sParams);
- tuples = getTuples(stream);
+ // Basic w/ multi comp
+ sParams.set("q2", "search(" + COLLECTIONORALIAS + ", q=\"id:(1 2)\", fl=\"id,a_s,a_i,a_f\", sort=${mySort})");
+ sParams.set("mySort", "\"a_f asc, a_s asc\"");
+ stream = new SolrStream(url, sParams);
+ tuples = getTuples(stream);
- assertEquals(5, tuples.size());
- assertOrder(tuples, 0, 2, 1, 3, 4);
+ assertEquals(5, tuples.size());
+ assertOrder(tuples, 0, 2, 1, 3, 4);
+ } finally {
+ System.setProperty("StreamingExpressionMacros", oldVal);
+ }
}
@@ -693,8 +698,8 @@ public class StreamExpressionTest extends SolrCloudTestCase {
.withFunctionName("min", MinMetric.class)
.withFunctionName("max", MaxMetric.class)
.withFunctionName("avg", MeanMetric.class)
- .withFunctionName("count", CountMetric.class);
-
+ .withFunctionName("count", CountMetric.class);
+
StreamExpression expression;
TupleStream stream;
List<Tuple> tuples;
@@ -847,11 +852,11 @@ public class StreamExpressionTest extends SolrCloudTestCase {
.add(id, "8", "a_s", "hello3", "a_i", "13", "a_f", "9")
.add(id, "9", "a_s", "hello0", "a_i", "14", "a_f", "10")
.commit(cluster.getSolrClient(), COLLECTIONORALIAS);
-
+
String clause;
TupleStream stream;
List<Tuple> tuples;
-
+
StreamFactory factory = new StreamFactory()
.withCollectionZkHost("collection1", cluster.getZkServer().getZkAddress())
.withFunctionName("facet", FacetStream.class)
@@ -860,7 +865,7 @@ public class StreamExpressionTest extends SolrCloudTestCase {
.withFunctionName("max", MaxMetric.class)
.withFunctionName("avg", MeanMetric.class)
.withFunctionName("count", CountMetric.class);
-
+
// Basic test
clause = "facet("
+ "collection1, "
@@ -876,7 +881,7 @@ public class StreamExpressionTest extends SolrCloudTestCase {
+ "avg(a_i), avg(a_f), "
+ "count(*)"
+ ")";
-
+
stream = factory.constructStream(clause);
tuples = getTuples(stream);
@@ -1526,7 +1531,7 @@ public class StreamExpressionTest extends SolrCloudTestCase {
String clause;
TupleStream stream;
List<Tuple> tuples;
-
+
StreamFactory factory = new StreamFactory()
.withCollectionZkHost("collection1", cluster.getZkServer().getZkAddress())
.withFunctionName("facet", FacetStream.class)
@@ -1535,7 +1540,7 @@ public class StreamExpressionTest extends SolrCloudTestCase {
.withFunctionName("max", MaxMetric.class)
.withFunctionName("avg", MeanMetric.class)
.withFunctionName("count", CountMetric.class);
-
+
// Basic test
clause = "facet("
+ "collection1, "
@@ -1545,7 +1550,7 @@ public class StreamExpressionTest extends SolrCloudTestCase {
+ "bucketSizeLimit=100, "
+ "sum(a_i), count(*)"
+ ")";
-
+
stream = factory.constructStream(clause);
tuples = getTuples(stream);
@@ -2796,7 +2801,7 @@ public class StreamExpressionTest extends SolrCloudTestCase {
StreamContext streamContext = new StreamContext();
streamContext.setSolrClientCache(cache);
String longQuery = "\"id:(" + IntStream.range(0, 4000).mapToObj(i -> "a").collect(Collectors.joining(" ", "", "")) + ")\"";
-
+
try {
assertSuccess("significantTerms("+COLLECTIONORALIAS+", q="+longQuery+", field=\"test_t\", limit=3, minTermLength=1, maxDocFreq=\".5\")", streamContext);
String expr = "timeseries("+COLLECTIONORALIAS+", q="+longQuery+", start=\"2013-01-01T01:00:00.000Z\", " +
@@ -2821,7 +2826,7 @@ public class StreamExpressionTest extends SolrCloudTestCase {
+ "count(*)"
+ ")";
assertSuccess(expr, streamContext);
- expr = "stats(" + COLLECTIONORALIAS + ", q="+longQuery+", sum(a_i), sum(a_f), min(a_i), min(a_f), max(a_i), max(a_f), avg(a_i), avg(a_f), count(*))";
+ expr = "stats(" + COLLECTIONORALIAS + ", q="+longQuery+", sum(a_i), sum(a_f), min(a_i), min(a_f), max(a_i), max(a_f), avg(a_i), avg(a_f), count(*))";
assertSuccess(expr, streamContext);
expr = "search(" + COLLECTIONORALIAS + ", q="+longQuery+", fl=\"id,a_s,a_i,a_f\", sort=\"a_f asc, a_i asc\")";
assertSuccess(expr, streamContext);
@@ -2880,10 +2885,10 @@ public class StreamExpressionTest extends SolrCloudTestCase {
return true;
}
-
+
public boolean assertString(Tuple tuple, String fieldName, String expected) throws Exception {
String actual = (String)tuple.get(fieldName);
-
+
if( (null == expected && null != actual) ||
(null != expected && null == actual) ||
(null != expected && !expected.equals(actual))){
@@ -2901,7 +2906,7 @@ public class StreamExpressionTest extends SolrCloudTestCase {
return true;
}
-
+
protected boolean assertMaps(List<Map> maps, int... ids) throws Exception {
if(maps.size() != ids.length) {
throw new Exception("Expected id count != actual map count:"+ids.length+":"+maps.size());
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/expr/InjectionDefenseTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/expr/InjectionDefenseTest.java
new file mode 100644
index 0000000..4e39500
--- /dev/null
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/expr/InjectionDefenseTest.java
@@ -0,0 +1,115 @@
+/*
+ * 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.solr.client.solrj.io.stream.expr;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+public class InjectionDefenseTest {
+
+ private static final String EXPLOITABLE = "let(a=search(foo,q=\"time_dt:[?$? TO ?$?]\",fl=\"id,time_dt\",sort=\"time_dt asc\"))";
+ private static final String NUMBER = "let(a=search(foo,q=\"gallons_f:[?#? TO ?#?]\",fl=\"id,gallons_f,time_dt\",sort=\"time_dt asc\"))";
+ private static final String NUMBER_OK = "let(a=search(foo,q=\"gallons_f:[2 TO 3.5]\",fl=\"id,gallons_f,time_dt\",sort=\"time_dt asc\"))";
+ private static final String ALLOWED = "let(a=search(foo,q=\"time_dt:[?$? TO ?$?]\",fl=\"id,time_dt\",sort=\"time_dt asc\"), x=?(2)?)";
+ private static final String INJECTED = "let(a=search(foo,q=\"time_dt:[2000-01-01T00:00:00Z TO 2020-01-01T00:00:00Z]\",fl=\"id,time_dt\",sort=\"time_dt asc\"), x=jdbc( connection=\"jdbc:postgresql://localhost:5432/ouchdb\",sql=\"select * from users\",sort=\"id asc\"),z=jdbc( connection=\"jdbc:postgresql://localhost:5432/ouchdb\",sql=\"select * from race_cars\",sort=\"id asc\"))";
+
+ @Test(expected = InjectedExpressionException.class)
+ public void testSafeExpression() {
+
+ InjectionDefense defender = new InjectionDefense(EXPLOITABLE);
+
+ defender.addParameter("2000-01-01T00:00:00Z");
+ defender.addParameter("2020-01-01T00:00:00Z]\",fl=\"id\",sort=\"id asc\"), b=jdbc( connection=\"jdbc:postgresql://localhost:5432/ouchdb\",sql=\"select * from users\",sort=\"id asc\"),c=search(foo,q=\"time_dt:[* TO 2020-01-01T00:00:00Z");
+
+ defender.safeExpression();
+ }
+
+ @Test(expected = InjectedExpressionException.class)
+ public void testSafeString() {
+
+ InjectionDefense defender = new InjectionDefense(EXPLOITABLE);
+
+ defender.addParameter("2000-01-01T00:00:00Z");
+ defender.addParameter("2020-01-01T00:00:00Z]\",fl=\"id\",sort=\"id asc\"), b=jdbc( connection=\"jdbc:postgresql://localhost:5432/ouchdb\",sql=\"select * from users\",sort=\"id asc\"),c=search(foo,q=\"time_dt:[* TO 2020-01-01T00:00:00Z");
+
+ defender.safeExpressionString();
+ }
+
+ @Test
+ public void testExpectedInjectionOfExpressions() {
+ InjectionDefense defender = new InjectionDefense(ALLOWED);
+
+ defender.addParameter("2000-01-01T00:00:00Z");
+ defender.addParameter("2020-01-01T00:00:00Z");
+ defender.addParameter("jdbc( connection=\"jdbc:postgresql://localhost:5432/ouchdb\",sql=\"select * from users\",sort=\"id asc\"),z=jdbc( connection=\"jdbc:postgresql://localhost:5432/ouchdb\",sql=\"select * from race_cars\",sort=\"id asc\")");
+
+ // no exceptions
+ assertNotNull(defender.safeExpression());
+ assertEquals(INJECTED, defender.safeExpressionString());
+
+ }
+
+ @Test(expected = InjectedExpressionException.class)
+ public void testWrongNumberInjected() {
+ InjectionDefense defender = new InjectionDefense(ALLOWED);
+
+ defender.addParameter("2000-01-01T00:00:00Z");
+ defender.addParameter("2020-01-01T00:00:00Z");
+ defender.addParameter("jdbc( connection=\"jdbc:postgresql://localhost:5432/ouchdb\",sql=\"select * from users\",sort=\"id asc\")");
+
+ // no exceptions
+ defender.safeExpression();
+ assertEquals(INJECTED, defender.safeExpressionString());
+
+ }
+
+ @Test
+ public void testBuildExpression() {
+ InjectionDefense defender = new InjectionDefense(ALLOWED);
+
+ defender.addParameter("2000-01-01T00:00:00Z");
+ defender.addParameter("2020-01-01T00:00:00Z");
+ defender.addParameter("jdbc( connection=\"jdbc:postgresql://localhost:5432/ouchdb\",sql=\"select * from users\",sort=\"id asc\"),z=jdbc( connection=\"jdbc:postgresql://localhost:5432/ouchdb\",sql=\"select * from race_cars\",sort=\"id asc\")");
+
+ assertEquals(INJECTED, defender.buildExpression());
+ }
+
+ @Test
+ public void testInjectNumber() {
+ InjectionDefense defender = new InjectionDefense(NUMBER);
+
+ defender.addParameter("2");
+ defender.addParameter("3.5");
+
+ assertEquals(NUMBER_OK, defender.buildExpression());
+ }
+
+ @Test(expected = NumberFormatException.class)
+ public void testInjectAlphaFail() {
+ InjectionDefense defender = new InjectionDefense(NUMBER);
+
+ defender.addParameter("a");
+ defender.addParameter("3.5");
+
+ defender.buildExpression();
+
+ }
+}
+