This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_9_8 in repository https://gitbox.apache.org/repos/asf/solr.git
commit 11075f7259b00da0614eb3236a33f712303395c7 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]> (cherry picked from commit 82083ea13ad2b2114d4311439be2f90ed868fac1) --- 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 8989430fb30..de389c38463 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -18,6 +18,10 @@ Bug Fixes * SOLR-17649: Fix a regression of faceting on multi-valued EnumFieldType, introduced by an earlier 9.x version. (Thomas Wöckinger, 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 --------------------- (No changes) 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 cf8415ee64c..2f4ea6509a1 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 @@ -75,7 +75,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; @@ -90,7 +89,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; @@ -817,18 +815,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 0748b133d6a..15cfc0432e1 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 07a202ccc65..701a8a0dcfc 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 @@ -179,6 +179,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 919af4ef266..5b9a352623e 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);
