From a2b78e0eff7540b818dbcbd22b9b9e143457d1e6 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 29 Nov 2018 11:45:45 -0600 Subject: [PATCH] Support content type `application/x-ndjson` in DeprecationRestHandler (#36025) org.elasticsearch.rest.RestController#hasContentType checks to see if the RestHandler supports the `application/x-ndjson` Content-Type. DeprecationRestHandler is a wrapper around the real RestHandler, and prior to this change would always return `false` due to the interface's default supportsContentStream(). This prevents API's that use multi-line JSON from properly being deprecated resulting in an HTTP 406 error. This change ensures that the DeprecationRestHandler honors the supportsContentStream() of the wrapped RestHandler. Relates to #35958 --- .../rest/DeprecationRestHandler.java | 5 +++++ .../rest/DeprecationRestHandlerTests.java | 22 +++++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/DeprecationRestHandler.java b/server/src/main/java/org/elasticsearch/rest/DeprecationRestHandler.java index 1a78a4c8c0452..2408f83d2b54e 100644 --- a/server/src/main/java/org/elasticsearch/rest/DeprecationRestHandler.java +++ b/server/src/main/java/org/elasticsearch/rest/DeprecationRestHandler.java @@ -62,6 +62,11 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c handler.handleRequest(request, channel, client); } + @Override + public boolean supportsContentStream() { + return handler.supportsContentStream(); + } + /** * This does a very basic pass at validating that a header's value contains only expected characters according to RFC-5987, and those * that it references. diff --git a/server/src/test/java/org/elasticsearch/rest/DeprecationRestHandlerTests.java b/server/src/test/java/org/elasticsearch/rest/DeprecationRestHandlerTests.java index 0a8ace71d8860..0dc108ff587cd 100644 --- a/server/src/test/java/org/elasticsearch/rest/DeprecationRestHandlerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/DeprecationRestHandlerTests.java @@ -24,22 +24,30 @@ import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.test.ESTestCase; +import org.junit.Before; import org.mockito.InOrder; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; /** * Tests {@link DeprecationRestHandler}. */ public class DeprecationRestHandlerTests extends ESTestCase { - private final RestHandler handler = mock(RestHandler.class); + private RestHandler handler; /** * Note: Headers should only use US ASCII (and this inevitably becomes one!). */ private final String deprecationMessage = randomAlphaOfLengthBetween(1, 30); - private final DeprecationLogger deprecationLogger = mock(DeprecationLogger.class); + private DeprecationLogger deprecationLogger; + + @Before + public void setup() { + handler = mock(RestHandler.class); + deprecationLogger = mock(DeprecationLogger.class); + } public void testNullHandler() { expectThrows(NullPointerException.class, () -> new DeprecationRestHandler(null, deprecationMessage, deprecationLogger)); @@ -114,6 +122,16 @@ public void testInvalidHeaderValueEmpty() { expectThrows(IllegalArgumentException.class, () -> DeprecationRestHandler.requireValidHeader(blank)); } + public void testSupportsContentStreamTrue() { + when(handler.supportsContentStream()).thenReturn(true); + assertTrue(new DeprecationRestHandler(handler, deprecationMessage, deprecationLogger).supportsContentStream()); + } + + public void testSupportsContentStreamFalse() { + when(handler.supportsContentStream()).thenReturn(false); + assertFalse(new DeprecationRestHandler(handler, deprecationMessage, deprecationLogger).supportsContentStream()); + } + /** * {@code ASCIIHeaderGenerator} only uses characters expected to be valid in headers (simplified US-ASCII). */