This is an automated email from the ASF dual-hosted git repository.
gerlowskija pushed a commit to branch branch_10x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_10x by this push:
new 6f17d5244b2 Replace HTTP-method string-handling with enum
6f17d5244b2 is described below
commit 6f17d5244b27b5443d02eda5b5a4f5d7f95ea7a3
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 | 65 ++++++++++++----------
.../org/apache/solr/handler/SolrConfigHandler.java | 41 ++++++++------
.../solr/handler/admin/SecurityConfHandler.java | 7 ++-
.../org/apache/solr/request/SolrQueryRequest.java | 3 +-
.../apache/solr/servlet/SolrRequestParsers.java | 8 ++-
.../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, 125 insertions(+), 62 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 df749dedeec..c27798e1bcf 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
@@ -168,7 +168,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);
@@ -221,7 +221,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 967054208b1..349b46a9423 100644
--- a/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
@@ -35,6 +35,7 @@ import java.util.Set;
import java.util.stream.Collectors;
import org.apache.solr.api.Api;
import org.apache.solr.api.JerseyResource;
+import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.common.SolrErrorWrappingException;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.MapSolrParams;
@@ -79,38 +80,46 @@ 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<CommandOperation> ops = req.getCommands(false);
- List<Map<String, Object>> errs = CommandOperation.captureErrors(ops);
- if (!errs.isEmpty()) {
- throw new SolrErrorWrappingException(
- SolrException.ErrorCode.BAD_REQUEST, "error processing
commands", errs);
+ final var httpMethod = (SolrRequest.METHOD)
req.getContext().get("httpMethod");
+ switch (httpMethod) {
+ case SolrRequest.METHOD.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");
}
- final var operationList =
- ops.stream()
- .map(co ->
SchemaManagerUtils.convertToSchemaChangeOperations(co))
- .collect(Collectors.toList());
- errs = new SchemaManager(req).performOperations(operationList);
- if (!errs.isEmpty()) {
- throw new SolrErrorWrappingException(
- SolrException.ErrorCode.BAD_REQUEST, "error processing
commands", errs);
+ try {
+ List<CommandOperation> ops = req.getCommands(false);
+ List<Map<String, Object>> errs = CommandOperation.captureErrors(ops);
+ if (!errs.isEmpty()) {
+ throw new SolrErrorWrappingException(
+ SolrException.ErrorCode.BAD_REQUEST, "error processing
commands", errs);
+ }
+
+ final var operationList =
+ ops.stream()
+ .map(co ->
SchemaManagerUtils.convertToSchemaChangeOperations(co))
+ .collect(Collectors.toList());
+ errs = new SchemaManager(req).performOperations(operationList);
+ if (!errs.isEmpty()) {
+ throw new SolrErrorWrappingException(
+ 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);
}
- } catch (IOException e) {
+ break;
+ case SolrRequest.METHOD.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 652649e3669..7b8d0417d20 100644
--- a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
@@ -53,6 +53,7 @@ import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import org.apache.solr.api.AnnotatedApi;
import org.apache.solr.api.Api;
+import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrResponse;
import org.apache.solr.client.solrj.io.stream.expr.Expressible;
import org.apache.solr.client.solrj.jetty.HttpJettySolrClient;
@@ -131,24 +132,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 (!configEditingEnabled || isImmutableConfigSet) {
- final String reason =
- !configEditingEnabled
- ? "due to " + CONFIG_EDITING_ENABLED_ARG + " setting"
- : "because ConfigSet is marked immutable";
+ final var httpMethod = (SolrRequest.METHOD)
req.getContext().get("httpMethod");
+ Command command = new Command(req, rsp, httpMethod.name());
+ switch (httpMethod) {
+ case SolrRequest.METHOD.POST:
+ if (!configEditingEnabled || isImmutableConfigSet) {
+ final String reason =
+ !configEditingEnabled
+ ? "due to " + CONFIG_EDITING_ENABLED_ARG + " setting"
+ : "because ConfigSet is marked immutable";
+ throw new SolrException(
+ SolrException.ErrorCode.FORBIDDEN, " solrconfig editing is not
enabled " + reason);
+ }
+ try {
+ command.handlePOST();
+ } finally {
+ RequestHandlerUtils.addExperimentalFormatWarning(rsp);
+ }
+ break;
+ case SolrRequest.METHOD.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 de8078ddc31..10115192fba 100644
--- a/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java
+++ b/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java
@@ -21,6 +21,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;
@@ -162,7 +163,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 b609f378aef..3cf12aa7982 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java
@@ -43,6 +43,7 @@ import java.util.Set;
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.MultiMapSolrParams;
@@ -152,7 +153,12 @@ 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);
+
return sreq;
}
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 93ec52df840..f1e16e46dc4 100644
--- a/solr/core/src/test/org/apache/solr/servlet/SolrRequestParserTest.java
+++ b/solr/core/src/test/org/apache/solr/servlet/SolrRequestParserTest.java
@@ -145,6 +145,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 97a4687c64d..49b92587312 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 af83beb70fd..f33dd5dd0c2 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,6 +23,7 @@ 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;
@@ -31,6 +32,7 @@ import org.apache.solr.client.solrj.impl.HttpSolrClientBase;
import org.apache.solr.client.solrj.request.RequestWriter;
import org.apache.solr.client.solrj.response.ResponseParser;
import org.apache.solr.client.solrj.response.StreamingResponseCallback;
+import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.ContentStream;
import org.apache.solr.common.util.NamedList;
@@ -60,9 +62,23 @@ public abstract class SolrRequest<T> implements Serializable
{
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 {
