[grizzly~git:3122babe] [master] + fix issue #1666

  • From: oleksiys@...
  • To: commits@...
  • Subject: [grizzly~git:3122babe] [master] + fix issue #1666
  • Date: Fri, 21 Mar 2014 18:57:23 +0000

Project:    grizzly
Repository: git
Revision:   3122babee463ce89cf4251f5efea632aa3ba1ec4
Author:     oleksiys
Date:       2014-03-21 08:11:14 UTC
Link:       

Log Message:
------------
[master] + fix issue #1666
https://java.net/jira/browse/GRIZZLY-1666
"HTTP request is not fully consumed for persistent connection"



Revisions:
----------
3122babee463ce89cf4251f5efea632aa3ba1ec4


Modified Paths:
---------------
modules/http-server/src/main/java/org/glassfish/grizzly/http/server/HttpServer.java
modules/http-server/src/main/java/org/glassfish/grizzly/http/server/ServerConfiguration.java
modules/http-server/src/test/java/org/glassfish/grizzly/http/server/HttpInputStreamsTest.java
modules/http/src/main/java/org/glassfish/grizzly/http/HttpServerFilter.java
modules/http/src/main/java/org/glassfish/grizzly/http/Method.java
modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java
modules/http/src/test/java/org/glassfish/grizzly/http/HttpSemanticsTest.java


Diffs:
------
--- 
a/modules/http-server/src/main/java/org/glassfish/grizzly/http/server/HttpServer.java
+++ 
b/modules/http-server/src/main/java/org/glassfish/grizzly/http/server/HttpServer.java
@@ -651,7 +651,9 @@ public class HttpServer {
             for (ContentEncoding contentEncoding : contentEncodings) {
                 httpServerCodecFilter.addContentEncoding(contentEncoding);
             }
-
+            httpServerCodecFilter.setAllowPayloadForUndefinedHttpMethods(
+                    serverConfig.isAllowPayloadForUndefinedHttpMethods());
+            
             httpServerCodecFilter.getMonitoringConfig().addProbes(
                     
serverConfig.getMonitoringConfig().getHttpConfig().getProbes());
             builder.add(httpServerCodecFilter);--- 
a/modules/http-server/src/main/java/org/glassfish/grizzly/http/server/ServerConfiguration.java
+++ 
b/modules/http-server/src/main/java/org/glassfish/grizzly/http/server/ServerConfiguration.java
@@ -1,7 +1,7 @@
 /*
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
  *
- * Copyright (c) 2010-2013 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010-2014 Oracle and/or its affiliates. All rights reserved.
  *
  * The contents of this file are subject to the terms of either the GNU
  * General Public License Version 2 only ("GPL") or the Common Development
@@ -71,7 +71,7 @@ public class ServerConfiguration extends 
ServerFilterConfiguration {
     final List<HttpHandler> orderedHandlers =
             new LinkedList<HttpHandler>();
 
-    private Set<JmxEventListener> jmxEventListeners = new 
CopyOnWriteArraySet<JmxEventListener>();
+    private final Set<JmxEventListener> jmxEventListeners = new 
CopyOnWriteArraySet<JmxEventListener>();
 
     private final HttpServerMonitoringConfig monitoringConfig = new 
HttpServerMonitoringConfig();
 
@@ -81,6 +81,12 @@ public class ServerConfiguration extends 
ServerFilterConfiguration {
 
     private boolean jmxEnabled;
 
+    // flag, which enables/disables payload support for HTTP methods,
+    // for which HTTP spec doesn't clearly state whether they support 
payload.
+    // Known "undefined" methods are: GET, HEAD, DELETE
+    private boolean allowPayloadForUndefinedHttpMethods;
+    
+    
     final Object handlersSync = new Object();
     
     // ------------------------------------------------------------ 
Constructors
@@ -272,5 +278,29 @@ public class ServerConfiguration extends 
ServerFilterConfiguration {
         return jmxEventListeners;
         
     }
+    
+    /**
+     * The flag, which enables/disables payload support for HTTP methods,
+     * for which HTTP spec doesn't clearly state whether they support 
payload.
+     * Known "undefined" methods are: GET, HEAD, DELETE.
+     * 
+     * @return <tt>true</tt> if "undefined" methods support payload, or 
<tt>false</tt> otherwise
+     * @since 2.3.12
+     */
+    public boolean isAllowPayloadForUndefinedHttpMethods() {
+        return allowPayloadForUndefinedHttpMethods;
+    }
+
+    /**
+     * The flag, which enables/disables payload support for HTTP methods,
+     * for which HTTP spec doesn't clearly state whether they support 
payload.
+     * Known "undefined" methods are: GET, HEAD, DELETE.
+     * 
+     * @param allowPayloadForUndefinedHttpMethods <tt>true</tt> if 
"undefined" methods support payload, or <tt>false</tt> otherwise
+     * @since 2.3.12
+     */
+    public void setAllowPayloadForUndefinedHttpMethods(boolean 
allowPayloadForUndefinedHttpMethods) {
+        this.allowPayloadForUndefinedHttpMethods = 
allowPayloadForUndefinedHttpMethods;
+    }    
 
 } // END ServerConfiguration--- 
a/modules/http-server/src/test/java/org/glassfish/grizzly/http/server/HttpInputStreamsTest.java
+++ 
b/modules/http-server/src/test/java/org/glassfish/grizzly/http/server/HttpInputStreamsTest.java
@@ -1,7 +1,7 @@
 /*
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
  *
- * Copyright (c) 2010-2013 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010-2014 Oracle and/or its affiliates. All rights reserved.
  *
  * The contents of this file are subject to the terms of either the GNU
  * General Public License Version 2 only ("GPL") or the Common Development
@@ -77,6 +77,7 @@ import org.glassfish.grizzly.utils.Futures;
 
 import static junit.framework.Assert.assertEquals;
 import org.glassfish.grizzly.http.HttpHeader;
+import org.glassfish.grizzly.http.Method;
 import org.glassfish.grizzly.http.util.HeaderValue;
 
 /**
@@ -1254,7 +1255,8 @@ public class HttpInputStreamsTest extends TestCase {
                 .method(method)
                 .protocol(Protocol.HTTP_1_1)
                 .uri("/path")
-                .chunked(content == null)
+                .chunked(content == null &&
+                        Method.valueOf(method).getPayloadExpectation() == 
Method.PayloadExpectation.ALLOWED)
                 .header("Host", "localhost");
         if (content != null) {
             assert contentBuffer != null;--- 
a/modules/http/src/main/java/org/glassfish/grizzly/http/HttpServerFilter.java
+++ 
b/modules/http/src/main/java/org/glassfish/grizzly/http/HttpServerFilter.java
@@ -128,6 +128,11 @@ public class HttpServerFilter extends HttpCodecFilter {
     private int maxRequestHeaders;
     private int maxResponseHeaders;
     
+    // flag, which enables/disables payload support for HTTP methods,
+    // for which HTTP spec doesn't clearly state whether they support 
payload.
+    // Known "undefined" methods are: GET, HEAD, DELETE
+    private boolean allowPayloadForUndefinedHttpMethods;
+    
     /**
      * Constructor, which creates <tt>HttpServerFilter</tt> instance
      */
@@ -231,6 +236,32 @@ public class HttpServerFilter extends HttpCodecFilter {
         this.maxResponseHeaders = maxResponseHeaders;
     }
     
+    // ----------------------------------------------------------- 
Configuration
+    
+    /**
+     * The flag, which enables/disables payload support for HTTP methods,
+     * for which HTTP spec doesn't clearly state whether they support 
payload.
+     * Known "undefined" methods are: GET, HEAD, DELETE.
+     * 
+     * @return <tt>true</tt> if "undefined" methods support payload, or 
<tt>false</tt> otherwise
+     * @since 2.3.12
+     */
+    public boolean isAllowPayloadForUndefinedHttpMethods() {
+        return allowPayloadForUndefinedHttpMethods;
+    }
+
+    /**
+     * The flag, which enables/disables payload support for HTTP methods,
+     * for which HTTP spec doesn't clearly state whether they support 
payload.
+     * Known "undefined" methods are: GET, HEAD, DELETE.
+     * 
+     * @param allowPayloadForUndefinedHttpMethods <tt>true</tt> if 
"undefined" methods support payload, or <tt>false</tt> otherwise
+     * @since 2.3.12
+     */
+    public void setAllowPayloadForUndefinedHttpMethods(boolean 
allowPayloadForUndefinedHttpMethods) {
+        this.allowPayloadForUndefinedHttpMethods = 
allowPayloadForUndefinedHttpMethods;
+    }
+
     // ----------------------------------------------------------- Parsing
     
     /**
@@ -677,8 +708,19 @@ public class HttpServerFilter extends HttpCodecFilter {
         
         final PayloadExpectation payloadExpectation = 
method.getPayloadExpectation();
         if (payloadExpectation != PayloadExpectation.NOT_ALLOWED) {
-            request.setExpectContent(
-                    request.getContentLength() != -1 || request.isChunked());
+            final boolean hasPayload =
+                    request.getContentLength() > 0 || request.isChunked();
+            
+            if (hasPayload && payloadExpectation == 
PayloadExpectation.UNDEFINED &&
+                    !allowPayloadForUndefinedHttpMethods) {
+                // if payload is not allowed for the "undefined" methods
+                state.error = true;
+                // Send 400; Bad Request
+                HttpStatus.BAD_REQUEST_400.setValues(response);
+                return;
+            }
+            
+            request.setExpectContent(hasPayload);
         } else {
             request.setExpectContent(method == Method.CONNECT);
         }--- 
a/modules/http/src/main/java/org/glassfish/grizzly/http/Method.java
+++ b/modules/http/src/main/java/org/glassfish/grizzly/http/Method.java
@@ -1,7 +1,7 @@
 /*
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
  *
- * Copyright (c) 2010-2013 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010-2014 Oracle and/or its affiliates. All rights reserved.
  *
  * The contents of this file are subject to the terms of either the GNU
  * General Public License Version 2 only ("GPL") or the Common Development
@@ -54,15 +54,15 @@ public final class Method {
     public static final Method OPTIONS =
             new Method("OPTIONS", PayloadExpectation.ALLOWED);
     public static final Method GET =
-            new Method("GET", PayloadExpectation.NOT_ALLOWED); // Even 
though it is UNDEFINED
+            new Method("GET", PayloadExpectation.UNDEFINED);
     public static final Method HEAD =
-            new Method("HEAD", PayloadExpectation.NOT_ALLOWED); // Even 
though it is UNDEFINED
+            new Method("HEAD", PayloadExpectation.UNDEFINED);
     public static final Method POST
             = new Method("POST", PayloadExpectation.ALLOWED);
     public static final Method PUT
             = new Method("PUT", PayloadExpectation.ALLOWED);
     public static final Method DELETE
-            = new Method("DELETE", PayloadExpectation.NOT_ALLOWED); // Even 
though it is UNDEFINED
+            = new Method("DELETE", PayloadExpectation.UNDEFINED);
     public static final Method TRACE
             = new Method("TRACE", PayloadExpectation.NOT_ALLOWED);
     public static final Method CONNECT--- 
a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java
+++ 
b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpRequestParseTest.java
@@ -96,7 +96,7 @@ public class HttpRequestParseTest extends TestCase {
                 new HashMap<String, Pair<String, String>>();
         headers.put("Host", new Pair<String,String>("localhost", 
"localhost"));
         headers.put("Content-length", new Pair<String,String>("2345", 
"2345"));
-        doHttpRequestTest("GET", "/index.html", "HTTP/1.1", headers, "\r\n");
+        doHttpRequestTest("POST", "/index.html", "HTTP/1.1", headers, 
"\r\n");
     }
 
     public void testMultiLineHeaders() throws Exception {
@@ -105,7 +105,7 @@ public class HttpRequestParseTest extends TestCase {
         headers.put("Host", new Pair<String,String>("localhost", 
"localhost"));
         headers.put("Multi-line", new Pair<String,String>("first\r\n         
 second\r\n       third", "first second third"));
         headers.put("Content-length", new Pair<String,String>("2345", 
"2345"));
-        doHttpRequestTest("GET", "/index.html", "HTTP/1.1", headers, "\r\n");
+        doHttpRequestTest("POST", "/index.html", "HTTP/1.1", headers, 
"\r\n");
     }
 
     public void testHeadersN() throws Exception {
@@ -114,7 +114,7 @@ public class HttpRequestParseTest extends TestCase {
         headers.put("Host", new Pair<String,String>("localhost", 
"localhost"));
         headers.put("Multi-line", new Pair<String,String>("first\r\n         
 second\n       third", "first second third"));
         headers.put("Content-length", new Pair<String,String>("2345", 
"2345"));
-        doHttpRequestTest("GET", "/index.html", "HTTP/1.1", headers, "\n");
+        doHttpRequestTest("POST", "/index.html", "HTTP/1.1", headers, "\n");
     }
 
     public void testCompleteURI() throws Exception {
@@ -122,7 +122,7 @@ public class HttpRequestParseTest extends TestCase {
                 new HashMap<String, Pair<String, String>>();
         headers.put("Host", new Pair<String,String>(null, "localhost:8180"));
         headers.put("Content-length", new Pair<String,String>("2345", 
"2345"));
-        doHttpRequestTest(new Pair<String, String>("GET", "GET"),
+        doHttpRequestTest(new Pair<String, String>("POST", "POST"),
                 new Pair<String, String>("http://localhost:8180/index.html", ;
"/index.html"),
                 new Pair<String,String>("HTTP/1.1", "HTTP/1.1"), headers, 
"\n");
     }
@@ -132,7 +132,7 @@ public class HttpRequestParseTest extends TestCase {
                 new HashMap<String, Pair<String, String>>();
         headers.put("Host", new Pair<String,String>(null, "localhost:8180"));
         headers.put("Content-length", new Pair<String,String>("2345", 
"2345"));
-        doHttpRequestTest(new Pair<String, String>("GET", "GET"),
+        doHttpRequestTest(new Pair<String, String>("POST", "POST"),
                 new Pair<String, String>("http://localhost:8180", "/";),
                 new Pair<String,String>("HTTP/1.1", "HTTP/1.1"), headers, 
"\n");
     }--- 
a/modules/http/src/test/java/org/glassfish/grizzly/http/HttpSemanticsTest.java
+++ 
b/modules/http/src/test/java/org/glassfish/grizzly/http/HttpSemanticsTest.java
@@ -275,7 +275,6 @@ public class HttpSemanticsTest extends TestCase {
                 .uri("/path")
                 .header("Host", "localhost:" + PORT)
                 .protocol("HTTP/1.0")
-                .chunked(true)
                 .build();
         ExpectedResult result = new ExpectedResult();
         result.setProtocol("HTTP/1.1");
@@ -386,6 +385,94 @@ public class HttpSemanticsTest extends TestCase {
         });
     }
 
+    public void testHttpGetWithPayloadDisabled() throws Throwable {
+
+        final HttpRequestPacket header = HttpRequestPacket.builder()
+                .method("GET")
+                .uri("/path")
+                .contentLength(10)
+                .header("Host", "localhost:" + PORT)
+                .protocol("HTTP/1.1")
+                .build();
+
+        final HttpContent chunk1 = HttpContent.builder(header)
+                .content(Buffers.wrap(MemoryManager.DEFAULT_MEMORY_MANAGER, 
"0123456789"))
+                .build();
+        
+        ExpectedResult result = new ExpectedResult();
+        result.setProtocol("HTTP/1.1");
+        result.setStatusCode(400);
+        result.addHeader("Connection", "close");
+        result.addHeader("Content-Length", "0");
+        result.setStatusMessage("bad request");
+        result.appendContent("");
+        doTest(new ClientFilter(chunk1, result, 1000), new BaseFilter() {
+            @Override
+            public NextAction handleRead(FilterChainContext ctx) throws 
IOException {
+                final HttpContent requestContent = ctx.getMessage();
+                if (!requestContent.isLast()) {
+                    return ctx.getStopAction(requestContent);
+                }
+                
+                HttpRequestPacket request = (HttpRequestPacket) 
requestContent.getHttpHeader();
+                HttpResponsePacket response = request.getResponse();
+
+                final Buffer payload = requestContent.getContent();
+                
+                HttpContent responseContent = response.httpContentBuilder().
+                        
content(payload).last(requestContent.isLast()).build();
+                ctx.write(responseContent);
+                return ctx.getStopAction();
+            }
+        });
+    }
+    
+    public void testHttpGetWithPayloadEnabled() throws Throwable {
+
+        // allow payload for GET
+        httpServerFilter.setAllowPayloadForUndefinedHttpMethods(true);
+        
+        final HttpRequestPacket header = HttpRequestPacket.builder()
+                .method("GET")
+                .uri("/path")
+                .contentLength(10)
+                .header("Host", "localhost:" + PORT)
+                .protocol("HTTP/1.1")
+                .build();
+
+        final HttpContent chunk1 = HttpContent.builder(header)
+                .content(Buffers.wrap(MemoryManager.DEFAULT_MEMORY_MANAGER, 
"0123456789"))
+                .build();
+        
+        ExpectedResult result = new ExpectedResult();
+        result.setProtocol("HTTP/1.1");
+        result.setStatusCode(200);
+        result.addHeader("!Connection", "close");
+        result.addHeader("!Transfer-Encoding", "chunked");
+        result.addHeader("Content-Length", "10");
+        result.setStatusMessage("ok");
+        result.appendContent("0123456789");
+        doTest(new ClientFilter(chunk1, result, 1000), new BaseFilter() {
+            @Override
+            public NextAction handleRead(FilterChainContext ctx) throws 
IOException {
+                final HttpContent requestContent = ctx.getMessage();
+                if (!requestContent.isLast()) {
+                    return ctx.getStopAction(requestContent);
+                }
+                
+                HttpRequestPacket request = (HttpRequestPacket) 
requestContent.getHttpHeader();
+                HttpResponsePacket response = request.getResponse();
+
+                final Buffer payload = requestContent.getContent();
+                
+                HttpContent responseContent = response.httpContentBuilder().
+                        
content(payload).last(requestContent.isLast()).build();
+                ctx.write(responseContent);
+                return ctx.getStopAction();
+            }
+        });
+    }
+    
     public void testUpgradeIgnoresTransferEncoding() throws Throwable {
 
         final HttpRequestPacket header = HttpRequestPacket.builder()





[grizzly~git:3122babe] [master] + fix issue #1666

oleksiys 03/21/2014
Terms of Use; Privacy Policy; Copyright ©2013-2017 (revision 20160708.bf2ac18)
 
 
Close
loading
Please Confirm
Close