This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new f6872652966 SOLR-18345: ClientUtils.encodeLocalParamVal fix for \, ',
" (#4729)
f6872652966 is described below
commit f6872652966b1d8b52c376f4a08cd21f622123ab
Author: David Smiley <[email protected]>
AuthorDate: Wed Aug 19 13:22:37 2026 -0400
SOLR-18345: ClientUtils.encodeLocalParamVal fix for \, ', " (#4729)
SolrJ ClientUtils.encodeLocalParamVal() can produce lossy/invalid encodings
with a backslash or leading quotes.
Affects faceting with a custom facet response key.
Affects the SQL module for LIKE queries.
---
.../unreleased/SOLR-18345-encodeLocalParamVal.yml | 10 +++++
.../apache/solr/client/solrj/util/ClientUtils.java | 21 ++++++++--
.../solr/client/solrj/util/ClientUtilsTest.java | 48 +++++++++++++++++++++-
3 files changed, 74 insertions(+), 5 deletions(-)
diff --git a/changelog/unreleased/SOLR-18345-encodeLocalParamVal.yml
b/changelog/unreleased/SOLR-18345-encodeLocalParamVal.yml
new file mode 100644
index 00000000000..f176e84b675
--- /dev/null
+++ b/changelog/unreleased/SOLR-18345-encodeLocalParamVal.yml
@@ -0,0 +1,10 @@
+title: >
+ SolrJ ClientUtils.encodeLocalParamVal() can produce lossy/invalid encodings
with a backslash or leading quotes.
+ Affects faceting with a custom facet response key.
+ Affects the SQL module for LIKE queries.
+type: fixed
+authors:
+ - name: David Smiley
+links:
+ - name: SOLR-18345
+ url: https://issues.apache.org/jira/browse/SOLR-18345
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java
index bf06c491d03..32d8006ccca 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/util/ClientUtils.java
@@ -236,8 +236,19 @@ public class ClientUtils {
int len = val.length();
if (0 == len) return "''"; // quoted empty string
+ // Note: QueryParsing#parseLocalParams's peek() (used to check for a '='
or the closing quote
+ // char) skips leading whitespace as a side effect, so an unquoted empty
value would silently
+ // absorb the whitespace meant to separate it from the next local param,
corrupting parsing of
+ // everything that follows. Quoting sidesteps this entirely.
+
int i = 0;
- if (len > 0 && val.charAt(0) != '$') {
+ char first = val.charAt(0);
+ // A leading '$' would be read back as a param dereference, and a leading
quote char would be
+ // read back as the start of a quoted string (StrParser#getQuotedString
accepts both ' and "
+ // as delimiters); both must be quoted regardless of the rest of the value.
+ if (first == '$' || first == '\'' || first == '"') {
+ // leave i == 0 so the quoting branch below is taken
+ } else {
for (; i < len; i++) {
char ch = val.charAt(i);
if (Character.isWhitespace(ch) || ch == '}') break;
@@ -246,12 +257,16 @@ public class ClientUtils {
if (i >= len) return val;
- // We need to enclose in quotes... but now we need to escape
+ // We need to enclose in quotes... but now we need to escape. Both the
quote delimiter itself
+ // and a literal backslash must be escaped: StrParser#getQuotedString
treats any '\' as the
+ // start of an escape sequence when reading a quoted value, so an
un-escaped '\' here would be
+ // silently consumed (or worse, combined with the following char into an
unintended escape like
+ // \n) when the value is parsed back.
StringBuilder sb = new StringBuilder(val.length() + 4);
sb.append('\'');
for (i = 0; i < len; i++) {
char ch = val.charAt(i);
- if (ch == '\'') {
+ if (ch == '\'' || ch == '\\') {
sb.append('\\');
}
sb.append(ch);
diff --git
a/solr/solrj/src/test/org/apache/solr/client/solrj/util/ClientUtilsTest.java
b/solr/solrj/src/test/org/apache/solr/client/solrj/util/ClientUtilsTest.java
index 2b17ed41386..da6083b108e 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/util/ClientUtilsTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/util/ClientUtilsTest.java
@@ -16,12 +16,14 @@
*/
package org.apache.solr.client.solrj.util;
+import org.apache.lucene.tests.util.TestUtil;
import org.apache.solr.SolrTestCase;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.request.HealthCheckRequest;
import org.apache.solr.client.solrj.request.QueryRequest;
import org.apache.solr.client.solrj.request.UpdateRequest;
-import org.apache.solr.client.solrj.request.XMLRequestWriter;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.search.QueryParsing;
import org.junit.Test;
/**
@@ -37,6 +39,49 @@ public class ClientUtilsTest extends SolrTestCase {
assertEquals("h\\~\\!", ClientUtils.escapeQueryChars("h~!"));
}
+ // FYI also tested via
org.apache.solr.common.params.SolrParamTest.testLocalParamRoundTripParsing
+ public void testEncodeLocalParamValRoundTrip() throws Exception {
+ // Values that require quoting (whitespace, '}', or a leading '$') must
round-trip through
+ // Solr's own local-params reader, in particular values containing a
literal backslash or
+ // single quote.
+ assertRoundTrips("");
+ assertRoundTrips("'leadingQuote");
+ assertRoundTrips("\"leadingDoubleQuote");
+ assertRoundTrips("plain");
+ assertRoundTrips("has space");
+ assertRoundTrips("trailing}brace");
+ assertRoundTrips("has'quote and space");
+ assertRoundTrips("has\\backslash and space");
+ assertRoundTrips("both\\'kinds together");
+ assertRoundTrips("$dollarPrefixed");
+ assertRoundTrips("$dollarPrefixed with space");
+ assertRoundTrips("$\\'mix of everything");
+
+ for (int i = 0; i < 100; i++) {
+ assertRoundTrips(TestUtil.randomUnicodeString(random()));
+ }
+ }
+
+ private void assertRoundTrips(String original) throws Exception {
+ String encoded = ClientUtils.encodeLocalParamVal(original);
+ String txt = "{!key=" + encoded + "}";
+ ModifiableSolrParams target = new ModifiableSolrParams();
+ QueryParsing.parseLocalParams(txt, 0, target, null);
+ assertEquals(
+ "encodeLocalParamVal(" + original + ") -> " + encoded + " did not
round-trip",
+ original,
+ target.get("key"));
+
+ // Also confirm the encoded value doesn't swallow whatever follows it.
+ String txtFollowedByAnother = "{!key=" + encoded + " next=followed}";
+ ModifiableSolrParams targetFollowedByAnother = new ModifiableSolrParams();
+ QueryParsing.parseLocalParams(txtFollowedByAnother, 0,
targetFollowedByAnother, null);
+ assertEquals(
+ "encodeLocalParamVal(" + original + ") -> " + encoded + " swallowed
the next local param",
+ "followed",
+ targetFollowedByAnother.get("next"));
+ }
+
@Test
public void testDeterminesWhenToUseDefaultCollection() {
final var noDefaultNeededRequest = new CollectionAdminRequest.List();
@@ -55,7 +100,6 @@ public class ClientUtilsTest extends SolrTestCase {
@Test
public void testUrlBuilding() throws Exception {
- final var rw = new XMLRequestWriter();
// Simple case, non-collection request
{
final var request = new HealthCheckRequest();
