From 99a511473760c6c4628ecd638d68c107be541a00 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Tue, 27 Jan 2026 13:02:34 -0500 Subject: [PATCH 1/6] Commenting out code and adding a content type --> wt converter appears to work Some failing tests for plugins like CborResponseWriter that aren't in the content type --> wt converter. --- .../org/apache/solr/core/RequestHandlers.java | 3 +- .../java/org/apache/solr/core/SolrCore.java | 3 +- .../apache/solr/request/SolrQueryRequest.java | 41 ++++++++++++++++++- .../solr/servlet/SolrRequestParsers.java | 24 +++++------ .../org/apache/solr/util/TestHarness.java | 1 + 5 files changed, 55 insertions(+), 17 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/core/RequestHandlers.java b/solr/core/src/java/org/apache/solr/core/RequestHandlers.java index dca7c832e3fa..e20c05ced542 100644 --- a/solr/core/src/java/org/apache/solr/core/RequestHandlers.java +++ b/solr/core/src/java/org/apache/solr/core/RequestHandlers.java @@ -117,7 +117,8 @@ void initHandlersFromConfig(SolrConfig config) { modifiedInfos.add(applyInitParams(config, info)); } handlers.init(Collections.emptyMap(), core, modifiedInfos); - handlers.alias(handlers.getDefault(), ""); + // Curious if this is actually needed! + // handlers.alias(handlers.getDefault(), ""); if (log.isDebugEnabled()) { log.debug("Registered paths: {}", StrUtils.join(new ArrayList<>(handlers.keySet()), ',')); } diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java index 24c7dc83b0c9..6ea54daf9ff8 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -3094,7 +3094,6 @@ public PluginBag getResponseWriters() { HashMap m = new HashMap<>(15, 1); m.put("xml", new XMLResponseWriter()); m.put(CommonParams.JSON, new JacksonJsonWriter()); - m.put("standard", m.get(CommonParams.JSON)); m.put("geojson", new GeoJSONResponseWriter()); m.put("graphml", new GraphMLResponseWriter()); m.put("raw", new RawResponseWriter()); @@ -3154,7 +3153,7 @@ default String getContentType() { private void initWriters() { responseWriters.init(DEFAULT_RESPONSE_WRITERS, this); // configure the default response writer; this one should never be null - if (responseWriters.getDefault() == null) responseWriters.setDefault("standard"); + // if (responseWriters.getDefault() == null) responseWriters.setDefault("standard"); } /** Finds a writer by name, or returns the default writer if not found. */ 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 10115192fbaf..28a5bb9935d5 100644 --- a/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java +++ b/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java @@ -200,12 +200,49 @@ default QueryResponseWriter getResponseWriter() { // it's weird this method is here instead of SolrQueryResponse, but it's practical/convenient SolrCore core = getCore(); String wt = getParams().get(CommonParams.WT); + + if (wt == null || wt.isEmpty()) { + // Fall back to Accept header if wt parameter is not provided + wt = getWtFromAcceptHeader(); + } + if (core != null) { return core.getQueryResponseWriter(wt); } else { - return SolrCore.DEFAULT_RESPONSE_WRITERS.getOrDefault( - wt, SolrCore.DEFAULT_RESPONSE_WRITERS.get("standard")); + return SolrCore.DEFAULT_RESPONSE_WRITERS.get(wt); + } + } + + /** + * Maps the HTTP Accept header to a wt parameter value. Returns "json" as default if no Accept + * header is present or if the content type is not recognized. + * + *

This is a quick implmentation, but it's not pluggable. For example, how do we support CBOR + * without modifying this. In V2ApiUtils.getMediaTypeFromWtParam() we do the same basic thing. i + * also dont' see cbor. + * + * @return the wt parameter value corresponding to the Accept header, or "json" as default + */ + default String getWtFromAcceptHeader() { + HttpSolrCall httpSolrCall = getHttpSolrCall(); + if (httpSolrCall != null) { + String acceptHeader = httpSolrCall.getReq().getHeader("Accept"); + if (acceptHeader != null && !acceptHeader.isEmpty()) { + // Handle multiple accept types (e.g., "application/json, text/plain") + // by checking if the header contains specific content types + if (acceptHeader.contains("application/xml") || acceptHeader.contains("text/xml")) { + return "xml"; + } else if (acceptHeader.contains("application/json")) { + return CommonParams.JSON; + } else if (acceptHeader.contains("application/javabin") + || acceptHeader.contains("application/vnd.apache.solr.javabin")) { + return CommonParams.JAVABIN; + } + // For "*/*" or unrecognized types, fall through to default + } } + // Default to JSON when no Accept header or unrecognized content type + return CommonParams.JSON; } /** 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 3cf12aa7982b..ab82aa5905a8 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java @@ -68,11 +68,11 @@ public class SolrRequestParsers { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); // Should these constants be in a more public place? - public static final String MULTIPART = "multipart"; - public static final String FORMDATA = "formdata"; - public static final String RAW = "raw"; - public static final String SIMPLE = "simple"; - public static final String STANDARD = "standard"; + // public static final String MULTIPART = "multipart"; + // public static final String FORMDATA = "formdata"; + // public static final String RAW = "raw"; + // public static final String SIMPLE = "simple"; + // public static final String STANDARD = "standard"; private static final Charset CHARSET_US_ASCII = StandardCharsets.US_ASCII; @@ -81,7 +81,7 @@ public class SolrRequestParsers { public static final String REQUEST_TIMER_SERVLET_ATTRIBUTE = "org.apache.solr.RequestTimer"; - private final HashMap parsers = new HashMap<>(); + // private final HashMap parsers = new HashMap<>(); private StandardRequestParser standard; /** @@ -120,12 +120,12 @@ private void init(int multipartUploadLimitKB, int formUploadLimitKB) { // I don't see a need to have this publicly configured just yet // adding it is trivial - parsers.put(MULTIPART, multi); - parsers.put(FORMDATA, formdata); - parsers.put(RAW, raw); - parsers.put(SIMPLE, new SimpleRequestParser()); - parsers.put(STANDARD, standard); - parsers.put("", standard); + // parsers.put(MULTIPART, multi); + // parsers.put(FORMDATA, formdata); + // parsers.put(RAW, raw); + // parsers.put(SIMPLE, new SimpleRequestParser()); + // parsers.put(STANDARD, standard); + // parsers.put("", standard); } private static RTimerTree getRequestTimer(HttpServletRequest req) { diff --git a/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java b/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java index 29caa318a048..bee341b78429 100644 --- a/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java +++ b/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java @@ -419,6 +419,7 @@ public LocalRequestFactory() {} @SuppressWarnings({"unchecked"}) public LocalSolrQueryRequest makeRequest(String... q) { if (q.length == 1) { + args.put("wt", "xml"); return new LocalSolrQueryRequest( TestHarness.this.getCore(), q[0], qtype, start, limit, args); } From d976a1a6e281d6035a8f974f6df999171f47043c Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Tue, 27 Jan 2026 14:04:26 -0500 Subject: [PATCH 2/6] don't overwrite the wt if it's been set. --- .../src/java/org/apache/solr/util/TestHarness.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java b/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java index bee341b78429..a933605466fc 100644 --- a/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java +++ b/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java @@ -419,7 +419,7 @@ public LocalRequestFactory() {} @SuppressWarnings({"unchecked"}) public LocalSolrQueryRequest makeRequest(String... q) { if (q.length == 1) { - args.put("wt", "xml"); + args.computeIfAbsent("wt", k -> "xml"); return new LocalSolrQueryRequest( TestHarness.this.getCore(), q[0], qtype, start, limit, args); } From 63d428485187eff43253ccdb9b7cbf3a722bd049 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Tue, 27 Jan 2026 14:09:41 -0500 Subject: [PATCH 3/6] Change the logic so that if you use a bad wt then instead of a 400 not found you get a 500 error. I think using a bad responsewriter should be an error, and we shoudln't cover it up, or return json... --- .../apache/solr/request/SolrQueryRequest.java | 11 ++++++-- .../solr/handler/V2ApiIntegrationTest.java | 25 ++++++++++--------- .../TestPrometheusResponseWriter.java | 3 ++- 3 files changed, 24 insertions(+), 15 deletions(-) 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 28a5bb9935d5..2c7480e2c2b7 100644 --- a/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java +++ b/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java @@ -23,6 +23,7 @@ import java.util.Map; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.cloud.CloudDescriptor; +import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.CommandOperation; @@ -206,11 +207,17 @@ default QueryResponseWriter getResponseWriter() { wt = getWtFromAcceptHeader(); } + QueryResponseWriter writer; if (core != null) { - return core.getQueryResponseWriter(wt); + writer = core.getQueryResponseWriter(wt); } else { - return SolrCore.DEFAULT_RESPONSE_WRITERS.get(wt); + writer = SolrCore.DEFAULT_RESPONSE_WRITERS.get(wt); } + if (writer == null) { + throw new SolrException( + SolrException.ErrorCode.SERVER_ERROR, "Unknown response writer type: " + wt); + } + return writer; } /** diff --git a/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java b/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java index 3b7a85eb0967..cbd12f8a99b4 100644 --- a/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java +++ b/solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java @@ -114,26 +114,27 @@ public void testIntrospect() throws Exception { } @Test - public void testWTParam() throws Exception { + public void testInvalidWTParamReturnsError() throws Exception { V2Request request = new V2Request.Builder("/c/" + COLL_NAME + "/get/_introspect").build(); - // TODO: If possible do this in a better way + // Using an invalid wt parameter should return a 500 error request.setResponseParser(new InputStreamResponseParser("bleh")); NamedList res = cluster.getSolrClient().request(request); String respString = InputStreamResponseParser.consumeResponseToString(res); - assertFalse(respString.contains("

HTTP ERROR 500

")); - assertFalse(respString.contains("500")); - assertFalse(respString.contains("NullPointerException")); - assertFalse( - respString.contains( - "

Problem accessing /solr/____v2/c/collection1/get/_introspect. Reason:")); - // since no-op response writer is used, doing contains match - assertTrue(respString.contains("/c/collection1/get")); + // Should get a 500 Server Error for unknown writer type + assertTrue( + "Expected error message about unknown writer type", + respString.contains("Unknown response writer type")); + assertTrue("Expected 500 error code", respString.contains("500")); + } - // no response parser + @Test + public void testWTParam() throws Exception { + // When no response parser is set, the default JSON writer should be used + V2Request request = new V2Request.Builder("/c/" + COLL_NAME + "/get/_introspect").build(); request.setResponseParser(null); Map resp = resAsMap(cluster.getSolrClient(), request); - respString = resp.toString(); + String respString = resp.toString(); assertFalse(respString.contains("

HTTP ERROR 500

")); assertFalse( diff --git a/solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriter.java b/solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriter.java index d9d2493eb3a1..63ba5a448923 100644 --- a/solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriter.java +++ b/solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriter.java @@ -195,7 +195,8 @@ public void testUnsupportedMetricsFormat() throws Exception { try (SolrClient adminClient = getHttpSolrClient(solrTestRule.getBaseUrl())) { NamedList res = adminClient.request(req); - assertEquals(400, res.get("responseStatus")); + // Unknown wt parameter should return a 500 error + assertEquals(500, res.get("responseStatus")); } } } From 90dd9871b8a693949506a48fb929ec6022cee2ba Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Tue, 27 Jan 2026 16:28:31 -0500 Subject: [PATCH 4/6] json is the new standard --- solr/core/src/test/org/apache/solr/OutputWriterTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/core/src/test/org/apache/solr/OutputWriterTest.java b/solr/core/src/test/org/apache/solr/OutputWriterTest.java index 30df9ed98fab..7aa332653ba1 100644 --- a/solr/core/src/test/org/apache/solr/OutputWriterTest.java +++ b/solr/core/src/test/org/apache/solr/OutputWriterTest.java @@ -44,7 +44,7 @@ public static void beforeClass() throws Exception { @Test public void testSOLR59responseHeaderVersions() { // default results in "new" responseHeader - lrf.args.put("wt", "standard"); + lrf.args.put("wt", "json"); assertQ(req("foo"), "/response/lst[@name='responseHeader']/int[@name='status'][.='0']"); lrf.args.remove("wt"); assertQ(req("foo"), "/response/lst[@name='responseHeader']/int[@name='QTime']"); From 947464b13c4bfab1a68e9deef453365bda25c093 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Tue, 27 Jan 2026 17:32:56 -0500 Subject: [PATCH 5/6] No longer supporting a "default" since we specify what we want everywhere. --- .../java/org/apache/solr/core/SolrCore.java | 4 ++-- .../solr/response/TestRawResponseWriter.java | 24 +++++++++---------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java index 6ea54daf9ff8..b204d0d5c3fd 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -3156,9 +3156,9 @@ private void initWriters() { // if (responseWriters.getDefault() == null) responseWriters.setDefault("standard"); } - /** Finds a writer by name, or returns the default writer if not found. */ + /** Finds a writer by name, or null if not found. */ public final QueryResponseWriter getQueryResponseWriter(String writerName) { - return responseWriters.get(writerName, true); + return responseWriters.get(writerName, false); } /** diff --git a/solr/core/src/test/org/apache/solr/response/TestRawResponseWriter.java b/solr/core/src/test/org/apache/solr/response/TestRawResponseWriter.java index 7b822b18848d..8799277bddde 100644 --- a/solr/core/src/test/org/apache/solr/response/TestRawResponseWriter.java +++ b/solr/core/src/test/org/apache/solr/response/TestRawResponseWriter.java @@ -44,7 +44,7 @@ public class TestRawResponseWriter extends SolrTestCaseJ4 { private static RawResponseWriter writerJsonBase; private static RawResponseWriter writerBinBase; private static RawResponseWriter writerCborBase; - private static RawResponseWriter writerNoBase; + // private static RawResponseWriter writerNoBase; private static RawResponseWriter[] allWriters; @@ -56,16 +56,14 @@ public static void setupCoreAndWriters() throws Exception { // we spin up. initCore("solrconfig.xml", "schema.xml"); - writerNoBase = newRawResponseWriter(null); /* defaults to standard writer as base */ + // writerNoBase = newRawResponseWriter(null); /* defaults to standard writer as base */ writerXmlBase = newRawResponseWriter("xml"); writerJsonBase = newRawResponseWriter("json"); writerBinBase = newRawResponseWriter("javabin"); writerCborBase = newRawResponseWriter("cbor"); allWriters = - new RawResponseWriter[] { - writerXmlBase, writerJsonBase, writerBinBase, writerCborBase, writerNoBase - }; + new RawResponseWriter[] {writerXmlBase, writerJsonBase, writerBinBase, writerCborBase}; } @AfterClass @@ -73,7 +71,7 @@ public static void cleanupWriters() { writerXmlBase = null; writerJsonBase = null; writerBinBase = null; - writerNoBase = null; + // writerNoBase = null; writerCborBase = null; allWriters = null; } @@ -120,7 +118,7 @@ public void testRawStringContentStream() throws IOException { // we should have UTF-8 Bytes if we use an OutputStream ByteArrayOutputStream bout = new ByteArrayOutputStream(); writer.write(bout, req(), rsp); - assertEquals(data, bout.toString(StandardCharsets.UTF_8.toString())); + assertEquals(data, bout.toString(StandardCharsets.UTF_8)); } } @@ -133,7 +131,7 @@ public void testStructuredDataViaBaseWriters() throws IOException { rsp.add("foo", "bar"); // check Content-Type against each writer - assertEquals("application/xml; charset=UTF-8", writerNoBase.getContentType(req(), rsp)); + // assertEquals("application/xml; charset=UTF-8", writerNoBase.getContentType(req(), rsp)); assertEquals("application/xml; charset=UTF-8", writerXmlBase.getContentType(req(), rsp)); assertEquals("application/json; charset=UTF-8", writerJsonBase.getContentType(req(), rsp)); assertEquals("application/octet-stream", writerBinBase.getContentType(req(), rsp)); @@ -153,13 +151,13 @@ public void testStructuredDataViaBaseWriters() throws IOException { assertEquals(xml, writerXmlBase.writeToString(req(), rsp)); ByteArrayOutputStream xmlBout = new ByteArrayOutputStream(); writerXmlBase.write(xmlBout, req(), rsp); - assertEquals(xml, xmlBout.toString(StandardCharsets.UTF_8.toString())); + assertEquals(xml, xmlBout.toString(StandardCharsets.UTF_8)); // - assertEquals(xml, writerNoBase.writeToString(req(), rsp)); - ByteArrayOutputStream noneBout = new ByteArrayOutputStream(); - writerNoBase.write(noneBout, req(), rsp); - assertEquals(xml, noneBout.toString(StandardCharsets.UTF_8.toString())); + // assertEquals(xml, writerNoBase.writeToString(req(), rsp)); + // ByteArrayOutputStream noneBout = new ByteArrayOutputStream(); + // writerNoBase.write(noneBout, req(), rsp); + // assertEquals(xml, noneBout.toString(StandardCharsets.UTF_8.toString())); // json String json = "{\n" + " \"content\":\"test\",\n" + " \"foo\":\"bar\"}\n"; From c76ad676f35d637c109abafbc148322db69a7cb0 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Sat, 31 Jan 2026 15:29:30 -0500 Subject: [PATCH 6/6] fix merge --- .../apache/solr/request/SolrQueryRequest.java | 45 ------------------- 1 file changed, 45 deletions(-) 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 5fd7da8dae12..0ce4d82e5516 100644 --- a/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java +++ b/solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java @@ -23,7 +23,6 @@ import java.util.Map; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.cloud.CloudDescriptor; -import org.apache.solr.common.SolrException; import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.CommandOperation; @@ -213,50 +212,6 @@ default QueryResponseWriter getResponseWriter() { } else { return ResponseWritersRegistry.getWriter(wt); } - - QueryResponseWriter writer; - if (core != null) { - writer = core.getQueryResponseWriter(wt); - } else { - writer = SolrCore.DEFAULT_RESPONSE_WRITERS.get(wt); - } - if (writer == null) { - throw new SolrException( - SolrException.ErrorCode.SERVER_ERROR, "Unknown response writer type: " + wt); - } - return writer; - } - - /** - * Maps the HTTP Accept header to a wt parameter value. Returns "json" as default if no Accept - * header is present or if the content type is not recognized. - * - *

This is a quick implmentation, but it's not pluggable. For example, how do we support CBOR - * without modifying this. In V2ApiUtils.getMediaTypeFromWtParam() we do the same basic thing. i - * also dont' see cbor. - * - * @return the wt parameter value corresponding to the Accept header, or "json" as default - */ - default String getWtFromAcceptHeader() { - HttpSolrCall httpSolrCall = getHttpSolrCall(); - if (httpSolrCall != null) { - String acceptHeader = httpSolrCall.getReq().getHeader("Accept"); - if (acceptHeader != null && !acceptHeader.isEmpty()) { - // Handle multiple accept types (e.g., "application/json, text/plain") - // by checking if the header contains specific content types - if (acceptHeader.contains("application/xml") || acceptHeader.contains("text/xml")) { - return "xml"; - } else if (acceptHeader.contains("application/json")) { - return CommonParams.JSON; - } else if (acceptHeader.contains("application/javabin") - || acceptHeader.contains("application/vnd.apache.solr.javabin")) { - return CommonParams.JAVABIN; - } - // For "*/*" or unrecognized types, fall through to default - } - } - // Default to JSON when no Accept header or unrecognized content type - return CommonParams.JSON; } /**