Fix BZ 66442 - h2 responses without bodies should not use DATA frames

https://bz.apache.org/bugzilla/show_bug.cgi?id=66442
1210xx · Jan 25, 2023 · 519403f · 519403f
1 parent aaff739
commit 519403f
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 2 deletions.
diff --git a/java/org/apache/coyote/http2/Stream.java b/java/org/apache/coyote/http2/Stream.java
@@ -335,8 +335,7 @@ public final void emitHeader(String name, String value) throws HpackException {
                 if (coyoteRequest.method().isNull()) {
                     coyoteRequest.method().setString(value);
                     if ("HEAD".equals(value)) {
-                        addOutputFilter(new VoidOutputFilter());
-                        streamOutputBuffer.closed = true;
+                        configureVoidOutputFilter();
                     }
                 } else {
                     throw new HpackException(
@@ -448,6 +447,12 @@ public final void emitHeader(String name, String value) throws HpackException {
     }
 
 
+    void configureVoidOutputFilter() {
+        addOutputFilter(new VoidOutputFilter());
+        // Prevent further writes by the application
+        streamOutputBuffer.closed = true;
+    }
+
     private void parseAuthority(String value, boolean host) throws HpackException {
         int i;
         try {

diff --git a/java/org/apache/coyote/http2/StreamProcessor.java b/java/org/apache/coyote/http2/StreamProcessor.java
@@ -205,6 +205,10 @@ static void prepareHeaders(Request coyoteRequest, Response coyoteResponse, boole
                 headers.addValue("content-length").setLong(contentLength);
             }
         } else {
+            // Disable response body
+            if (stream != null) {
+                stream.configureVoidOutputFilter();
+            }
             if (statusCode == 205) {
                 // RFC 7231 requires the server to explicitly signal an empty
                 // response in this case

diff --git a/test/org/apache/coyote/http2/Http2TestBase.java b/test/org/apache/coyote/http2/Http2TestBase.java
@@ -1339,6 +1339,18 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
     }
 
 
+    public static class NoContentServlet extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+            resp.setStatus(HttpServletResponse.SC_NO_CONTENT);
+        }
+    }
+
+
     public static class SimpleServlet extends HttpServlet {
 
         private static final long serialVersionUID = 1L;

diff --git a/test/org/apache/coyote/http2/TestStreamProcessor.java b/test/org/apache/coyote/http2/TestStreamProcessor.java
@@ -164,6 +164,55 @@ public void testPrepareHeaders() throws Exception {
         }
         expected.append("3-Header-[date]-[Wed, 11 Nov 2015 19:18:42 GMT]\n");
         expected.append("3-HeadersEnd\n");
+        expected.append("3-EndOfStream\n");
+
+        Assert.assertEquals(expected.toString(), output.getTrace());
+    }
+
+
+    @Test
+    public void testPrepareHeadersNoContent() throws Exception {
+        enableHttp2();
+
+        Tomcat tomcat = getTomcatInstance();
+
+        File appDir = new File("test/webapp");
+        Context ctxt = tomcat.addWebapp(null, "", appDir.getAbsolutePath());
+
+        Tomcat.addServlet(ctxt, "simple", new SimpleServlet());
+        ctxt.addServletMappingDecoded("/simple", "simple");
+        Tomcat.addServlet(ctxt, "noContent", new NoContentServlet());
+        ctxt.addServletMappingDecoded("/noContent", "noContent");
+
+
+        tomcat.start();
+
+        openClientConnection();
+        doHttpUpgrade();
+        sendClientPreface();
+        validateHttp2InitialResponse();
+
+        byte[] frameHeader = new byte[9];
+        ByteBuffer headersPayload = ByteBuffer.allocate(128);
+
+        List<Header> headers = new ArrayList<>(3);
+        headers.add(new Header(":method", "GET"));
+        headers.add(new Header(":scheme", "http"));
+        headers.add(new Header(":path", "/noContent"));
+        headers.add(new Header(":authority", "localhost:" + getPort()));
+
+        buildGetRequest(frameHeader, headersPayload, null, headers, 3);
+
+        writeFrame(frameHeader, headersPayload);
+
+        parser.readFrame();
+
+        StringBuilder expected = new StringBuilder();
+        expected.append("3-HeadersStart\n");
+        expected.append("3-Header-[:status]-[204]\n");
+        expected.append("3-Header-[date]-[Wed, 11 Nov 2015 19:18:42 GMT]\n");
+        expected.append("3-HeadersEnd\n");
+        expected.append("3-EndOfStream\n");
 
         Assert.assertEquals(expected.toString(), output.getTrace());
     }

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
@@ -151,6 +151,11 @@
         Log basic information for each configured TLS certificate when Tomcat
         starts. (markt)
       </add>
+      <fix>
+        <bug>66442</bug>: When an HTTP/2 response must not include a body,
+        ensure that the end of stream flag is set on the headers frame and that
+        no data frame is sent. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">