Remove MediaType as instance variable from RestSqlQueryAction. (#69901)

x-pack/plugin/sql/qa/server/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java

@@ -926,6 +926,35 @@ public abstract class RestSqlTestCase extends BaseRestSqlTestCase implements Err
         executeQueryWithNextPage("text/csv; header=present", "text,number,sum\r\n", "%s,%d,%d\r\n");
     }
 
+    public void testCSVWithDelimiterParameter() throws IOException {
+        String format = randomFrom("txt", "tsv", "json", "yaml", "smile", "cbor");
+        String query = "SELECT * FROM test";
+        index("{\"foo\":1}");
+
+        Request badRequest = new Request("POST", SQL_QUERY_REST_ENDPOINT);
+        badRequest.addParameter("format", format);
+        badRequest.addParameter("delimiter", ";");
+        badRequest.setEntity(
+            new StringEntity(
+                query(query).mode(randomValueOtherThan(Mode.JDBC.toString(), BaseRestSqlTestCase::randomMode)).toString(),
+                ContentType.APPLICATION_JSON
+            )
+        );
+        expectBadRequest(() -> {
+            client().performRequest(badRequest);
+            return Collections.emptyMap();
+        }, containsString("request [/_sql] contains unrecognized parameter: [delimiter]"));
+
+        Request csvRequest = new Request("POST", SQL_QUERY_REST_ENDPOINT + "?format=csv&delimiter=%3B");
+        csvRequest.setEntity(
+            new StringEntity(
+                query(query).mode(randomValueOtherThan(Mode.JDBC.toString(), BaseRestSqlTestCase::randomMode)).toString(),
+                ContentType.APPLICATION_JSON
+            )
+        );
+        assertOK(client().performRequest(csvRequest));
+    }
+
     public void testQueryInTSV() throws IOException {
         index(
             "{\"name\":" + toJson("first") + ", \"number\" : 1 }",

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlQueryAction.java

@@ -31,6 +31,7 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 
+import static java.util.Collections.emptySet;
 import static org.elasticsearch.rest.RestRequest.Method.GET;
 import static org.elasticsearch.rest.RestRequest.Method.POST;
 import static org.elasticsearch.xpack.sql.proto.Protocol.URL_PARAM_DELIMITER;
@@ -38,7 +39,6 @@ import static org.elasticsearch.xpack.sql.proto.Protocol.URL_PARAM_DELIMITER;
 public class RestSqlQueryAction extends BaseRestHandler {
 
     private final SqlMediaTypeParser sqlMediaTypeParser = new SqlMediaTypeParser();
-    MediaType responseMediaType;
 
     @Override
     public List<Route> routes() {
@@ -59,12 +59,24 @@ public class RestSqlQueryAction extends BaseRestHandler {
             sqlRequest = SqlQueryRequest.fromXContent(parser);
         }
 
-        responseMediaType = sqlMediaTypeParser.getResponseMediaType(request, sqlRequest);
+        MediaType responseMediaType = sqlMediaTypeParser.getResponseMediaType(request, sqlRequest);
         if (responseMediaType == null) {
             String msg = String.format(Locale.ROOT, "Invalid response content type: Accept=[%s], Content-Type=[%s], format=[%s]",
                 request.header("Accept"), request.header("Content-Type"), request.param("format"));
             throw new IllegalArgumentException(msg);
         }
+
+        /*
+         * Special handling for the "delimiter" parameter which should only be
+         * checked for being present or not in the case of CSV format. We cannot
+         * override {@link BaseRestHandler#responseParams()} because this
+         * parameter should only be checked for CSV, not always.
+         */
+        if ((responseMediaType instanceof XContentType || ((TextFormat) responseMediaType) != TextFormat.CSV)
+            && request.hasParam(URL_PARAM_DELIMITER)) {
+            throw new IllegalArgumentException(unrecognized(request, Collections.singleton(URL_PARAM_DELIMITER), emptySet(), "parameter"));
+        }
+
         long startNanos = System.nanoTime();
         return channel -> client.execute(SqlQueryAction.INSTANCE, sqlRequest, new RestResponseListener<SqlQueryResponse>(channel) {
             @Override
@@ -78,11 +90,10 @@ public class RestSqlQueryAction extends BaseRestHandler {
                     response.toXContent(builder, request);
                     restResponse = new BytesRestResponse(RestStatus.OK, builder);
                 } else { // TextFormat
-                    TextFormat type = (TextFormat)responseMediaType;
+                    TextFormat type = (TextFormat) responseMediaType;
                     final String data = type.format(request, response);
 
-                    restResponse = new BytesRestResponse(RestStatus.OK, type.contentType(request),
-                        data.getBytes(StandardCharsets.UTF_8));
+                    restResponse = new BytesRestResponse(RestStatus.OK, type.contentType(request), data.getBytes(StandardCharsets.UTF_8));
 
                     if (response.hasCursor()) {
                         restResponse.addHeader("Cursor", response.cursor());
@@ -97,7 +108,7 @@ public class RestSqlQueryAction extends BaseRestHandler {
 
     @Override
     protected Set<String> responseParams() {
-        return responseMediaType == TextFormat.CSV ? Collections.singleton(URL_PARAM_DELIMITER) : Collections.emptySet();
+        return Collections.singleton(URL_PARAM_DELIMITER);
     }
 
     @Override

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/SqlMediaTypeParser.java

@@ -51,7 +51,7 @@ public class SqlMediaTypeParser {
     }
 
     private static MediaType validateColumnarRequest(boolean requestIsColumnar, MediaType fromMediaType) {
-        if(requestIsColumnar && fromMediaType instanceof TextFormat){
+        if (requestIsColumnar && fromMediaType instanceof TextFormat) {
             throw new IllegalArgumentException("Invalid use of [columnar] argument: cannot be used in combination with "
                 + "txt, csv or tsv formats");
         }