Enforce Content-Type requirement on the rest layer and remove depreca…

…ted methods (#23146)

This commit enforces the requirement of Content-Type for the REST layer and removes the deprecated methods in transport
requests and their usages.

While doing this, it turns out that there are many places where *Entity classes are used from the apache http client
libraries and many of these usages did not specify the content type. The methods that do not specify a content type
explicitly have been added to forbidden apis to prevent more of these from entering our code base.

Relates #19388

buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy

@@ -91,6 +91,7 @@ class PrecommitTasks {
         if (testForbidden != null) {
             testForbidden.configure {
                 signaturesURLs += getClass().getResource('/forbidden/es-test-signatures.txt')
+                signaturesURLs += getClass().getResource('/forbidden/http-signatures.txt')
             }
         }
         Task forbiddenApis = project.tasks.findByName('forbiddenApis')

buildSrc/src/main/resources/forbidden/http-signatures.txt

@@ -0,0 +1,45 @@
+# Licensed to Elasticsearch under one or more contributor
+# license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright
+# ownership. Elasticsearch licenses this file to you under
+# the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance  with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on
+# an 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
+# either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+
+@defaultMessage Explicitly specify the ContentType of HTTP entities when creating
+org.apache.http.entity.StringEntity#<init>(java.lang.String)
+org.apache.http.entity.StringEntity#<init>(java.lang.String,java.lang.String)
+org.apache.http.entity.StringEntity#<init>(java.lang.String,java.nio.charset.Charset)
+org.apache.http.entity.ByteArrayEntity#<init>(byte[])
+org.apache.http.entity.ByteArrayEntity#<init>(byte[],int,int)
+org.apache.http.entity.FileEntity#<init>(java.io.File)
+org.apache.http.entity.InputStreamEntity#<init>(java.io.InputStream)
+org.apache.http.entity.InputStreamEntity#<init>(java.io.InputStream,long)
+org.apache.http.nio.entity.NByteArrayEntity#<init>(byte[])
+org.apache.http.nio.entity.NByteArrayEntity#<init>(byte[],int,int)
+org.apache.http.nio.entity.NFileEntity#<init>(java.io.File)
+org.apache.http.nio.entity.NStringEntity#<init>(java.lang.String)
+org.apache.http.nio.entity.NStringEntity#<init>(java.lang.String,java.lang.String)
+
+@defaultMessage Use non-deprecated constructors
+org.apache.http.nio.entity.NFileEntity#<init>(java.io.File,java.lang.String)
+org.apache.http.nio.entity.NFileEntity#<init>(java.io.File,java.lang.String,boolean)
+org.apache.http.entity.FileEntity#<init>(java.io.File,java.lang.String)
+org.apache.http.entity.StringEntity#<init>(java.lang.String,java.lang.String,java.lang.String)
+
+@defaultMessage BasicEntity is easy to mess up and forget to set content type
+org.apache.http.entity.BasicHttpEntity#<init>()
+
+@defaultMessage EntityTemplate is easy to mess up and forget to set content type
+org.apache.http.entity.EntityTemplate#<init>(org.apache.http.entity.ContentProducer)
+
+@defaultMessage SerializableEntity uses java serialization and makes it easy to forget to set content type
+org.apache.http.entity.SerializableEntity#<init>(java.io.Serializable)

client/rest-high-level/build.gradle

@@ -1,3 +1,5 @@
+import org.elasticsearch.gradle.precommit.PrecommitTasks
+
 /*
  * Licensed to Elasticsearch under one or more contributor
  * license agreements. See the NOTICE file distributed with
@@ -39,3 +41,9 @@ dependencyLicenses {
     it.group.startsWith('org.elasticsearch') == false
   }
 }
+
+forbiddenApisMain {
+  // core does not depend on the httpclient for compile so we add the signatures here. We don't add them for test as they are already
+  // specified
+  signaturesURLs += [PrecommitTasks.getResource('/forbidden/http-signatures.txt')]
+}

client/rest-high-level/src/test/java/org/elasticsearch/client/RestHighLevelClientTests.java

@@ -27,7 +27,6 @@
 import org.apache.http.ProtocolVersion;
 import org.apache.http.RequestLine;
 import org.apache.http.StatusLine;
-import org.apache.http.entity.BasicHttpEntity;
 import org.apache.http.entity.ByteArrayEntity;
 import org.apache.http.entity.ContentType;
 import org.apache.http.entity.StringEntity;
@@ -144,7 +143,7 @@ public void testParseEntity() throws IOException {
         }
         {
             IllegalStateException ise = expectThrows(IllegalStateException.class,
-                    () -> RestHighLevelClient.parseEntity(new BasicHttpEntity(), null));
+                    () -> RestHighLevelClient.parseEntity(new StringEntity("", (ContentType) null), null));
             assertEquals("Elasticsearch didn't return the [Content-Type] header, unable to parse response body", ise.getMessage());
         }
         {

client/rest/build.gradle

@@ -49,16 +49,18 @@ dependencies {
 }
 
 forbiddenApisMain {
-  //client does not depend on core, so only jdk signatures should be checked
-  signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt')]
+  //client does not depend on core, so only jdk and http signatures should be checked
+  signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt'),
+                    PrecommitTasks.getResource('/forbidden/http-signatures.txt')]
 }
 
 forbiddenApisTest {
   //we are using jdk-internal instead of jdk-non-portable to allow for com.sun.net.httpserver.* usage
   bundledSignatures -= 'jdk-non-portable'
   bundledSignatures += 'jdk-internal'
   //client does not depend on core, so only jdk signatures should be checked
-  signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt')]
+  signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt'),
+                    PrecommitTasks.getResource('/forbidden/http-signatures.txt')]
 }
 
 dependencyLicenses {

client/rest/src/test/java/org/elasticsearch/client/HeapBufferedAsyncResponseConsumerTests.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.client;
 
 import org.apache.http.ContentTooLongException;
+import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 import org.apache.http.ProtocolVersion;
 import org.apache.http.StatusLine;
@@ -32,6 +33,8 @@
 import org.apache.http.nio.IOControl;
 import org.apache.http.protocol.HttpContext;
 
+import java.util.concurrent.atomic.AtomicReference;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
@@ -56,7 +59,7 @@ public void testResponseProcessing() throws Exception {
         ProtocolVersion protocolVersion = new ProtocolVersion("HTTP", 1, 1);
         StatusLine statusLine = new BasicStatusLine(protocolVersion, 200, "OK");
         HttpResponse httpResponse = new BasicHttpResponse(statusLine);
-        httpResponse.setEntity(new StringEntity("test"));
+        httpResponse.setEntity(new StringEntity("test", ContentType.TEXT_PLAIN));
 
         //everything goes well
         consumer.responseReceived(httpResponse);
@@ -99,11 +102,17 @@ private static void bufferLimitTest(HeapBufferedAsyncResponseConsumer consumer,
         StatusLine statusLine = new BasicStatusLine(protocolVersion, 200, "OK");
         consumer.onResponseReceived(new BasicHttpResponse(statusLine));
 
-        BasicHttpEntity entity = new BasicHttpEntity();
-        entity.setContentLength(randomInt(bufferLimit));
+        final AtomicReference<Long> contentLength = new AtomicReference<>();
+        HttpEntity entity = new StringEntity("", ContentType.APPLICATION_JSON) {
+            @Override
+            public long getContentLength() {
+                return contentLength.get();
+            }
+        };
+        contentLength.set(randomLong(bufferLimit));
         consumer.onEntityEnclosed(entity, ContentType.APPLICATION_JSON);
 
-        entity.setContentLength(randomIntBetween(bufferLimit + 1, MAX_TEST_BUFFER_SIZE));
+        contentLength.set(randomLongBetween(bufferLimit + 1, MAX_TEST_BUFFER_SIZE));
         try {
             consumer.onEntityEnclosed(entity, ContentType.APPLICATION_JSON);
         } catch(ContentTooLongException e) {

client/rest/src/test/java/org/elasticsearch/client/RequestLoggerTests.java

@@ -31,6 +31,7 @@
 import org.apache.http.client.methods.HttpPut;
 import org.apache.http.client.methods.HttpTrace;
 import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.entity.ContentType;
 import org.apache.http.entity.InputStreamEntity;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.message.BasicHeader;
@@ -71,20 +72,21 @@ public void testTraceRequest() throws IOException, URISyntaxException {
             HttpEntity entity;
             switch(randomIntBetween(0, 4)) {
                 case 0:
-                    entity = new StringEntity(requestBody, StandardCharsets.UTF_8);
+                    entity = new StringEntity(requestBody, ContentType.APPLICATION_JSON);
                     break;
                 case 1:
-                    entity = new InputStreamEntity(new ByteArrayInputStream(requestBody.getBytes(StandardCharsets.UTF_8)));
+                    entity = new InputStreamEntity(new ByteArrayInputStream(requestBody.getBytes(StandardCharsets.UTF_8)),
+                        ContentType.APPLICATION_JSON);
                     break;
                 case 2:
-                    entity = new NStringEntity(requestBody, StandardCharsets.UTF_8);
+                    entity = new NStringEntity(requestBody, ContentType.APPLICATION_JSON);
                     break;
                 case 3:
-                    entity = new NByteArrayEntity(requestBody.getBytes(StandardCharsets.UTF_8));
+                    entity = new NByteArrayEntity(requestBody.getBytes(StandardCharsets.UTF_8), ContentType.APPLICATION_JSON);
                     break;
                 case 4:
                     // Evil entity without a charset
-                    entity = new StringEntity(requestBody, (Charset) null);
+                    entity = new StringEntity(requestBody, ContentType.create("application/json", (Charset) null));
                     break;
                 default:
                     throw new UnsupportedOperationException();
@@ -122,15 +124,16 @@ public void testTraceResponse() throws IOException {
             HttpEntity entity;
             switch(randomIntBetween(0, 2)) {
             case 0:
-                entity = new StringEntity(responseBody, StandardCharsets.UTF_8);
+                entity = new StringEntity(responseBody, ContentType.APPLICATION_JSON);
                 break;
             case 1:
                 //test a non repeatable entity
-                entity = new InputStreamEntity(new ByteArrayInputStream(responseBody.getBytes(StandardCharsets.UTF_8)));
+                entity = new InputStreamEntity(new ByteArrayInputStream(responseBody.getBytes(StandardCharsets.UTF_8)),
+                    ContentType.APPLICATION_JSON);
                 break;
             case 2:
                 // Evil entity without a charset
-                entity = new StringEntity(responseBody, (Charset) null);
+                entity = new StringEntity(responseBody, ContentType.create("application/json", (Charset) null));
                 break;
             default:
                 throw new UnsupportedOperationException();

client/rest/src/test/java/org/elasticsearch/client/ResponseExceptionTests.java

@@ -25,6 +25,7 @@
 import org.apache.http.ProtocolVersion;
 import org.apache.http.RequestLine;
 import org.apache.http.StatusLine;
+import org.apache.http.entity.ContentType;
 import org.apache.http.entity.InputStreamEntity;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.message.BasicHttpResponse;
@@ -52,10 +53,11 @@ public void testResponseException() throws IOException {
         if (hasBody) {
             HttpEntity entity;
             if (getRandom().nextBoolean()) {
-                entity = new StringEntity(responseBody, StandardCharsets.UTF_8);
+                entity = new StringEntity(responseBody, ContentType.APPLICATION_JSON);
             } else {
                 //test a non repeatable entity
-                entity = new InputStreamEntity(new ByteArrayInputStream(responseBody.getBytes(StandardCharsets.UTF_8)));
+                entity = new InputStreamEntity(new ByteArrayInputStream(responseBody.getBytes(StandardCharsets.UTF_8)),
+                    ContentType.APPLICATION_JSON);
             }
             httpResponse.setEntity(entity);
         }

client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostIntegTests.java

@@ -28,6 +28,7 @@
 import org.apache.http.HttpHost;
 import org.apache.http.auth.AuthScope;
 import org.apache.http.auth.UsernamePasswordCredentials;
+import org.apache.http.entity.ContentType;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.impl.client.BasicCredentialsProvider;
 import org.apache.http.impl.nio.client.HttpAsyncClientBuilder;
@@ -249,7 +250,7 @@ private Response bodyTest(final String method) throws IOException {
 
     private Response bodyTest(final RestClient restClient, final String method) throws IOException {
         String requestBody = "{ \"field\": \"value\" }";
-        StringEntity entity = new StringEntity(requestBody);
+        StringEntity entity = new StringEntity(requestBody, ContentType.APPLICATION_JSON);
         int statusCode = randomStatusCode(getRandom());
         Response esResponse;
         try {

client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostTests.java

@@ -38,6 +38,7 @@
 import org.apache.http.client.utils.URIBuilder;
 import org.apache.http.concurrent.FutureCallback;
 import org.apache.http.conn.ConnectTimeoutException;
+import org.apache.http.entity.ContentType;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.impl.auth.BasicScheme;
 import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
@@ -293,7 +294,7 @@ public void testIOExceptions() throws IOException {
      */
     public void testBody() throws IOException {
         String body = "{ \"field\": \"value\" }";
-        StringEntity entity = new StringEntity(body);
+        StringEntity entity = new StringEntity(body, ContentType.APPLICATION_JSON);
         for (String method : Arrays.asList("DELETE", "GET", "PATCH", "POST", "PUT")) {
             for (int okStatusCode : getOkStatusCodes()) {
                 Response response = restClient.performRequest(method, "/" + okStatusCode, Collections.<String, String>emptyMap(), entity);
@@ -431,7 +432,7 @@ private HttpUriRequest performRandomRequest(String method) throws Exception {
         HttpEntity entity = null;
         boolean hasBody = request instanceof HttpEntityEnclosingRequest && getRandom().nextBoolean();
         if (hasBody) {
-            entity = new StringEntity(randomAsciiOfLengthBetween(10, 100));
+            entity = new StringEntity(randomAsciiOfLengthBetween(10, 100), ContentType.APPLICATION_JSON);
             ((HttpEntityEnclosingRequest) request).setEntity(entity);
         }
 

core/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutRepositoryRequest.java

@@ -139,19 +139,6 @@ public PutRepositoryRequest settings(Settings.Builder settings) {
         return this;
     }
 
-    /**
-     * Sets the repository settings.
-     *
-     * @param source repository settings in json or yaml format
-     * @return this request
-     * @deprecated use {@link #settings(String, XContentType)} to avoid content type auto-detection
-     */
-    @Deprecated
-    public PutRepositoryRequest settings(String source) {
-        this.settings = Settings.builder().loadFromSource(source).build();
-        return this;
-    }
-
     /**
      * Sets the repository settings.
      *

core/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutRepositoryRequestBuilder.java

@@ -89,19 +89,6 @@ public PutRepositoryRequestBuilder setSettings(Settings.Builder settings) {
         return this;
     }
 
-    /**
-     * Sets the repository settings in Json or Yaml format
-     *
-     * @param source repository settings
-     * @return this builder
-     * @deprecated use {@link #setSettings(String, XContentType)} instead to avoid content type auto detection
-     */
-    @Deprecated
-    public PutRepositoryRequestBuilder setSettings(String source) {
-        request.settings(source);
-        return this;
-    }
-
     /**
      * Sets the repository settings in Json or Yaml format
      *

core/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsRequest.java

@@ -81,16 +81,6 @@ public ClusterUpdateSettingsRequest transientSettings(Settings.Builder settings)
         return this;
     }
 
-    /**
-     * Sets the source containing the transient settings to be updated. They will not survive a full cluster restart
-     * @deprecated use {@link #transientSettings(String, XContentType)} to avoid content type detection
-     */
-    @Deprecated
-    public ClusterUpdateSettingsRequest transientSettings(String source) {
-        this.transientSettings = Settings.builder().loadFromSource(source).build();
-        return this;
-    }
-
     /**
      * Sets the source containing the transient settings to be updated. They will not survive a full cluster restart
      */
@@ -130,16 +120,6 @@ public ClusterUpdateSettingsRequest persistentSettings(Settings.Builder settings
         return this;
     }
 
-    /**
-     * Sets the source containing the persistent settings to be updated. They will get applied cross restarts
-     * @deprecated use {@link #persistentSettings(String, XContentType)} to avoid content type detection
-     */
-    @Deprecated
-    public ClusterUpdateSettingsRequest persistentSettings(String source) {
-        this.persistentSettings = Settings.builder().loadFromSource(source).build();
-        return this;
-    }
-
     /**
      * Sets the source containing the persistent settings to be updated. They will get applied cross restarts
      */

core/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsRequestBuilder.java

@@ -51,16 +51,6 @@ public ClusterUpdateSettingsRequestBuilder setTransientSettings(Settings.Builder
         return this;
     }
 
-    /**
-     * Sets the source containing the transient settings to be updated. They will not survive a full cluster restart
-     * @deprecated use {@link #setTransientSettings(String, XContentType)} to avoid content type detection
-     */
-    @Deprecated
-    public ClusterUpdateSettingsRequestBuilder setTransientSettings(String settings) {
-        request.transientSettings(settings);
-        return this;
-    }
-
     /**
      * Sets the source containing the transient settings to be updated. They will not survive a full cluster restart
      */
@@ -93,16 +83,6 @@ public ClusterUpdateSettingsRequestBuilder setPersistentSettings(Settings.Builde
         return this;
     }
 
-    /**
-     * Sets the source containing the persistent settings to be updated. They will get applied cross restarts
-     * @deprecated use {@link #setPersistentSettings(String, XContentType)} to avoid content type detection
-     */
-    @Deprecated
-    public ClusterUpdateSettingsRequestBuilder setPersistentSettings(String settings) {
-        request.persistentSettings(settings);
-        return this;
-    }
-
     /**
      * Sets the source containing the persistent settings to be updated. They will get applied cross restarts
      */