1
0
Эх сурвалжийг харах

Forbid direct usage of ContentType.create() methods (#26457)

It's easy to create a wrong Content-Type header when converting a 
XContentType to a Apache HTTP ContentType instance.

This commit the direct usages of ContentType.create() methods in favor of a Request.createContentType(XContentType) method that does the right thing.

Closes #26438
Tanguy Leroux 8 жил өмнө
parent
commit
ad355f3e0b

+ 1 - 0
client/rest-high-level/build.gradle

@@ -59,4 +59,5 @@ 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')]
+  signaturesURLs += [file('src/main/resources/forbidden/rest-high-level-signatures.txt').toURI().toURL()]
 }

+ 17 - 4
client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java

@@ -42,6 +42,7 @@ import org.elasticsearch.action.support.WriteRequest;
 import org.elasticsearch.action.update.UpdateRequest;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.lucene.uid.Versions;
 import org.elasticsearch.common.unit.TimeValue;
@@ -57,6 +58,7 @@ import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.nio.charset.Charset;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Locale;
@@ -139,8 +141,8 @@ final class Request {
             bulkContentType = XContentType.JSON;
         }
 
-        byte separator = bulkContentType.xContent().streamSeparator();
-        ContentType requestContentType = ContentType.create(bulkContentType.mediaType());
+        final byte separator = bulkContentType.xContent().streamSeparator();
+        final ContentType requestContentType = createContentType(bulkContentType);
 
         ByteArrayOutputStream content = new ByteArrayOutputStream();
         for (DocWriteRequest<?> request : bulkRequest.requests()) {
@@ -268,7 +270,7 @@ final class Request {
         parameters.withWaitForActiveShards(indexRequest.waitForActiveShards());
 
         BytesRef source = indexRequest.source().toBytesRef();
-        ContentType contentType = ContentType.create(indexRequest.getContentType().mediaType());
+        ContentType contentType = createContentType(indexRequest.getContentType());
         HttpEntity entity = new ByteArrayEntity(source.bytes, source.offset, source.length, contentType);
 
         return new Request(method, endpoint, parameters.getParams(), entity);
@@ -352,7 +354,7 @@ final class Request {
 
     private static HttpEntity createEntity(ToXContent toXContent, XContentType xContentType) throws IOException {
         BytesRef source = XContentHelper.toXContent(toXContent, xContentType, false).toBytesRef();
-        return new ByteArrayEntity(source.bytes, source.offset, source.length, ContentType.create(xContentType.mediaType()));
+        return new ByteArrayEntity(source.bytes, source.offset, source.length, createContentType(xContentType));
     }
 
     static String endpoint(String[] indices, String[] types, String endpoint) {
@@ -372,6 +374,17 @@ final class Request {
         return joiner.toString();
     }
 
+    /**
+     * Returns a {@link ContentType} from a given {@link XContentType}.
+     *
+     * @param xContentType the {@link XContentType}
+     * @return the {@link ContentType}
+     */
+    @SuppressForbidden(reason = "Only allowed place to convert a XContentType to a ContentType")
+    static ContentType createContentType(final XContentType xContentType) {
+        return ContentType.create(xContentType.mediaTypeWithoutParameters(), (Charset) null);
+    }
+
     /**
      * Utility class to build request's parameters map and centralize all parameter names.
      */

+ 21 - 0
client/rest-high-level/src/main/resources/forbidden/rest-high-level-signatures.txt

@@ -0,0 +1,21 @@
+# 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 Use Request#createContentType(XContentType) to be sure to pass the right MIME type
+org.apache.http.entity.ContentType#create(java.lang.String)
+org.apache.http.entity.ContentType#create(java.lang.String,java.lang.String)
+org.apache.http.entity.ContentType#create(java.lang.String,java.nio.charset.Charset)
+org.apache.http.entity.ContentType#create(java.lang.String,org.apache.http.NameValuePair[])

+ 13 - 8
client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java

@@ -278,7 +278,7 @@ public class RequestTests extends ESTestCase {
 
         HttpEntity entity = request.entity;
         assertTrue(entity instanceof ByteArrayEntity);
-        assertEquals(indexRequest.getContentType().mediaType(), entity.getContentType().getValue());
+        assertEquals(indexRequest.getContentType().mediaTypeWithoutParameters(), entity.getContentType().getValue());
         try (XContentParser parser = createParser(xContentType.xContent(), entity.getContent())) {
             assertEquals(nbFields, parser.map().size());
         }
@@ -488,7 +488,7 @@ public class RequestTests extends ESTestCase {
         assertEquals("/_bulk", request.endpoint);
         assertEquals(expectedParams, request.params);
         assertEquals("POST", request.method);
-        assertEquals(xContentType.mediaType(), request.entity.getContentType().getValue());
+        assertEquals(xContentType.mediaTypeWithoutParameters(), request.entity.getContentType().getValue());
         byte[] content = new byte[(int) request.entity.getContentLength()];
         try (InputStream inputStream = request.entity.getContent()) {
             Streams.readFully(inputStream, content);
@@ -541,7 +541,7 @@ public class RequestTests extends ESTestCase {
             bulkRequest.add(new DeleteRequest("index", "type", "2"));
 
             Request request = Request.bulk(bulkRequest);
-            assertEquals(XContentType.JSON.mediaType(), request.entity.getContentType().getValue());
+            assertEquals(XContentType.JSON.mediaTypeWithoutParameters(), request.entity.getContentType().getValue());
         }
         {
             XContentType xContentType = randomFrom(XContentType.JSON, XContentType.SMILE);
@@ -551,7 +551,7 @@ public class RequestTests extends ESTestCase {
             bulkRequest.add(new DeleteRequest("index", "type", "2"));
 
             Request request = Request.bulk(bulkRequest);
-            assertEquals(xContentType.mediaType(), request.entity.getContentType().getValue());
+            assertEquals(xContentType.mediaTypeWithoutParameters(), request.entity.getContentType().getValue());
         }
         {
             XContentType xContentType = randomFrom(XContentType.JSON, XContentType.SMILE);
@@ -563,7 +563,7 @@ public class RequestTests extends ESTestCase {
             }
 
             Request request = Request.bulk(new BulkRequest().add(updateRequest));
-            assertEquals(xContentType.mediaType(), request.entity.getContentType().getValue());
+            assertEquals(xContentType.mediaTypeWithoutParameters(), request.entity.getContentType().getValue());
         }
         {
             BulkRequest bulkRequest = new BulkRequest();
@@ -732,7 +732,7 @@ public class RequestTests extends ESTestCase {
         assertEquals("/_search/scroll", request.endpoint);
         assertEquals(0, request.params.size());
         assertToXContentBody(searchScrollRequest, request.entity);
-        assertEquals(Request.REQUEST_BODY_CONTENT_TYPE.mediaType(), request.entity.getContentType().getValue());
+        assertEquals(Request.REQUEST_BODY_CONTENT_TYPE.mediaTypeWithoutParameters(), request.entity.getContentType().getValue());
     }
 
     public void testClearScroll() throws IOException {
@@ -746,12 +746,12 @@ public class RequestTests extends ESTestCase {
         assertEquals("/_search/scroll", request.endpoint);
         assertEquals(0, request.params.size());
         assertToXContentBody(clearScrollRequest, request.entity);
-        assertEquals(Request.REQUEST_BODY_CONTENT_TYPE.mediaType(), request.entity.getContentType().getValue());
+        assertEquals(Request.REQUEST_BODY_CONTENT_TYPE.mediaTypeWithoutParameters(), request.entity.getContentType().getValue());
     }
 
     private static void assertToXContentBody(ToXContent expectedBody, HttpEntity actualEntity) throws IOException {
         BytesReference expectedBytes = XContentHelper.toXContent(expectedBody, Request.REQUEST_BODY_CONTENT_TYPE, false);
-        assertEquals(XContentType.JSON.mediaType(), actualEntity.getContentType().getValue());
+        assertEquals(XContentType.JSON.mediaTypeWithoutParameters(), actualEntity.getContentType().getValue());
         assertEquals(expectedBytes, new BytesArray(EntityUtils.toByteArray(actualEntity)));
     }
 
@@ -793,6 +793,11 @@ public class RequestTests extends ESTestCase {
         assertEquals("/a/_create", Request.endpoint("a", null, null, "_create"));
     }
 
+    public void testCreateContentType() {
+        final XContentType xContentType = randomFrom(XContentType.values());
+        assertEquals(xContentType.mediaTypeWithoutParameters(), Request.createContentType(xContentType).getMimeType());
+    }
+
     public void testEnforceSameContentType() {
         XContentType xContentType = randomFrom(XContentType.JSON, XContentType.SMILE);
         IndexRequest indexRequest = new IndexRequest().source(singletonMap("field", "value"), xContentType);