This is an automated email from the ASF dual-hosted git repository.
gerlowskija 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 f9c50fa101e Replace HTTP-method string-handling with enum
f9c50fa101e is described below
commit f9c50fa101e3ff92d8529fb19a6cb44bfed199d7
Author: Jason Gerlowski <[email protected]>
AuthorDate: Tue Jan 6 06:19:38 2026 -0500
Replace HTTP-method string-handling with enum
A number of places in Solr referenced the HTTP method used to make the
request, often inspecting the value as a string. This is needlessly
brittle (e.g. case sensitivity issues).
This commit fixes this problem by parsing the method value into an enum,
which can then be used by later inspection.
---
.../client/solrj/embedded/EmbeddedSolrServer.java | 4 +-
.../org/apache/solr/handler/SchemaHandler.java | 46 +++++++++++++---------
.../org/apache/solr/handler/SolrConfigHandler.java | 40 +++++++++++--------
.../solr/handler/admin/SecurityConfHandler.java | 7 ++--
.../org/apache/solr/request/SolrQueryRequest.java | 3 +-
.../apache/solr/servlet/SolrRequestParsers.java | 7 +++-
.../handler/admin/SecurityConfHandlerTest.java | 17 ++++----
.../apache/solr/servlet/SolrRequestParserTest.java | 22 +++++++++++
.../handler/MirroringConfigSetsHandlerTest.java | 2 +-
.../org/apache/solr/client/solrj/SolrRequest.java | 18 ++++++++-
10 files changed, 113 insertions(+), 53 deletions(-)
diff --git
a/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java
b/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java
index 45aaea07d1d..82edafe2b7b 100644
---
a/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java
+++
b/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java
@@ -171,7 +171,7 @@ public class EmbeddedSolrServer extends SolrClient {
SolrQueryRequest req =
_parser.buildRequestFrom(
null, getParams(request), getContentStreams(request),
request.getUserPrincipal());
- req.getContext().put("httpMethod", request.getMethod().name());
+ req.getContext().put("httpMethod", request.getMethod());
req.getContext().put(PATH, path);
SolrQueryResponse resp = new SolrQueryResponse();
handler.handleRequest(req, resp);
@@ -224,7 +224,7 @@ public class EmbeddedSolrServer extends SolrClient {
.buildRequestFrom(
core, params, getContentStreams(request),
request.getUserPrincipal());
req.getContext().put(PATH, path);
- req.getContext().put("httpMethod", request.getMethod().name());
+ req.getContext().put("httpMethod", request.getMethod());
SolrQueryResponse rsp = new SolrQueryResponse();
SolrRequestInfo.setRequestInfo(new SolrRequestInfo(req, rsp));
diff --git a/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
b/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
index c81fa8eb572..86a5c8b2323 100644
--- a/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
@@ -36,6 +36,7 @@ import org.apache.solr.api.AnnotatedApi;
import org.apache.solr.api.Api;
import org.apache.solr.api.ApiBag;
import org.apache.solr.api.JerseyResource;
+import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.MapSolrParams;
import org.apache.solr.common.params.SolrParams;
@@ -77,26 +78,33 @@ public class SchemaHandler extends RequestHandlerBase
@Override
public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp)
throws Exception {
RequestHandlerUtils.setWt(req, JSON);
- String httpMethod = (String) req.getContext().get("httpMethod");
- if ("POST".equals(httpMethod)) {
- if (isImmutableConfigSet) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
"ConfigSet is immutable");
- }
- if (req.getContentStreams() == null) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "no
stream");
- }
-
- try {
- List<Map<String, Object>> errs = new
SchemaManager(req).performOperations();
- if (!errs.isEmpty())
- throw new ApiBag.ExceptionWithErrObject(
- SolrException.ErrorCode.BAD_REQUEST, "error processing
commands", errs);
- } catch (IOException e) {
+ final SolrRequest.METHOD httpMethod = (SolrRequest.METHOD)
req.getContext().get("httpMethod");
+ switch (httpMethod) {
+ case POST:
+ if (isImmutableConfigSet) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
"ConfigSet is immutable");
+ }
+ if (req.getContentStreams() == null) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "no
stream");
+ }
+ try {
+ List<Map<String, Object>> errs = new
SchemaManager(req).performOperations();
+ if (!errs.isEmpty())
+ throw new ApiBag.ExceptionWithErrObject(
+ SolrException.ErrorCode.BAD_REQUEST, "error processing
commands", errs);
+ } catch (IOException e) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Error reading input String " + e.getMessage(),
+ e);
+ }
+ break;
+ case GET:
+ handleGET(req, rsp);
+ break;
+ default:
throw new SolrException(
- SolrException.ErrorCode.BAD_REQUEST, "Error reading input String "
+ e.getMessage(), e);
- }
- } else {
- handleGET(req, rsp);
+ SolrException.ErrorCode.BAD_REQUEST, "Unexpected HTTP method: " +
httpMethod);
}
}
diff --git a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
index 2e82dd375f9..a9cb278b2f0 100644
--- a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
@@ -131,24 +131,30 @@ public class SolrConfigHandler extends RequestHandlerBase
public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp)
throws Exception {
RequestHandlerUtils.setWt(req, CommonParams.JSON);
- String httpMethod = (String) req.getContext().get("httpMethod");
- Command command = new Command(req, rsp, httpMethod);
- if ("POST".equals(httpMethod)) {
- if (configEditing_disabled || isImmutableConfigSet) {
- final String reason =
- configEditing_disabled
- ? "due to " + CONFIGSET_EDITING_DISABLED_ARG
- : "because ConfigSet is immutable";
+ final var httpMethod = (SolrRequest.METHOD)
req.getContext().get("httpMethod");
+ Command command = new Command(req, rsp, httpMethod.name());
+ switch (httpMethod) {
+ case POST:
+ if (configEditing_disabled || isImmutableConfigSet) {
+ final String reason =
+ configEditing_disabled
+ ? "due to " + CONFIGSET_EDITING_DISABLED_ARG
+ : "because ConfigSet is immutable";
+ throw new SolrException(
+ SolrException.ErrorCode.FORBIDDEN, " solrconfig editing is not
enabled " + reason);
+ }
+ try {
+ command.handlePOST();
+ } finally {
+ RequestHandlerUtils.addExperimentalFormatWarning(rsp);
+ }
+ break;
+ case GET:
+ command.handleGET();
+ break;
+ default:
throw new SolrException(
- SolrException.ErrorCode.FORBIDDEN, " solrconfig editing is not
enabled " + reason);
- }
- try {
- command.handlePOST();
- } finally {
- RequestHandlerUtils.addExperimentalFormatWarning(rsp);
- }
- } else {
- command.handleGET();
+ SolrException.ErrorCode.BAD_REQUEST, "Unexpected HTTP method: " +
httpMethod);
}
}
diff --git
a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandler.java
b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandler.java
index 95e1d10f351..f5b528f3d70 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandler.java
@@ -33,6 +33,7 @@ import org.apache.solr.api.AnnotatedApi;
import org.apache.solr.api.Api;
import org.apache.solr.api.ApiBag;
import org.apache.solr.api.ApiBag.ReqHandlerToApi;
+import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SpecProvider;
import org.apache.solr.common.params.CommonParams;
@@ -81,12 +82,12 @@ public abstract class SecurityConfHandler extends
RequestHandlerBase
@Override
public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp)
throws Exception {
RequestHandlerUtils.setWt(req, CommonParams.JSON);
- String httpMethod = (String) req.getContext().get("httpMethod");
+ final var httpMethod = (SolrRequest.METHOD)
req.getContext().get("httpMethod");
String path = (String) req.getContext().get("path");
String key = path.substring(path.lastIndexOf('/') + 1);
- if ("GET".equals(httpMethod)) {
+ if (SolrRequest.METHOD.GET.equals(httpMethod)) {
getConf(rsp, key);
- } else if ("POST".equals(httpMethod)) {
+ } else if (SolrRequest.METHOD.POST.equals(httpMethod)) {
Object plugin = getPlugin(key);
doEdit(req, rsp, path, key, plugin);
}
diff --git a/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java
b/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java
index bf571619e3e..5c139751f7b 100644
--- a/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java
+++ b/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java
@@ -24,6 +24,7 @@ import java.security.Principal;
import java.util.Collections;
import java.util.List;
import java.util.Map;
+import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.cloud.CloudDescriptor;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.SolrParams;
@@ -165,7 +166,7 @@ public interface SolrQueryRequest extends AutoCloseable {
}
default String getHttpMethod() {
- return (String) getContext().get("httpMethod");
+ return ((SolrRequest.METHOD) getContext().get("httpMethod")).name();
}
default HttpSolrCall getHttpSolrCall() {
diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
b/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
index 83dcb65c9b2..dfc7e056585 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
@@ -45,6 +45,7 @@ import javax.servlet.http.Part;
import org.apache.commons.io.input.CloseShieldInputStream;
import org.apache.lucene.util.IOUtils;
import org.apache.solr.api.V2HttpCall;
+import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.params.CommonParams;
@@ -173,7 +174,11 @@ public class SolrRequestParsers {
// Handlers and login will want to know the path. If it contains a ':'
// the handler could use it for RESTful URLs
sreq.getContext().put(PATH, RequestHandlers.normalize(path));
- sreq.getContext().put("httpMethod", req.getMethod());
+
+ final var methodStr = req.getMethod();
+ final var parsedMethod =
+ SolrRequest.METHOD.fromString(methodStr); // Throws error if method
not recognized
+ sreq.getContext().put("httpMethod", parsedMethod);
if (addHttpRequestToContext) {
sreq.getContext().put("httpRequest", req);
diff --git
a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java
b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java
index c9335b2a7b6..5a971eb7dd6 100644
---
a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java
+++
b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java
@@ -24,6 +24,7 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.util.CommandOperation;
import org.apache.solr.common.util.ContentStreamBase;
@@ -44,7 +45,7 @@ public class SecurityConfHandlerTest extends SolrTestCaseJ4 {
+ "'set-user':{ 'tom':'TomIsUberCool'}\n"
+ "}";
LocalSolrQueryRequest req = new LocalSolrQueryRequest(null, new
ModifiableSolrParams());
- req.getContext().put("httpMethod", "POST");
+ req.getContext().put("httpMethod", SolrRequest.METHOD.POST);
req.getContext().put("path", "/admin/authentication");
ContentStreamBase.ByteArrayStream o =
new
ContentStreamBase.ByteArrayStream(command.getBytes(StandardCharsets.UTF_8), "");
@@ -77,7 +78,7 @@ public class SecurityConfHandlerTest extends SolrTestCaseJ4 {
+ "}";
req = new LocalSolrQueryRequest(null, new ModifiableSolrParams());
- req.getContext().put("httpMethod", "POST");
+ req.getContext().put("httpMethod", SolrRequest.METHOD.POST);
req.getContext().put("path", "/admin/authorization");
o = new
ContentStreamBase.ByteArrayStream(command.getBytes(StandardCharsets.UTF_8), "");
req.setContentStreams(Collections.singletonList(o));
@@ -101,7 +102,7 @@ public class SecurityConfHandlerTest extends SolrTestCaseJ4
{
+ " 'role': ['admin','dev']\n"
+ " }}";
req = new LocalSolrQueryRequest(null, new ModifiableSolrParams());
- req.getContext().put("httpMethod", "POST");
+ req.getContext().put("httpMethod", SolrRequest.METHOD.POST);
req.getContext().put("path", "/admin/authorization");
o = new
ContentStreamBase.ByteArrayStream(command.getBytes(StandardCharsets.UTF_8), "");
req.setContentStreams(Collections.singletonList(o));
@@ -122,7 +123,7 @@ public class SecurityConfHandlerTest extends SolrTestCaseJ4
{
+ " 'role': ['guest','admin']\n"
+ " }}";
req = new LocalSolrQueryRequest(null, new ModifiableSolrParams());
- req.getContext().put("httpMethod", "POST");
+ req.getContext().put("httpMethod", SolrRequest.METHOD.POST);
req.getContext().put("path", "/admin/authorization");
o = new
ContentStreamBase.ByteArrayStream(command.getBytes(StandardCharsets.UTF_8), "");
req.setContentStreams(Collections.singletonList(o));
@@ -139,7 +140,7 @@ public class SecurityConfHandlerTest extends SolrTestCaseJ4
{
command = "{\n" + "delete-permission: 1,\n" + " set-user-role : { tom
:null}\n" + "}";
req = new LocalSolrQueryRequest(null, new ModifiableSolrParams());
- req.getContext().put("httpMethod", "POST");
+ req.getContext().put("httpMethod", SolrRequest.METHOD.POST);
req.getContext().put("path", "/admin/authorization");
o = new
ContentStreamBase.ByteArrayStream(command.getBytes(StandardCharsets.UTF_8), "");
req.setContentStreams(Collections.singletonList(o));
@@ -163,7 +164,7 @@ public class SecurityConfHandlerTest extends SolrTestCaseJ4
{
+ " 'role': 'admin'\n"
+ " }}";
req = new LocalSolrQueryRequest(null, new ModifiableSolrParams());
- req.getContext().put("httpMethod", "POST");
+ req.getContext().put("httpMethod", SolrRequest.METHOD.POST);
req.getContext().put("path", "/admin/authorization");
o = new
ContentStreamBase.ByteArrayStream(command.getBytes(StandardCharsets.UTF_8), "");
req.setContentStreams(Collections.singletonList(o));
@@ -237,7 +238,7 @@ public class SecurityConfHandlerTest extends SolrTestCaseJ4
{
public String getStandardJson() throws Exception {
String command = "{\n" + "'set-user': {'solr':'SolrRocks'}\n" + "}";
LocalSolrQueryRequest req = new LocalSolrQueryRequest(null, new
ModifiableSolrParams());
- req.getContext().put("httpMethod", "POST");
+ req.getContext().put("httpMethod", SolrRequest.METHOD.POST);
req.getContext().put("path", "/admin/authentication");
ContentStreamBase.ByteArrayStream o =
new
ContentStreamBase.ByteArrayStream(command.getBytes(StandardCharsets.UTF_8), "");
@@ -249,7 +250,7 @@ public class SecurityConfHandlerTest extends SolrTestCaseJ4
{
+ "'set-permission':{'name': 'security-edit', 'role': 'admin'}"
+ "}";
req = new LocalSolrQueryRequest(null, new ModifiableSolrParams());
- req.getContext().put("httpMethod", "POST");
+ req.getContext().put("httpMethod", SolrRequest.METHOD.POST);
req.getContext().put("path", "/admin/authorization");
o = new
ContentStreamBase.ByteArrayStream(command.getBytes(StandardCharsets.UTF_8), "");
req.setContentStreams(Collections.singletonList(o));
diff --git
a/solr/core/src/test/org/apache/solr/servlet/SolrRequestParserTest.java
b/solr/core/src/test/org/apache/solr/servlet/SolrRequestParserTest.java
index e86e6c12c4b..2c7c947e2ca 100644
--- a/solr/core/src/test/org/apache/solr/servlet/SolrRequestParserTest.java
+++ b/solr/core/src/test/org/apache/solr/servlet/SolrRequestParserTest.java
@@ -263,6 +263,28 @@ public class SolrRequestParserTest extends SolrTestCaseJ4 {
}
}
+ @Test
+ public void testReportsErrorForUnexpectedHttpMethod() throws Exception {
+ final String getParams = "q=hello";
+ HttpServletRequest request = getMock("/solr/select",
"application/x-www-form-urlencoded", 0);
+ when(request.getMethod()).thenReturn("UNEXPECTED");
+ when(request.getQueryString()).thenReturn(getParams);
+
+ MultipartRequestParser multipart = new MultipartRequestParser(2048);
+ RawRequestParser raw = new RawRequestParser();
+ FormDataRequestParser formdata = new FormDataRequestParser(2048);
+ StandardRequestParser standard = new StandardRequestParser(multipart, raw,
formdata);
+
+ final SolrException thrown =
+ expectThrows(
+ SolrException.class,
+ () -> {
+ parser.parse(h.getCore(), "/select", request);
+ });
+ assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, thrown.code());
+ assertEquals("Request contained unexpected HTTP method: UNEXPECTED",
thrown.getMessage());
+ }
+
static class ByteServletInputStream extends ServletInputStream {
final BufferedInputStream in;
final int len;
diff --git
a/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/handler/MirroringConfigSetsHandlerTest.java
b/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/handler/MirroringConfigSetsHandlerTest.java
index 21a8d378208..ad93644e7f3 100644
---
a/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/handler/MirroringConfigSetsHandlerTest.java
+++
b/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/handler/MirroringConfigSetsHandlerTest.java
@@ -91,7 +91,7 @@ public class MirroringConfigSetsHandlerTest extends
SolrTestCaseJ4 {
List.of(new ContentStreamBase.ByteArrayStream(content,
configSetName, "application/zip"));
req.setContentStreams(streams);
}
- req.getContext().put("httpMethod", method);
+ req.getContext().put("httpMethod", SolrRequest.METHOD.fromString(method));
return req;
}
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java
b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java
index 49c52f18d36..03b23d5dcdd 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/SolrRequest.java
@@ -23,11 +23,13 @@ import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import org.apache.solr.client.solrj.impl.HttpSolrClientBase;
import org.apache.solr.client.solrj.request.RequestWriter;
+import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.ContentStream;
@@ -48,9 +50,23 @@ public abstract class SolrRequest<T extends SolrResponse>
implements Serializabl
public enum METHOD {
GET,
+ HEAD,
POST,
PUT,
- DELETE
+ DELETE;
+
+ /**
+ * Returns the METHOD enum value matching the provided string, or 'null'
if no match is found.
+ */
+ public static METHOD fromString(String methodStr) {
+ try {
+ return METHOD.valueOf(methodStr.toUpperCase(Locale.ROOT));
+ } catch (IllegalArgumentException e) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Request contained unexpected HTTP method: " + methodStr);
+ }
+ }
};
public enum ApiVersion {
