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 82083ea13ad SOLR-17221: Fix Http2SolrClient merging case sensitive
solr params (#3028)
82083ea13ad is described below
commit 82083ea13ad2b2114d4311439be2f90ed868fac1
Author: Yue Yu <[email protected]>
AuthorDate: Thu Jan 16 00:55:21 2025 -0600
SOLR-17221: Fix Http2SolrClient merging case sensitive solr params (#3028)
If multiple query param keys are sent that only vary by case, they were
wrongly merged when doing distributed search (sharded collections). This could
likely occur for fielded parameters such as
f.CASE_SENSITIVE_FIELD.facet.limit=50
Also: compose the request content directly using
SolrParams.toQueryString(). This will avoid to use Jetty Fields as an
intermediary mapping
---------
Co-authored-by: David Smiley <[email protected]>
---
solr/CHANGES.txt | 4 +++
.../solr/client/solrj/impl/Http2SolrClient.java | 20 ++++--------
.../client/solrj/impl/Http2SolrClientTest.java | 2 ++
.../client/solrj/impl/HttpJdkSolrClientTest.java | 2 ++
.../client/solrj/impl/HttpSolrClientTestBase.java | 36 ++++++++++++++++++++--
5 files changed, 48 insertions(+), 16 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 4c5bf3b5faa..373cd50b9f4 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -169,6 +169,10 @@ Bug Fixes
* SOLR-17587: Metrics: wt=prometheus fix for non-compliant exposition format
containing duplicate TYPE lines.
(Matthew Biscocho via David Smiley)
+* SOLR-17221: If multiple query param keys are sent that only vary by case,
they were wrongly merged when
+ doing distributed search (sharded collections). This could likely occur for
fielded parameters such as
+ f.CASE_SENSITIVE_FIELD.facet.limit=50. (Yue Yu, David Smiley)
+
Dependency Upgrades
---------------------
* SOLR-17471: Upgrade Lucene to 9.12.1. (Pierre Salagnac, Christine Poerschke)
diff --git
a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
index fb2eb1a123f..14fe9d7f4cf 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/Http2SolrClient.java
@@ -71,7 +71,6 @@ import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.api.Result;
import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP;
-import org.eclipse.jetty.client.util.FormRequestContent;
import org.eclipse.jetty.client.util.InputStreamRequestContent;
import org.eclipse.jetty.client.util.InputStreamResponseListener;
import org.eclipse.jetty.client.util.MultiPartRequestContent;
@@ -86,7 +85,6 @@ import org.eclipse.jetty.http2.client.HTTP2Client;
import org.eclipse.jetty.http2.client.http.HttpClientTransportOverHTTP2;
import org.eclipse.jetty.io.ClientConnector;
import org.eclipse.jetty.util.BlockingArrayQueue;
-import org.eclipse.jetty.util.Fields;
import org.eclipse.jetty.util.HttpCookieStore;
import org.eclipse.jetty.util.ssl.KeyStoreScanner;
import org.eclipse.jetty.util.ssl.SslContextFactory;
@@ -784,18 +782,12 @@ public class Http2SolrClient extends HttpSolrClientBase {
}
} else {
// application/x-www-form-urlencoded
- Fields fields = new Fields();
- Iterator<String> iter = wparams.getParameterNamesIterator();
- while (iter.hasNext()) {
- String key = iter.next();
- String[] vals = wparams.getParams(key);
- if (vals != null) {
- for (String val : vals) {
- fields.add(key, val);
- }
- }
- }
- req.body(new FormRequestContent(fields, FALLBACK_CHARSET));
+ String queryString = wparams.toQueryString();
+ // remove the leading "?" if there is any
+ queryString = queryString.startsWith("?") ? queryString.substring(1) :
queryString;
+ req.body(
+ new StringRequestContent(
+ "application/x-www-form-urlencoded", queryString,
FALLBACK_CHARSET));
}
return req;
diff --git
a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java
index 5927811906c..6bf74daa0e6 100644
---
a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java
+++
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientTest.java
@@ -147,6 +147,8 @@ public class Http2SolrClientTest extends
HttpSolrClientTestBase {
String url = getBaseUrl() + DEBUG_SERVLET_PATH;
SolrQuery q = new SolrQuery("foo");
q.setParam("a", MUST_ENCODE);
+ q.setParam("case_sensitive_param", "lowercase");
+ q.setParam("CASE_SENSITIVE_PARAM", "uppercase");
Http2SolrClient.Builder b =
new Http2SolrClient.Builder(url).withDefaultCollection(DEFAULT_CORE);
if (rp != null) {
diff --git
a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
index f0880e1ae7c..9161cb6bde5 100644
---
a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
+++
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java
@@ -143,6 +143,8 @@ public class HttpJdkSolrClientTest extends
HttpSolrClientTestBase {
String url = getBaseUrl() + DEBUG_SERVLET_PATH;
SolrQuery q = new SolrQuery("foo");
q.setParam("a", MUST_ENCODE);
+ q.setParam("case_sensitive_param", "lowercase");
+ q.setParam("CASE_SENSITIVE_PARAM", "uppercase");
HttpJdkSolrClient.Builder b = builder(url);
if (rp != null) {
b.withResponseParser(rp);
diff --git
a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java
index 1c8525b74b6..17ce59e1cc4 100644
---
a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java
+++
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpSolrClientTestBase.java
@@ -116,6 +116,11 @@ public abstract class HttpSolrClientTestBase extends
SolrJettyTestBase {
// param encoding
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
+ // case sensitive param
+ assertEquals(1,
DebugServlet.parameters.get("case_sensitive_param").length);
+ assertEquals("lowercase",
DebugServlet.parameters.get("case_sensitive_param")[0]);
+ assertEquals(1,
DebugServlet.parameters.get("CASE_SENSITIVE_PARAM").length);
+ assertEquals("uppercase",
DebugServlet.parameters.get("CASE_SENSITIVE_PARAM")[0]);
}
public void testQueryPost() throws Exception {
@@ -128,10 +133,14 @@ public abstract class HttpSolrClientTestBase extends
SolrJettyTestBase {
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
+ assertEquals(1,
DebugServlet.parameters.get("case_sensitive_param").length);
+ assertEquals("lowercase",
DebugServlet.parameters.get("case_sensitive_param")[0]);
+ assertEquals(1,
DebugServlet.parameters.get("CASE_SENSITIVE_PARAM").length);
+ assertEquals("uppercase",
DebugServlet.parameters.get("CASE_SENSITIVE_PARAM")[0]);
assertEquals(expectedUserAgent(), DebugServlet.headers.get("user-agent"));
assertEquals("application/x-www-form-urlencoded",
DebugServlet.headers.get("content-type"));
// this validates that URI encoding has been applied - the content-length
is smaller if not
- assertEquals("41", DebugServlet.headers.get("content-length"));
+ assertEquals("103", DebugServlet.headers.get("content-length"));
}
public void testQueryPut() throws Exception {
@@ -144,9 +153,13 @@ public abstract class HttpSolrClientTestBase extends
SolrJettyTestBase {
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
+ assertEquals(1,
DebugServlet.parameters.get("case_sensitive_param").length);
+ assertEquals("lowercase",
DebugServlet.parameters.get("case_sensitive_param")[0]);
+ assertEquals(1,
DebugServlet.parameters.get("CASE_SENSITIVE_PARAM").length);
+ assertEquals("uppercase",
DebugServlet.parameters.get("CASE_SENSITIVE_PARAM")[0]);
assertEquals(expectedUserAgent(), DebugServlet.headers.get("user-agent"));
assertEquals("application/x-www-form-urlencoded",
DebugServlet.headers.get("content-type"));
- assertEquals("41", DebugServlet.headers.get("content-length"));
+ assertEquals("103", DebugServlet.headers.get("content-length"));
}
public void testQueryXmlGet() throws Exception {
@@ -159,6 +172,10 @@ public abstract class HttpSolrClientTestBase extends
SolrJettyTestBase {
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
+ assertEquals(1,
DebugServlet.parameters.get("case_sensitive_param").length);
+ assertEquals("lowercase",
DebugServlet.parameters.get("case_sensitive_param")[0]);
+ assertEquals(1,
DebugServlet.parameters.get("CASE_SENSITIVE_PARAM").length);
+ assertEquals("uppercase",
DebugServlet.parameters.get("CASE_SENSITIVE_PARAM")[0]);
assertEquals(expectedUserAgent(), DebugServlet.headers.get("user-agent"));
}
@@ -172,6 +189,10 @@ public abstract class HttpSolrClientTestBase extends
SolrJettyTestBase {
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
+ assertEquals(1,
DebugServlet.parameters.get("case_sensitive_param").length);
+ assertEquals("lowercase",
DebugServlet.parameters.get("case_sensitive_param")[0]);
+ assertEquals(1,
DebugServlet.parameters.get("CASE_SENSITIVE_PARAM").length);
+ assertEquals("uppercase",
DebugServlet.parameters.get("CASE_SENSITIVE_PARAM")[0]);
assertEquals(expectedUserAgent(), DebugServlet.headers.get("user-agent"));
assertEquals("application/x-www-form-urlencoded",
DebugServlet.headers.get("content-type"));
}
@@ -186,6 +207,10 @@ public abstract class HttpSolrClientTestBase extends
SolrJettyTestBase {
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
+ assertEquals(1,
DebugServlet.parameters.get("case_sensitive_param").length);
+ assertEquals("lowercase",
DebugServlet.parameters.get("case_sensitive_param")[0]);
+ assertEquals(1,
DebugServlet.parameters.get("CASE_SENSITIVE_PARAM").length);
+ assertEquals("uppercase",
DebugServlet.parameters.get("CASE_SENSITIVE_PARAM")[0]);
assertEquals(expectedUserAgent(), DebugServlet.headers.get("user-agent"));
assertEquals("application/x-www-form-urlencoded",
DebugServlet.headers.get("content-type"));
}
@@ -264,6 +289,9 @@ public abstract class HttpSolrClientTestBase extends
SolrJettyTestBase {
req.add(doc);
// non-ASCII characters and curly quotes should be URI-encoded
req.setParam("a", MUST_ENCODE);
+ // params should be case sensitive
+ req.setParam("case_sensitive_param", "lowercase");
+ req.setParam("CASE_SENSITIVE_PARAM", "uppercase");
try {
client.request(req);
@@ -281,6 +309,10 @@ public abstract class HttpSolrClientTestBase extends
SolrJettyTestBase {
assertEquals(contentType, DebugServlet.headers.get("content-type"));
assertEquals(1, DebugServlet.parameters.get("a").length);
assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
+ assertEquals(1,
DebugServlet.parameters.get("case_sensitive_param").length);
+ assertEquals("lowercase",
DebugServlet.parameters.get("case_sensitive_param")[0]);
+ assertEquals(1,
DebugServlet.parameters.get("CASE_SENSITIVE_PARAM").length);
+ assertEquals("uppercase",
DebugServlet.parameters.get("CASE_SENSITIVE_PARAM")[0]);
if (wt == WT.XML) {
String requestBody = new String(DebugServlet.requestBody,
StandardCharsets.UTF_8);
