From 116871362bf41115bda3522d244112d5dae2fba1 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 31 Aug 2020 09:25:07 -0500 Subject: [PATCH 1/6] Issue #5214 - Replicating HEAD Content-Length failure for huge files Signed-off-by: Joakim Erdfelt --- .../jetty/webapp/HugeResourceTest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java index 4c75b95bd171..0faccefe1805 100644 --- a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java +++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java @@ -244,6 +244,26 @@ public void testDownload(String filename, long expectedSize) throws Exception } } + @ParameterizedTest + @MethodSource("staticFiles") + public void testHead(String filename, long expectedSize) throws Exception + { + URI destUri = server.getURI().resolve("/" + filename); + InputStreamResponseListener responseListener = new InputStreamResponseListener(); + + Request request = client.newRequest(destUri) + .method(HttpMethod.HEAD); + request.send(responseListener); + Response response = responseListener.get(5, TimeUnit.SECONDS); + + assertThat("HTTP Response Code", response.getStatus(), is(200)); + // dumpResponse(response); + + String contentLength = response.getHeaders().get(HttpHeader.CONTENT_LENGTH); + long contentLengthLong = Long.parseLong(contentLength); + assertThat("Http Response Header: \"Content-Length: " + contentLength + "\"", contentLengthLong, is(expectedSize)); + } + @ParameterizedTest @MethodSource("staticFiles") public void testUpload(String filename, long expectedSize) throws Exception From eba360f66247784057c452c7e944b6f53575d9b9 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 31 Aug 2020 09:43:04 -0500 Subject: [PATCH 2/6] Issue #5214 - Use known content_length when in bypass write + In the case of HEAD, the servlet-api response is a wrapper of javax.servlet.http.HttpServlet$NoBodyResponse We know the content_length, use it. Signed-off-by: Joakim Erdfelt --- .../src/main/java/org/eclipse/jetty/server/ResourceService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java index 94bf5462e421..6ad0df3534c3 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java @@ -677,7 +677,7 @@ protected boolean sendData(HttpServletRequest request, else if (written || !(out instanceof HttpOutput)) { // write normally - putHeaders(response, content, written ? -1 : 0); + putHeaders(response, content, content_length); ByteBuffer buffer = content.getIndirectBuffer(); if (buffer != null) BufferUtil.writeTo(buffer, out); From dcb06d3c35c26e634c173d4fbcad20e6289128d7 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 31 Aug 2020 11:03:49 -0500 Subject: [PATCH 3/6] Issue #5214 - Use known content_length when in HEAD mode + Adding DefaultServlet.doHead() to avoid servlet wrapping + Making ResourceService HEAD aware Signed-off-by: Joakim Erdfelt --- .../eclipse/jetty/server/ResourceService.java | 12 +++++++++++- .../eclipse/jetty/servlet/DefaultServlet.java | 16 ++++++++++++---- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java index 6ad0df3534c3..0f356eb5c0a6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java @@ -673,11 +673,16 @@ protected boolean sendData(HttpServletRequest request, // write without headers content.getResource().writeTo(out, 0, content_length); } + // we are working with a HEAD request + else if (isHead(request) && content_length > 0) + { + putHeaders(response, content, content_length); + } // else if we can't do a bypass write because of wrapping else if (written || !(out instanceof HttpOutput)) { // write normally - putHeaders(response, content, content_length); + putHeaders(response, content, written ? -1 : 0); ByteBuffer buffer = content.getIndirectBuffer(); if (buffer != null) BufferUtil.writeTo(buffer, out); @@ -846,6 +851,11 @@ protected void putHeaders(HttpServletResponse response, HttpContent content, lon } } + private boolean isHead(HttpServletRequest request) + { + return "HEAD".equalsIgnoreCase(request.getMethod()); + } + public interface WelcomeFactory { diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java index c76d78958a54..f242949cf74f 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java @@ -462,20 +462,28 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response) doGet(request, response); } + @Override + protected void doHead(HttpServletRequest request, HttpServletResponse response) + throws ServletException, IOException + { + doGet(request, response); + } + /* (non-Javadoc) * @see javax.servlet.http.HttpServlet#doTrace(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse) */ @Override - protected void doTrace(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + protected void doTrace(HttpServletRequest request, HttpServletResponse response) + throws ServletException, IOException { - resp.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED); + response.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED); } @Override - protected void doOptions(HttpServletRequest req, HttpServletResponse resp) + protected void doOptions(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - resp.setHeader("Allow", "GET,HEAD,POST,OPTIONS"); + response.setHeader("Allow", "GET,HEAD,POST,OPTIONS"); } /* From 9128fc48e4921b72441f3c909151f7b5591cef3f Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 31 Aug 2020 11:16:22 -0500 Subject: [PATCH 4/6] Cleanup dead code in ResourceService Signed-off-by: Joakim Erdfelt --- .../java/org/eclipse/jetty/server/ResourceService.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java index 0f356eb5c0a6..d61e0b237770 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java @@ -578,7 +578,7 @@ protected boolean passConditionalHeaders(HttpServletRequest request, HttpServlet { //Get jetty's Response impl String mdlm = content.getLastModifiedValue(); - if (mdlm != null && ifms.equals(mdlm)) + if (ifms.equals(mdlm)) { sendStatus(response, HttpServletResponse.SC_NOT_MODIFIED, content::getETagValue); return false; @@ -679,10 +679,10 @@ else if (isHead(request) && content_length > 0) putHeaders(response, content, content_length); } // else if we can't do a bypass write because of wrapping - else if (written || !(out instanceof HttpOutput)) + else if (written) { // write normally - putHeaders(response, content, written ? -1 : 0); + putHeaders(response, content, -1); ByteBuffer buffer = content.getIndirectBuffer(); if (buffer != null) BufferUtil.writeTo(buffer, out); @@ -769,7 +769,7 @@ public String toString() // content-length header // putHeaders(response, content, -1); - String mimetype = (content == null ? null : content.getContentTypeValue()); + String mimetype = content.getContentTypeValue(); if (mimetype == null) LOG.warn("Unknown mimetype for " + request.getRequestURI()); MultiPartOutputStream multi = new MultiPartOutputStream(out); From 9e326fe5202e060a5f1726f093ae9db31c8dc203 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 31 Aug 2020 18:19:11 +0200 Subject: [PATCH 5/6] + improved sentinels values for content-length Signed-off-by: Greg Wilkins --- .../eclipse/jetty/server/ResourceService.java | 20 +++++-------------- .../org/eclipse/jetty/server/Response.java | 10 ++++++---- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java index 0f356eb5c0a6..4b97070b93ac 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java @@ -673,16 +673,11 @@ protected boolean sendData(HttpServletRequest request, // write without headers content.getResource().writeTo(out, 0, content_length); } - // we are working with a HEAD request - else if (isHead(request) && content_length > 0) - { - putHeaders(response, content, content_length); - } // else if we can't do a bypass write because of wrapping - else if (written || !(out instanceof HttpOutput)) + else if (written) { // write normally - putHeaders(response, content, written ? -1 : 0); + putHeaders(response, content, Response.NO_CONTENT_LENGTH); ByteBuffer buffer = content.getIndirectBuffer(); if (buffer != null) BufferUtil.writeTo(buffer, out); @@ -693,7 +688,7 @@ else if (written || !(out instanceof HttpOutput)) else { // write the headers - putHeaders(response, content, 0); + putHeaders(response, content, Response.USE_KNOWN_CONTENT_LENGTH); // write the content asynchronously if supported if (request.isAsyncSupported() && content.getContentLengthValue() > response.getBufferSize()) @@ -741,7 +736,7 @@ public String toString() // if there are no satisfiable ranges, send 416 response if (ranges == null || ranges.size() == 0) { - putHeaders(response, content, 0); + putHeaders(response, content, Response.USE_KNOWN_CONTENT_LENGTH); response.setHeader(HttpHeader.CONTENT_RANGE.asString(), InclusiveByteRange.to416HeaderRangeString(content_length)); sendStatus(response, HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE, null); @@ -768,7 +763,7 @@ public String toString() // 216 response which does not require an overall // content-length header // - putHeaders(response, content, -1); + putHeaders(response, content, Response.NO_CONTENT_LENGTH); String mimetype = (content == null ? null : content.getContentTypeValue()); if (mimetype == null) LOG.warn("Unknown mimetype for " + request.getRequestURI()); @@ -851,11 +846,6 @@ protected void putHeaders(HttpServletResponse response, HttpContent content, lon } } - private boolean isHead(HttpServletRequest request) - { - return "HEAD".equalsIgnoreCase(request.getMethod()); - } - public interface WelcomeFactory { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index 27c71750ad59..7d82d5318097 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -71,6 +71,8 @@ public class Response implements HttpServletResponse private static final Logger LOG = Log.getLogger(Response.class); private static final int __MIN_BUFFER_SIZE = 1; private static final HttpField __EXPIRES_01JAN1970 = new PreEncodedHttpField(HttpHeader.EXPIRES, DateGenerator.__01Jan1970); + public static final int NO_CONTENT_LENGTH = -1; + public static final int USE_KNOWN_CONTENT_LENGTH = -2; public enum OutputType { @@ -1289,12 +1291,12 @@ public void putHeaders(HttpContent content, long contentLength, boolean etag) if (lm != null) _fields.put(lm); - if (contentLength == 0) + if (contentLength == USE_KNOWN_CONTENT_LENGTH) { _fields.put(content.getContentLength()); _contentLength = content.getContentLengthValue(); } - else if (contentLength > 0) + else if (contentLength > NO_CONTENT_LENGTH) { _fields.putLongField(HttpHeader.CONTENT_LENGTH, contentLength); _contentLength = contentLength; @@ -1337,9 +1339,9 @@ public static void putHeaders(HttpServletResponse response, HttpContent content, if (lml >= 0) response.setDateHeader(HttpHeader.LAST_MODIFIED.asString(), lml); - if (contentLength == 0) + if (contentLength == USE_KNOWN_CONTENT_LENGTH) contentLength = content.getContentLengthValue(); - if (contentLength >= 0) + if (contentLength > NO_CONTENT_LENGTH) { if (contentLength < Integer.MAX_VALUE) response.setContentLength((int)contentLength); From 418093af28df8627301d6aee8d23dc843265367c Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 2 Sep 2020 13:01:20 +0200 Subject: [PATCH 6/6] Feedback from review: + check no content in HEAD response --- .../test/java/org/eclipse/jetty/webapp/HugeResourceTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java index 0faccefe1805..d530cd8a4f6d 100644 --- a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java +++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/HugeResourceTest.java @@ -256,6 +256,11 @@ public void testHead(String filename, long expectedSize) throws Exception request.send(responseListener); Response response = responseListener.get(5, TimeUnit.SECONDS); + try (InputStream in = responseListener.getInputStream()) + { + assertThat(in.read(), is(-1)); + } + assertThat("HTTP Response Code", response.getStatus(), is(200)); // dumpResponse(response);