You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ds...@apache.org on 2024/05/15 12:55:47 UTC
(solr) branch branch_9x updated: SOLR-17263: HttpJdkSolrClient doesn't encode curly braces etc (#2433)
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new be9fcf3d696 SOLR-17263: HttpJdkSolrClient doesn't encode curly braces etc (#2433)
be9fcf3d696 is described below
commit be9fcf3d696b2ac484f128a58561c2cf942eaf5b
Author: Andy Webb <an...@apache.org>
AuthorDate: Wed May 15 13:51:29 2024 +0100
SOLR-17263: HttpJdkSolrClient doesn't encode curly braces etc (#2433)
(cherry picked from commit 4c439d07ca35e7e80c5406c103e885307f0b64b9)
---
solr/CHANGES.txt | 2 ++
.../solr/client/solrj/impl/HttpJdkSolrClient.java | 8 ++++--
.../client/solrj/impl/Http2SolrClientTest.java | 8 +++---
.../client/solrj/impl/HttpJdkSolrClientTest.java | 6 ++--
.../client/solrj/impl/HttpSolrClientTestBase.java | 33 ++++++++++++++--------
5 files changed, 36 insertions(+), 21 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 4333199978c..6c1d58d451b 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -37,6 +37,8 @@ Bug Fixes
* SOLR-17049: Actually mark all replicas down at startup and truly wait for them.
This includes replicas that might not exist anymore locally. (Houston Putman, Vincent Primault)
+* SOLR-17263: Fix 'Illegal character in query' exception in HttpJdkSolrClient (Andy Webb)
+
Dependency Upgrades
---------------------
(No changes)
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java
index 96366d08c0f..37ea192f548 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java
@@ -260,7 +260,7 @@ public class HttpJdkSolrClient extends HttpSolrClientBase {
validateGetRequest(solrRequest);
reqb.GET();
decorateRequest(reqb, solrRequest);
- reqb.uri(new URI(url + "?" + queryParams));
+ reqb.uri(new URI(url + queryParams.toQueryString()));
return new PreparedRequest(reqb, null);
}
@@ -324,7 +324,9 @@ public class HttpJdkSolrClient extends HttpSolrClientBase {
ModifiableSolrParams requestParams = queryParams;
queryParams = calculateQueryParams(urlParamNames, requestParams);
queryParams.add(calculateQueryParams(solrRequest.getQueryParams(), requestParams));
- bodyPublisher = HttpRequest.BodyPublishers.ofString(requestParams.toString());
+ // note the toQueryString() method adds a leading question mark which needs to be removed here
+ bodyPublisher =
+ HttpRequest.BodyPublishers.ofString(requestParams.toQueryString().substring(1));
} else {
bodyPublisher = HttpRequest.BodyPublishers.noBody();
}
@@ -335,7 +337,7 @@ public class HttpJdkSolrClient extends HttpSolrClientBase {
} else {
reqb.method("POST", bodyPublisher);
}
- URI uriWithQueryParams = new URI(url + "?" + queryParams);
+ URI uriWithQueryParams = new URI(url + queryParams.toQueryString());
reqb.uri(uriWithQueryParams);
return new PreparedRequest(reqb, contentWritingFuture);
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 fce43e5b44f..e84abb1218c 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
@@ -130,7 +130,7 @@ public class Http2SolrClientTest extends HttpSolrClientTestBase {
DebugServlet.clear();
String url = getBaseUrl() + DEBUG_SERVLET_PATH;
SolrQuery q = new SolrQuery("foo");
- q.setParam("a", "\u1234");
+ q.setParam("a", MUST_ENCODE);
Http2SolrClient.Builder b =
new Http2SolrClient.Builder(url).withDefaultCollection(DEFAULT_CORE);
if (rp != null) {
@@ -233,7 +233,7 @@ public class Http2SolrClientTest extends HttpSolrClientTestBase {
String url = getBaseUrl() + DEBUG_SERVLET_PATH;
try (Http2SolrClient client =
new Http2SolrClient.Builder(url).withDefaultCollection(DEFAULT_CORE).build()) {
- testUpdate(client, WT.JAVABIN, "application/javabin", "\u1234");
+ testUpdate(client, WT.JAVABIN, "application/javabin", MUST_ENCODE);
}
}
@@ -246,7 +246,7 @@ public class Http2SolrClientTest extends HttpSolrClientTestBase {
.withRequestWriter(new RequestWriter())
.withResponseParser(new XMLResponseParser())
.build()) {
- testUpdate(client, WT.XML, "application/xml; charset=UTF-8", "\u1234");
+ testUpdate(client, WT.XML, "application/xml; charset=UTF-8", MUST_ENCODE);
}
}
@@ -259,7 +259,7 @@ public class Http2SolrClientTest extends HttpSolrClientTestBase {
.withRequestWriter(new BinaryRequestWriter())
.withResponseParser(new BinaryResponseParser())
.build()) {
- testUpdate(client, WT.JAVABIN, "application/javabin", "\u1234");
+ testUpdate(client, WT.JAVABIN, "application/javabin", MUST_ENCODE);
}
}
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 8b9cdd4a799..12115983b9a 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
@@ -178,7 +178,7 @@ public class HttpJdkSolrClientTest extends HttpSolrClientTestBase {
}
String url = getBaseUrl() + DEBUG_SERVLET_PATH;
SolrQuery q = new SolrQuery("foo");
- q.setParam("a", "\u1234");
+ q.setParam("a", MUST_ENCODE);
HttpJdkSolrClient.Builder b = builder(url);
if (rp != null) {
b.withResponseParser(rp);
@@ -382,7 +382,7 @@ public class HttpJdkSolrClientTest extends HttpSolrClientTestBase {
public void testUpdateDefault() throws Exception {
String url = getBaseUrl() + DEBUG_SERVLET_PATH;
try (HttpJdkSolrClient client = builder(url).build()) {
- testUpdate(client, WT.JAVABIN, "application/javabin", "\u1234");
+ testUpdate(client, WT.JAVABIN, "application/javabin", MUST_ENCODE);
}
}
@@ -433,7 +433,7 @@ public class HttpJdkSolrClientTest extends HttpSolrClientTestBase {
.withRequestWriter(new BinaryRequestWriter())
.withResponseParser(new BinaryResponseParser())
.build()) {
- testUpdate(client, WT.JAVABIN, "application/javabin", "\u1234");
+ testUpdate(client, WT.JAVABIN, "application/javabin", MUST_ENCODE);
assertNoHeadRequestWithSsl(client);
}
}
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 e744673b1dd..b545a83adf0 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
@@ -21,6 +21,7 @@ import static org.hamcrest.CoreMatchers.instanceOf;
import java.io.IOException;
import java.io.InputStream;
+import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Base64;
@@ -64,6 +65,8 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
protected static final String REDIRECT_SERVLET_PATH = "/redirect";
protected static final String REDIRECT_SERVLET_REGEX = REDIRECT_SERVLET_PATH + "/*";
protected static final String COLLECTION_1 = "collection1";
+ // example chars that must be URI encoded - non-ASCII and curly quote
+ protected static final String MUST_ENCODE = "\u1234\u007B";
@BeforeClass
public static void beforeTest() throws Exception {
@@ -113,7 +116,7 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
assertNull(DebugServlet.headers.get("content-type"));
// param encoding
assertEquals(1, DebugServlet.parameters.get("a").length);
- assertEquals("\u1234", DebugServlet.parameters.get("a")[0]);
+ assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
}
public void testQueryPost() throws Exception {
@@ -125,9 +128,11 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
assertEquals("javabin", DebugServlet.parameters.get(CommonParams.WT)[0]);
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(1, DebugServlet.parameters.get("a").length);
- assertEquals("\u1234", DebugServlet.parameters.get("a")[0]);
+ assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[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"));
}
public void testQueryPut() throws Exception {
@@ -139,9 +144,10 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
assertEquals("javabin", DebugServlet.parameters.get(CommonParams.WT)[0]);
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(1, DebugServlet.parameters.get("a").length);
- assertEquals("\u1234", DebugServlet.parameters.get("a")[0]);
+ assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[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"));
}
public void testQueryXmlGet() throws Exception {
@@ -153,7 +159,7 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
assertEquals("xml", DebugServlet.parameters.get(CommonParams.WT)[0]);
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(1, DebugServlet.parameters.get("a").length);
- assertEquals("\u1234", DebugServlet.parameters.get("a")[0]);
+ assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
assertEquals(expectedUserAgent(), DebugServlet.headers.get("user-agent"));
}
@@ -166,7 +172,7 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
assertEquals("xml", DebugServlet.parameters.get(CommonParams.WT)[0]);
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(1, DebugServlet.parameters.get("a").length);
- assertEquals("\u1234", DebugServlet.parameters.get("a")[0]);
+ assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
assertEquals(expectedUserAgent(), DebugServlet.headers.get("user-agent"));
assertEquals("application/x-www-form-urlencoded", DebugServlet.headers.get("content-type"));
}
@@ -180,7 +186,7 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
assertEquals("xml", DebugServlet.parameters.get(CommonParams.WT)[0]);
assertEquals(1, DebugServlet.parameters.get(CommonParams.VERSION).length);
assertEquals(1, DebugServlet.parameters.get("a").length);
- assertEquals("\u1234", DebugServlet.parameters.get("a")[0]);
+ assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
assertEquals(expectedUserAgent(), DebugServlet.headers.get("user-agent"));
assertEquals("application/x-www-form-urlencoded", DebugServlet.headers.get("content-type"));
}
@@ -284,7 +290,8 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
SolrInputDocument doc = new SolrInputDocument();
doc.addField("id", docIdValue);
req.add(doc);
- req.setParam("a", "\u1234");
+ // non-ASCII characters and curly quotes should be URI-encoded
+ req.setParam("a", MUST_ENCODE);
try {
client.request(req);
@@ -301,7 +308,7 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
client.getParser().getVersion(), DebugServlet.parameters.get(CommonParams.VERSION)[0]);
assertEquals(contentType, DebugServlet.headers.get("content-type"));
assertEquals(1, DebugServlet.parameters.get("a").length);
- assertEquals("\u1234", DebugServlet.parameters.get("a")[0]);
+ assertEquals(MUST_ENCODE, DebugServlet.parameters.get("a")[0]);
if (wt == WT.XML) {
String requestBody = new String(DebugServlet.requestBody, StandardCharsets.UTF_8);
@@ -338,12 +345,14 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
protected void setReqParamsOf(UpdateRequest req, String... keys) {
if (keys != null) {
for (String k : keys) {
- req.setParam(k, k + "Value");
+ // note inclusion of non-ASCII character, and curly quotes which should be URI encoded
+ req.setParam(k, k + "Value" + MUST_ENCODE);
}
}
}
- protected void verifyServletState(HttpSolrClientBase client, SolrRequest<?> request) {
+ protected void verifyServletState(HttpSolrClientBase client, SolrRequest<?> request)
+ throws Exception {
// check query String
Iterator<String> paramNames = request.getParams().getParameterNamesIterator();
while (paramNames.hasNext()) {
@@ -355,7 +364,9 @@ public abstract class HttpSolrClientTestBase extends SolrJettyTestBase {
client.getUrlParamNames().contains(name)
|| (request.getQueryParams() != null && request.getQueryParams().contains(name));
assertEquals(
- shouldBeInQueryString, DebugServlet.queryString.contains(name + "=" + value));
+ shouldBeInQueryString,
+ DebugServlet.queryString.contains(
+ name + "=" + URLEncoder.encode(value, StandardCharsets.UTF_8.name())));
// in either case, it should be in the parameters
assertNotNull(DebugServlet.parameters.get(name));
assertEquals(1, DebugServlet.parameters.get(name).length);