diff --git a/solr/core/src/java/org/apache/solr/jersey/PostRequestLoggingFilter.java b/solr/core/src/java/org/apache/solr/jersey/PostRequestLoggingFilter.java index 0840618bbe31..16641491ec19 100644 --- a/solr/core/src/java/org/apache/solr/jersey/PostRequestLoggingFilter.java +++ b/solr/core/src/java/org/apache/solr/jersey/PostRequestLoggingFilter.java @@ -37,6 +37,7 @@ import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.StringJoiner; import java.util.stream.Collectors; import org.apache.solr.client.api.model.SolrJerseyResponse; import org.apache.solr.common.util.CollectionUtil; @@ -164,7 +165,7 @@ public static String buildRequestBodyString(ContainerRequestContext requestConte public static String filterAndStringifyQueryParameters( MultivaluedMap unfilteredParams) { final var paramNamesToLog = getParamNamesToLog(unfilteredParams); - final StringBuilder sb = new StringBuilder(128); + var output = new StringJoiner("&"); unfilteredParams.entrySet().stream() .sorted(Map.Entry.comparingByKey()) .forEachOrdered( @@ -173,13 +174,11 @@ public static String filterAndStringifyQueryParameters( if (!paramNamesToLog.contains(name)) return; for (String val : entry.getValue()) { - if (sb.length() != 0) sb.append('&'); - StrUtils.partialURLEncodeVal(sb, name); - sb.append('='); - StrUtils.partialURLEncodeVal(sb, val); + output.add( + StrUtils.partialURLEncodeVal(name) + "=" + StrUtils.partialURLEncodeVal(val)); } }); - return sb.toString(); + return output.toString(); } private static Set getParamNamesToLog(MultivaluedMap queryParameters) { diff --git a/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java b/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java index e06cc4cd7cb3..1a77abe9bf65 100644 --- a/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java +++ b/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java @@ -25,6 +25,7 @@ import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; +import java.util.StringJoiner; import java.util.stream.Stream; import java.util.stream.StreamSupport; import org.apache.solr.client.solrj.util.ClientUtils; @@ -459,19 +460,16 @@ public NamedList toNamedList() { */ public String toQueryString() { final Charset charset = StandardCharsets.UTF_8; - final StringBuilder sb = new StringBuilder(128); - boolean first = true; + + var output = new StringJoiner("&", "?", ""); + output.setEmptyValue(""); for (final Iterator it = getParameterNamesIterator(); it.hasNext(); ) { - final String name = it.next(), nameEnc = URLEncoder.encode(name, charset); + final String name = it.next(); for (String val : getParams(name)) { - sb.append(first ? '?' : '&') - .append(nameEnc) - .append('=') - .append(URLEncoder.encode(val, charset)); - first = false; + output.add(URLEncoder.encode(name, charset) + "=" + URLEncoder.encode(val, charset)); } } - return sb.toString(); + return output.toString(); } /** @@ -510,19 +508,14 @@ public String toLocalParamsString() { */ @Override public String toString() { - final StringBuilder sb = new StringBuilder(128); - boolean first = true; + StringJoiner query = new StringJoiner("&"); for (final Iterator it = getParameterNamesIterator(); it.hasNext(); ) { final String name = it.next(); for (String val : getParams(name)) { - if (!first) sb.append('&'); - first = false; - StrUtils.partialURLEncodeVal(sb, name); - sb.append('='); - StrUtils.partialURLEncodeVal(sb, val); + query.add(StrUtils.partialURLEncodeVal(name) + "=" + StrUtils.partialURLEncodeVal(val)); } } - return sb.toString(); + return query.toString(); } /** diff --git a/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java b/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java index f2aaea286718..95e975027524 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java @@ -302,36 +302,38 @@ public static boolean parseBool(String s, boolean def) { * *

Characters with a numeric value less than 32 are encoded. &,=,%,+,space are encoded. */ - public static void partialURLEncodeVal(StringBuilder dest, String val) { + public static String partialURLEncodeVal(String val) { + var output = new StringBuilder(); for (int i = 0; i < val.length(); i++) { char ch = val.charAt(i); if (ch < 32) { - dest.append('%'); - if (ch < 0x10) dest.append('0'); - dest.append(Integer.toHexString(ch)); + output.append('%'); + if (ch < 0x10) output.append('0'); + output.append(Integer.toHexString(ch)); } else { switch (ch) { case ' ': - dest.append('+'); + output.append('+'); break; case '&': - dest.append("%26"); + output.append("%26"); break; case '%': - dest.append("%25"); + output.append("%25"); break; case '=': - dest.append("%3D"); + output.append("%3D"); break; case '+': - dest.append("%2B"); + output.append("%2B"); break; default: - dest.append(ch); + output.append(ch); break; } } } + return output.toString(); } /** diff --git a/solr/solrj/src/test/org/apache/solr/common/params/SolrParamTest.java b/solr/solrj/src/test/org/apache/solr/common/params/SolrParamTest.java index 0b60871deffd..67f26587e7e8 100644 --- a/solr/solrj/src/test/org/apache/solr/common/params/SolrParamTest.java +++ b/solr/solrj/src/test/org/apache/solr/common/params/SolrParamTest.java @@ -27,6 +27,8 @@ import org.apache.solr.SolrTestCase; import org.apache.solr.common.SolrException; import org.apache.solr.search.QueryParsing; +import org.apache.solr.servlet.SolrRequestParsers; +import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -352,6 +354,44 @@ public void testGetParams() { assertNull(defaults.get("asagdsaga")); } + @Test + public void testFirstParamHasQuestionMark() { + + final ModifiableSolrParams in = getDummySolrParams(); + String queryString = in.toQueryString(); + assertEquals('?', queryString.charAt(0)); + + MultiMapSolrParams out = SolrRequestParsers.parseQueryString(queryString.substring(1)); + assertEquals(in, out); + } + + @Test + public void testIfToStringCanBeParsed() { + + final ModifiableSolrParams in = getDummySolrParams(); + String queryString = in.toString(); + + assertEquals("first=1st&second=2nd&third=3rd", queryString); + MultiMapSolrParams out = SolrRequestParsers.parseQueryString(queryString); + assertEquals(in, out); + } + + public void testToStringWithEmptySolrParams() { + + ModifiableSolrParams in = new ModifiableSolrParams(); + String queryString = in.toString(); + assertEquals("", queryString); + MultiMapSolrParams out = SolrRequestParsers.parseQueryString(queryString); + assertEquals(in, out); + } + + private ModifiableSolrParams getDummySolrParams() { + return params( + "first", "1st", + "second", "2nd", + "third", "3rd"); + } + public static int getReturnCode(Runnable runnable) { try { runnable.run(); diff --git a/solr/test-framework/src/java/org/apache/solr/util/RestTestBase.java b/solr/test-framework/src/java/org/apache/solr/util/RestTestBase.java index 83b2d302c0c5..830cf0540ff7 100644 --- a/solr/test-framework/src/java/org/apache/solr/util/RestTestBase.java +++ b/solr/test-framework/src/java/org/apache/solr/util/RestTestBase.java @@ -459,7 +459,7 @@ private static String setParam(String query, String paramToSet, String valueToSe // empty query -> return "paramToSet=valueToSet" builder.append(paramToSet); builder.append('='); - StrUtils.partialURLEncodeVal(builder, valueToSet); + builder.append(StrUtils.partialURLEncodeVal(valueToSet)); return builder.toString(); } MultiMapSolrParams requestParams = SolrRequestParsers.parseQueryString(query); @@ -470,7 +470,7 @@ private static String setParam(String query, String paramToSet, String valueToSe builder.append('&'); builder.append(paramToSet); builder.append('='); - StrUtils.partialURLEncodeVal(builder, valueToSet); + builder.append(StrUtils.partialURLEncodeVal(valueToSet)); return builder.toString(); } if (1 == values.length && valueToSet.equals(values[0])) { @@ -490,14 +490,14 @@ private static String setParam(String query, String paramToSet, String valueToSe isFirst = false; builder.append(key); builder.append('='); - StrUtils.partialURLEncodeVal(builder, null == val ? "" : val); + builder.append(StrUtils.partialURLEncodeVal(null == val ? "" : val)); } } } builder.append(isFirst ? "" : '&'); builder.append(paramToSet); builder.append('='); - StrUtils.partialURLEncodeVal(builder, valueToSet); + builder.append(StrUtils.partialURLEncodeVal(valueToSet)); return builder.toString(); } }