From 3528cc32cb634137cf389beaa9ecdc2036588d96 Mon Sep 17 00:00:00 2001 From: Dennis Gove Date: Wed, 3 Feb 2016 20:17:29 -0500 Subject: [PATCH] SOLR-8409: Ensures that quotes in solr params (eg. q param) are properly handled --- solr/CHANGES.txt | 2 ++ .../solrj/io/stream/CloudSolrStream.java | 12 +++++++++--- .../expr/StreamExpressionNamedParameter.java | 2 -- .../stream/expr/StreamExpressionParser.java | 9 +++++++++ .../StreamExpressionToExpessionTest.java | 19 +++++++++++++++++++ 5 files changed, 39 insertions(+), 5 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index afe5a48f9c7..d34b601e0b2 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -158,6 +158,8 @@ Bug Fixes values settings when copying a FieldInfo (Ishan Chattopadhyaya via Mike McCandless) +* SOLR-8409: Ensures that quotes in solr params (eg. q param) are properly handled (Dennis Gove) + Optimizations ---------------------- * SOLR-7876: Speed up queries and operations that use many terms when timeAllowed has not been diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/CloudSolrStream.java b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/CloudSolrStream.java index a1a54bb54d0..f63c6427c13 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/CloudSolrStream.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/CloudSolrStream.java @@ -21,7 +21,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -38,8 +37,8 @@ import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.client.solrj.io.SolrClientCache; import org.apache.solr.client.solrj.io.Tuple; import org.apache.solr.client.solrj.io.comp.ComparatorOrder; -import org.apache.solr.client.solrj.io.comp.MultipleFieldComparator; import org.apache.solr.client.solrj.io.comp.FieldComparator; +import org.apache.solr.client.solrj.io.comp.MultipleFieldComparator; import org.apache.solr.client.solrj.io.comp.StreamComparator; import org.apache.solr.client.solrj.io.stream.expr.Expressible; import org.apache.solr.client.solrj.io.stream.expr.StreamExpression; @@ -165,7 +164,14 @@ public class CloudSolrStream extends TupleStream implements Expressible { // parameters for(Entry param : params.entrySet()){ - expression.addParameter(new StreamExpressionNamedParameter(param.getKey(), param.getValue())); + String value = param.getValue(); + + // SOLR-8409: This is a special case where the params contain a " character + // Do note that in any other BASE streams with parameters where a " might come into play + // that this same replacement needs to take place. + value = value.replace("\"", "\\\""); + + expression.addParameter(new StreamExpressionNamedParameter(param.getKey(), value)); } // zkHost diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamExpressionNamedParameter.java b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamExpressionNamedParameter.java index 74fdb41ee7a..cc1233c9ac8 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamExpressionNamedParameter.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamExpressionNamedParameter.java @@ -1,7 +1,5 @@ package org.apache.solr.client.solrj.io.stream.expr; -import java.util.ArrayList; -import java.util.List; /* * Licensed to the Apache Software Foundation (ASF) under one or more 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 305e121e199..1593f09a075 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 @@ -105,6 +105,15 @@ 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("\\\"", "\""); + if(0 == parameter.length()){ + throw new IllegalArgumentException(String.format(Locale.ROOT,"'%s' is not a proper named parameter clause", working)); + } + } + namedParameter.setParameter(new StreamExpressionValue(parameter)); } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionToExpessionTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionToExpessionTest.java index 5eaba9037db..bc98a7b5f1b 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionToExpessionTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionToExpessionTest.java @@ -307,6 +307,25 @@ public class StreamExpressionToExpessionTest extends LuceneTestCase { assertTrue(expressionString.contains("on=\"a_f,a_s\"")); } + @Test + public void testCloudSolrStreamWithEscapedQuote() throws Exception { + + // The purpose of this test is to ensure that a parameter with a contained " character is properly + // escaped when it is turned back into an expression. This is important when an expression is passed + // to a worker (parallel stream) or even for other reasons when an expression is string-ified. + + // Basic test + String originalExpressionString = "search(collection1,fl=\"id,first\",sort=\"first asc\",q=\"presentTitles:\\\"chief, executive officer\\\" AND age:[36 TO *]\")"; + CloudSolrStream firstStream = new CloudSolrStream(StreamExpressionParser.parse(originalExpressionString), factory); + String firstExpressionString = firstStream.toExpression(factory).toString(); + + CloudSolrStream secondStream = new CloudSolrStream(StreamExpressionParser.parse(firstExpressionString), factory); + String secondExpressionString = secondStream.toExpression(factory).toString(); + + assertTrue(firstExpressionString.contains("q=\"presentTitles:\\\"chief, executive officer\\\" AND age:[36 TO *]\"")); + assertTrue(secondExpressionString.contains("q=\"presentTitles:\\\"chief, executive officer\\\" AND age:[36 TO *]\"")); + } + @Test public void testCountMetric() throws Exception {