SOLR-18085: Review use of "wt=standard" and see if we can remove it. #4080

epugh · 2026-01-27T19:12:51Z

https://issues.apache.org/jira/browse/SOLR-18085

Description

The use of wt=standard is odd.. It really means return json formatted code. I dug around some more and we have werid complex logic around this parameter. Digging furthur, I think most of the complexity is not needed.

The one thing that I DID change is that before if you did "wt=fake" then you would get JSON back in many places. And in the V2 api's if you did "wt=fake" you would get a 400 back with an error. I think the right think in either of those cases is a 500 server error.

Summary of 63d4284

I've updated the code so that when an invalid/unknown wt parameter is provided, Solr returns a 500 Server Error instead of silently falling back or returning a null writer.

Changes Made to default behavior:

SolrQueryRequest.java (lines 207-222):
- Added validation that throws a SolrException with SERVER_ERROR (500) when the response writer is not found for the given wt parameter
- The error message clearly states: "Unknown response writer type: {wt}"
V2ApiIntegrationTest.java:
- Split the original testWTParam test into two tests:
  - testInvalidWTParamReturnsError - verifies that wt=bleh returns a 500 error with the appropriate message
  - testWTParam - verifies that when no wt is specified, JSON is used by default
TestPrometheusResponseWriter.java:
- Updated the comment in testUnsupportedMetricsFormat to clarify that unknown wt parameters return a 500 error
- The test already asserted assertEquals(500, res.get("responseStatus")) which now passes

Solution

Mostly refactoring, except for the big change in 63d4284 which changed the response logic.

Tests

Re ran test

…s to work Some failing tests for plugins like CborResponseWriter that aren't in the content type --> wt converter.

…t 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...

epugh · 2026-01-27T19:16:51Z

solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java

+
+    if (wt == null || wt.isEmpty()) {
+      // Fall back to Accept header if wt parameter is not provided
+      wt = getWtFromAcceptHeader();


this was interesting... It's a duplicate from some utilit code in v2. and I don't know if it's good or not...

epugh · 2026-01-27T19:17:02Z

solr/core/src/java/org/apache/solr/core/RequestHandlers.java

    handlers.init(Collections.emptyMap(), core, modifiedInfos);
-    handlers.alias(handlers.getDefault(), "");
+    // Curious if this is actually needed!
+    // handlers.alias(handlers.getDefault(), "");


tests all pass!

epugh · 2026-01-27T19:18:07Z

solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java

+      }
    }
+    // Default to JSON when no Accept header or unrecognized content type
+    return CommonParams.JSON;


i remain ambivalent about these... maybe okay? I mean, if I do a curl command don't provide wt=json OR the content types, maybe okay? It might be a perfectly nice thign for our users...

epugh · 2026-01-27T19:18:18Z

solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java

-  public static final String SIMPLE = "simple";
-  public static final String STANDARD = "standard";
+  // public static final String MULTIPART = "multipart";
+  // public static final String FORMDATA = "formdata";


so many unused statics!

epugh · 2026-01-27T19:18:37Z

solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java

+  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


This is the Big Change.

Copilot

Pull request overview

Updates Solr’s response-writer (wt) handling to reduce reliance on the legacy "standard" writer and to fail fast when an unknown response writer is requested.

Changes:

Adds stricter wt validation and introduces an Accept-header fallback when wt is absent.
Removes/neutralizes legacy "standard" response-writer wiring and adjusts handler aliasing behavior.
Updates tests and test utilities to reflect the new error behavior and defaults.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
solr/test-framework/src/java/org/apache/solr/util/TestHarness.java	Forces `wt=xml` in a common local test request path to preserve historical XML expectations.
solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriter.java	Updates expected status for unknown `wt` to 500.
solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java	Splits `wt` tests and asserts 500 + message for invalid writer types.
solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java	Comments out legacy constants/registry code (effectively disabling/removing it).
solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java	Adds `Accept` header → `wt` mapping and throws 500 for unknown response writers.
solr/core/src/java/org/apache/solr/core/SolrCore.java	Removes `"standard"` alias from default writers and stops setting a default writer.
solr/core/src/java/org/apache/solr/core/RequestHandlers.java	Stops aliasing the configured default handler to the empty path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-27T19:26:25Z

solr/core/src/java/org/apache/solr/servlet/SolrRequestParsers.java

  // 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";


This change leaves large blocks of commented-out code (public constants and the parser registry) rather than removing them. Since these are now dead code paths, it’s better to delete them to avoid confusion and reduce maintenance overhead (or keep them active if they’re still intended to be supported).

Copilot · 2026-01-27T19:26:25Z

solr/core/src/java/org/apache/solr/core/RequestHandlers.java

+    // Curious if this is actually needed!
+    // handlers.alias(handlers.getDefault(), "");


Commenting out the aliasing of the configured default handler to the empty path ("") can change runtime behavior for configs that set a non-/select handler as the default. For example, solr/modules/langid/src/test-files/langid/solr/collection1/conf/solrconfig-languageidentifier.xml defines a default handler named "search"; without this aliasing, requests to the core root will be routed to /select instead of the declared default. Consider restoring alias(handlers.getDefault(), "") (or otherwise ensuring plugin-bag default takes precedence over the /select fallback).

Suggested change

// Curious if this is actually needed!

// handlers.alias(handlers.getDefault(), "");

// Ensure the configured default handler (if any) is aliased to the empty path ("")

String defaultHandler = handlers.getDefault();

if (defaultHandler != null) {

handlers.alias(defaultHandler, "");

}

Copilot · 2026-01-27T19:26:25Z

solr/core/src/java/org/apache/solr/core/SolrCore.java

    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");


The default response writer is no longer being set when none is explicitly marked default in solrconfig. This can cause core.getQueryResponseWriter(req) / core.getQueryResponseWriter(null) to return null, which leads to NullPointerExceptions in call sites that assume a non-null default (e.g., GeoTransformerFactory uses req.getCore().getQueryResponseWriter(req) and then calls getClass() on it). Consider restoring a non-null default (likely "json" now that "standard" is being removed) and keep unknown-wt validation separate from default selection.

Suggested change

// if (responseWriters.getDefault() == null) responseWriters.setDefault("standard");

if (responseWriters.getDefault() == null) {

// Use "json" as the default writer now that the legacy "standard" writer is being removed.

responseWriters.setDefault("json");

}

Copilot · 2026-01-27T19:26:26Z

solr/core/src/java/org/apache/solr/core/SolrCore.java

@@ -3094,7 +3094,6 @@ public PluginBag<QueryResponseWriter> getResponseWriters() {
    HashMap<String, QueryResponseWriter> m = new HashMap<>(15, 1);
    m.put("xml", new XMLResponseWriter());
    m.put(CommonParams.JSON, new JacksonJsonWriter());


Removing the "standard" response writer alias is a backwards-incompatible change: existing code/tests in this repo still use wt=standard (e.g., solr/core/src/test/org/apache/solr/OutputWriterTest.java sets lrf.args.put("wt","standard")). If the intent is to deprecate it, consider keeping the alias (mapping to json) while emitting a deprecation warning, or update all internal and documented usages before removing it outright.

Suggested change

m.put(CommonParams.JSON, new JacksonJsonWriter());

m.put(CommonParams.JSON, new JacksonJsonWriter());

m.put("standard", new JacksonJsonWriter());

Copilot · 2026-01-27T19:26:26Z

solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java

+    QueryResponseWriter writer;
    if (core != null) {
-      return core.getQueryResponseWriter(wt);
+      writer = core.getQueryResponseWriter(wt);


Using core.getQueryResponseWriter(wt) here won’t reliably detect an unknown wt, because SolrCore#getQueryResponseWriter(String) falls back to the configured default when a writer name isn’t found. That means writer may be non-null even for invalid wt values, and the "Unknown response writer" exception won’t be thrown in configs that have a default writer set. Consider looking up the writer without default fallback (e.g., via core.getResponseWriters().get(wt) or a new SolrCore API that does not use the default) and only using defaults when wt is absent.

Suggested change

writer = core.getQueryResponseWriter(wt);

// Avoid using SolrCore#getQueryResponseWriter(wt) for explicit wt values, since it

// falls back to the default writer when the name is not found. That would mask

// unknown wt values and prevent us from throwing an error below.

if (wt == null || wt.isEmpty()) {

// No explicit wt: allow SolrCore to apply its default writer behavior.

writer = core.getQueryResponseWriter(wt);

} else {

// Explicit wt: look up without default fallback.

writer = core.getResponseWriters().get(wt);

}

Copilot · 2026-01-27T19:26:26Z

solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java

+   * 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.
+   *
+   * <p>This is a quick implmentation, but it's not pluggable. For example, how do we support CBOR


Typo in comment: "implmentation" should be "implementation".

Suggested change

* <p>This is a quick implmentation, but it's not pluggable. For example, how do we support CBOR

* <p>This is a quick implementation, but it's not pluggable. For example, how do we support CBOR

Copilot · 2026-01-27T19:26:27Z

solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java

+   *
+   * <p>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.


Typo in comment: "dont'" should be "don't".

Suggested change

* also dont' see cbor.

* also don't see cbor.

Copilot · 2026-01-27T19:26:27Z

solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java

+   * <p>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.
+   *


The comment says you “don’t see cbor”, but CBOR is present in SolrCore.DEFAULT_RESPONSE_WRITERS (key "cbor"). Either update the comment to match reality, or extend getWtFromAcceptHeader() to map Accept headers for the supported writers (including CBOR) so the comment and behavior are aligned.

…here.

epugh added 3 commits January 27, 2026 13:02

Commenting out code and adding a content type --> wt converter appear…

99a5114

…s to work Some failing tests for plugins like CborResponseWriter that aren't in the content type --> wt converter.

don't overwrite the wt if it's been set.

d976a1a

Change the logic so that if you use a bad wt then instead of a 400 no…

63d4284

…t 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...

github-actions bot added test-framework jetty-server tests cat:search labels Jan 27, 2026

epugh requested review from Copilot and dsmiley January 27, 2026 19:15

Copilot started reviewing on behalf of epugh January 27, 2026 19:16 View session

epugh commented Jan 27, 2026

View reviewed changes

Copilot AI reviewed Jan 27, 2026

View reviewed changes

epugh added 4 commits January 27, 2026 16:28

json is the new standard

90dd987

No longer supporting a "default" since we specify what we want everyw…

947464b

…here.

Merge remote-tracking branch 'upstream/main' into SOLR-18085

5753e33

fix merge

c76ad67

		// Curious if this is actually needed!
		// handlers.alias(handlers.getDefault(), "");

-    // Curious if this is actually needed!
-    // handlers.alias(handlers.getDefault(), "");
+    // Ensure the configured default handler (if any) is aliased to the empty path ("")
+    String defaultHandler = handlers.getDefault();
+    if (defaultHandler != null) {
+      handlers.alias(defaultHandler, "");
+    }

-    // if (responseWriters.getDefault() == null) responseWriters.setDefault("standard");
+    if (responseWriters.getDefault() == null) {
+      // Use "json" as the default writer now that the legacy "standard" writer is being removed.
+      responseWriters.setDefault("json");
+    }

	m.put(CommonParams.JSON, new JacksonJsonWriter());
	m.put(CommonParams.JSON, new JacksonJsonWriter());
	m.put("standard", new JacksonJsonWriter());

-      writer = core.getQueryResponseWriter(wt);
+      // Avoid using SolrCore#getQueryResponseWriter(wt) for explicit wt values, since it
+      // falls back to the default writer when the name is not found. That would mask
+      // unknown wt values and prevent us from throwing an error below.
+      if (wt == null || wt.isEmpty()) {
+        // No explicit wt: allow SolrCore to apply its default writer behavior.
+        writer = core.getQueryResponseWriter(wt);
+      } else {
+        // Explicit wt: look up without default fallback.
+        writer = core.getResponseWriters().get(wt);
+      }

	* <p>This is a quick implmentation, but it's not pluggable. For example, how do we support CBOR
	* <p>This is a quick implementation, but it's not pluggable. For example, how do we support CBOR

SOLR-18085: Review use of "wt=standard" and see if we can remove it. #4080

Are you sure you want to change the base?

SOLR-18085: Review use of "wt=standard" and see if we can remove it. #4080

Conversation

epugh commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes Made to default behavior:

Solution

Tests

Uh oh!

epugh Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

epugh Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

epugh Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

epugh Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

epugh Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

epugh Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

epugh commented Jan 27, 2026 •

edited

Loading