Add correct Content-Length on HEAD requests

This commit fixes responses to HEAD requests so that the value of the
Content-Length is correct per the HTTP spec. Namely, the value of this
header should be equal to the Content-Length if the request were not a
HEAD request.

This commit also fixes a memory leak on HEAD requests to the main action
that arose from the bytes on a builder not being released due to them
being dropped on the floor to ensure that the response to the main
action did not have a body.

Relates #21123

core/src/main/java/org/elasticsearch/rest/action/RestMainAction.java

@@ -60,9 +60,6 @@ public class RestMainAction extends BaseRestHandler {
 
     static BytesRestResponse convertMainResponse(MainResponse response, RestRequest request, XContentBuilder builder) throws IOException {
         RestStatus status = response.isAvailable() ? RestStatus.OK : RestStatus.SERVICE_UNAVAILABLE;
-        if (request.method() == RestRequest.Method.HEAD) {
-            return new BytesRestResponse(status, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY);
-        }
 
         // Default to pretty printing, but allow ?pretty=false to disable
         if (request.hasParam("pretty") == false) {

core/src/test/java/org/elasticsearch/rest/action/RestMainActionTests.java

@@ -61,9 +61,9 @@ public class RestMainActionTests extends ESTestCase {
         BytesRestResponse response = RestMainAction.convertMainResponse(mainResponse, restRequest, builder);
         assertNotNull(response);
         assertEquals(expectedStatus, response.status());
-        assertEquals(0, response.content().length());
 
-        assertEquals(0, builder.bytes().length());
+        // the empty responses are handled in the HTTP layer so we do
+        // not assert on them here
     }
 
     public void testGetResponse() throws Exception {

modules/transport-netty3/src/main/java/org/elasticsearch/http/netty3/Netty3HttpChannel.java

@@ -33,6 +33,7 @@ import org.elasticsearch.rest.AbstractRestChannel;
 import org.elasticsearch.rest.RestResponse;
 import org.elasticsearch.rest.RestStatus;
 import org.jboss.netty.buffer.ChannelBuffer;
+import org.jboss.netty.buffer.ChannelBuffers;
 import org.jboss.netty.channel.Channel;
 import org.jboss.netty.channel.ChannelFuture;
 import org.jboss.netty.channel.ChannelFutureListener;
@@ -41,6 +42,7 @@ import org.jboss.netty.handler.codec.http.CookieDecoder;
 import org.jboss.netty.handler.codec.http.CookieEncoder;
 import org.jboss.netty.handler.codec.http.DefaultHttpResponse;
 import org.jboss.netty.handler.codec.http.HttpHeaders;
+import org.jboss.netty.handler.codec.http.HttpMethod;
 import org.jboss.netty.handler.codec.http.HttpResponse;
 import org.jboss.netty.handler.codec.http.HttpResponseStatus;
 import org.jboss.netty.handler.codec.http.HttpVersion;
@@ -109,7 +111,11 @@ public final class Netty3HttpChannel extends AbstractRestChannel {
         boolean addedReleaseListener = false;
         try {
             buffer = Netty3Utils.toChannelBuffer(content);
+            if (HttpMethod.HEAD.equals(nettyRequest.getMethod())) {
+                resp.setContent(ChannelBuffers.EMPTY_BUFFER);
+            } else {
                 resp.setContent(buffer);
+            }
 
             // If our response doesn't specify a content-type header, set one
             setHeaderField(resp, HttpHeaders.Names.CONTENT_TYPE, response.contentType(), false);

modules/transport-netty3/src/test/java/org/elasticsearch/rest/Netty3HeadBodyIsEmptyIT.java

@@ -0,0 +1,23 @@
+/*
+ * 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.
+ */
+
+package org.elasticsearch.rest;
+
+public class Netty3HeadBodyIsEmptyIT extends HeadBodyIsEmptyIntegTestCase {
+}

modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpChannel.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.http.netty4;
 
 import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelFutureListener;
 import io.netty.channel.ChannelPromise;
@@ -29,6 +30,7 @@ import io.netty.handler.codec.http.FullHttpResponse;
 import io.netty.handler.codec.http.HttpHeaderNames;
 import io.netty.handler.codec.http.HttpHeaderValues;
 import io.netty.handler.codec.http.HttpHeaders;
+import io.netty.handler.codec.http.HttpMethod;
 import io.netty.handler.codec.http.HttpResponse;
 import io.netty.handler.codec.http.HttpResponseStatus;
 import io.netty.handler.codec.http.HttpVersion;
@@ -87,13 +89,17 @@ final class Netty4HttpChannel extends AbstractRestChannel {
         return new ReleasableBytesStreamOutput(transport.bigArrays);
     }
 
-
     @Override
     public void sendResponse(RestResponse response) {
         // if the response object was created upstream, then use it;
         // otherwise, create a new one
         ByteBuf buffer = Netty4Utils.toByteBuf(response.content());
-        FullHttpResponse resp = newResponse(buffer);
+        final FullHttpResponse resp;
+        if (HttpMethod.HEAD.equals(nettyRequest.method())) {
+            resp = newResponse(Unpooled.EMPTY_BUFFER);
+        } else {
+            resp = newResponse(buffer);
+        }
         resp.setStatus(getStatus(response.status()));
 
         Netty4CorsHandler.setCorsResponseHeaders(nettyRequest, resp, transport.getCorsConfig());

modules/transport-netty4/src/test/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java

@@ -0,0 +1,23 @@
+/*
+ * 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.
+ */
+
+package org.elasticsearch.rest;
+
+public class Netty4HeadBodyIsEmptyIT extends HeadBodyIsEmptyIntegTestCase {
+}

test/framework/src/main/java/org/elasticsearch/rest/HeadBodyIsEmptyIntegTestCase.java

@@ -17,10 +17,12 @@
  * under the License.
  */
 
-package org.elasticsearch.test.rest;
+package org.elasticsearch.rest;
 
 import org.apache.http.entity.StringEntity;
 import org.elasticsearch.client.Response;
+import org.elasticsearch.test.rest.ESRestTestCase;
+import org.hamcrest.Matcher;
 
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
@@ -28,15 +30,17 @@ import java.util.Map;
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonMap;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
 
 /**
  * Tests that HTTP HEAD requests don't respond with a body.
  */
-public class HeadBodyIsEmptyIT extends ESRestTestCase {
+public class HeadBodyIsEmptyIntegTestCase extends ESRestTestCase {
     public void testHeadRoot() throws IOException {
-        headTestCase("/", emptyMap());
+        headTestCase("/", emptyMap(), greaterThan(0));
-        headTestCase("/", singletonMap("pretty", ""));
+        headTestCase("/", singletonMap("pretty", ""), greaterThan(0));
-        headTestCase("/", singletonMap("pretty", "true"));
+        headTestCase("/", singletonMap("pretty", "true"), greaterThan(0));
     }
 
     private void createTestDoc() throws UnsupportedEncodingException, IOException {
@@ -45,28 +49,26 @@ public class HeadBodyIsEmptyIT extends ESRestTestCase {
 
     public void testDocumentExists() throws IOException {
         createTestDoc();
-        headTestCase("test/test/1", emptyMap());
+        headTestCase("test/test/1", emptyMap(), equalTo(0));
-        headTestCase("test/test/1", singletonMap("pretty", "true"));
+        headTestCase("test/test/1", singletonMap("pretty", "true"), equalTo(0));
     }
 
     public void testIndexExists() throws IOException {
         createTestDoc();
-        headTestCase("test", emptyMap());
+        headTestCase("test", emptyMap(), equalTo(0));
-        headTestCase("test", singletonMap("pretty", "true"));
+        headTestCase("test", singletonMap("pretty", "true"), equalTo(0));
     }
 
     public void testTypeExists() throws IOException {
         createTestDoc();
-        headTestCase("test/test", emptyMap());
+        headTestCase("test/test", emptyMap(), equalTo(0));
-        headTestCase("test/test", singletonMap("pretty", "true"));
+        headTestCase("test/test", singletonMap("pretty", "true"), equalTo(0));
     }
 
-    private void headTestCase(String url, Map<String, String> params) throws IOException {
+    private void headTestCase(String url, Map<String, String> params, Matcher<Integer> matcher) throws IOException {
         Response response = client().performRequest("HEAD", url, params);
         assertEquals(200, response.getStatusLine().getStatusCode());
-        /* Check that the content-length header is always 0. This isn't what we should be doing in the long run but it is what we expect
+        assertThat(Integer.valueOf(response.getHeader("Content-Length")), matcher);
-         * that we are *actually* doing. */
-        assertEquals("We expect HEAD requests to have 0 Content-Length but " + url + " didn't", "0", response.getHeader("Content-Length"));
         assertNull("HEAD requests shouldn't have a response body but " + url + " did", response.getEntity());
     }
 }