Special handling for pipelined HTTP requests with extra lines,

HEAD requests with chunked encoding, and new nextTick() handling.
apigee · Sep 18, 2014 · e3295c0 · e3295c0
1 parent 04cf5ae
commit e3295c0
Show file tree

Hide file tree

Showing 4 changed files with 92 additions and 20 deletions.
diff --git a/core/src/main/java/io/apigee/trireme/net/HTTPParsingMachine.java b/core/src/main/java/io/apigee/trireme/net/HTTPParsingMachine.java
@@ -169,8 +169,16 @@ public void reset()
      */
     public void setIgnoreBody(boolean ignore)
     {
-        if (ignore && (state == Status.BODY)) {
-            state = Status.COMPLETE;
+        if (ignore) {
+            switch (state) {
+            case BODY:
+            case CHUNK_HEADER:
+            case CHUNK_BODY:
+                state = Status.COMPLETE;
+                break;
+            default:
+                break;
+            }
         }
     }
 
@@ -180,6 +188,12 @@ public void setIgnoreBody(boolean ignore)
      */
     private boolean processStart(ByteBuffer buf, Result r)
     {
+        if ((buf.remaining() >= 2) &&
+            (buf.get(buf.position()) == '\r') && (buf.get(buf.position() + 1) == '\n')) {
+            // Special handling for pipelined requests that contain an extra newline at the end
+            buf.position(buf.position() + 2);
+        }
+
         String startLine = readLine(buf);
         if (startLine == null) {
             // We don't have a complete start line yet

diff --git a/core/src/test/java/io/apigee/trireme/core/test/HTTPParserTest.java b/core/src/test/java/io/apigee/trireme/core/test/HTTPParserTest.java
@@ -94,6 +94,66 @@ public void testCompleteRequestNoLength()
         parser.parse(null);
     }
 
+    @Test
+    public void testCompleteRequestNoLengthPipelined()
+    {
+        pipelineTest(COMPLETE_REQUEST_NOLENGTH  + COMPLETE_REQUEST_NOLENGTH + COMPLETE_REQUEST_NOLENGTH);
+    }
+
+    @Test
+    public void testCompleteRequestNoLengthPipelinedExtraLine()
+    {
+        pipelineTest(COMPLETE_REQUEST_NOLENGTH  + "\r\n" + COMPLETE_REQUEST_NOLENGTH + "\r\n" + COMPLETE_REQUEST_NOLENGTH);
+    }
+
+    private void pipelineTest(String requests)
+    {
+        HTTPParsingMachine parser = new HTTPParsingMachine(HTTPParsingMachine.ParsingMode.REQUEST);
+        ByteBuffer buf = Utils.stringToBuffer(requests, Charsets.ASCII);
+        HTTPParsingMachine.Result r = parser.parse(buf);
+        assertFalse(r.isError());
+        assertTrue(r.isComplete());
+        assertTrue(r.isHeadersComplete());
+        assertTrue(r.hasHeaders());
+        assertFalse(r.hasBody());
+        assertEquals(1, r.getMajor());
+        assertEquals(1, r.getMinor());
+        assertEquals("GET", r.getMethod());
+        assertEquals("/foo/bar/baz", r.getUri());
+        assertEquals("Myself", getFirstHeader(r, "User-Agent"));
+
+        assertTrue(buf.hasRemaining());
+        parser.reset();
+        r = parser.parse(buf);
+        assertFalse(r.isError());
+        assertTrue(r.isComplete());
+        assertTrue(r.isHeadersComplete());
+        assertTrue(r.hasHeaders());
+        assertFalse(r.hasBody());
+        assertEquals(1, r.getMajor());
+        assertEquals(1, r.getMinor());
+        assertEquals("GET", r.getMethod());
+        assertEquals("/foo/bar/baz", r.getUri());
+        assertEquals("Myself", getFirstHeader(r, "User-Agent"));
+
+        assertTrue(buf.hasRemaining());
+        parser.reset();
+        r = parser.parse(buf);
+        assertFalse(r.isError());
+        assertTrue(r.isComplete());
+        assertTrue(r.isHeadersComplete());
+        assertTrue(r.hasHeaders());
+        assertFalse(r.hasBody());
+        assertEquals(1, r.getMajor());
+        assertEquals(1, r.getMinor());
+        assertEquals("GET", r.getMethod());
+        assertEquals("/foo/bar/baz", r.getUri());
+        assertEquals("Myself", getFirstHeader(r, "User-Agent"));
+
+        assertFalse(buf.hasRemaining());
+        parser.parse(null);
+    }
+
     @Test
     public void testCompleteResponseLength()
     {

diff --git a/node10/node10src/src/main/javascript/io/apigee/trireme/node10/main/trireme.js b/node10/node10src/src/main/javascript/io/apigee/trireme/node10/main/trireme.js
@@ -339,6 +339,7 @@
   };
 
   startup.processNextTick = function() {
+    var _needTickCallback = process._needTickCallback.bind(process);
     var nextTickQueue = [];
     var needSpinner = true;
     var inTick = false;
@@ -353,8 +354,12 @@
     var index = 1;
     var depth = 2;
 
-    process.nextTick = nextTick;
+    process.nextTick = function nextTick(cb) {
+      process._currentTickHandler(cb);
+    };
+
     // needs to be accessible from cc land
+    process._currentTickHandler = _nextTick;
     process._nextDomainTick = _nextDomainTick;
     process._tickCallback = _tickCallback;
     process._tickDomainCallback = _tickDomainCallback;
@@ -378,7 +383,7 @@
           nextTickQueue.splice(0, infoBox[index]);
           infoBox[length] = nextTickQueue.length;
           if (needSpinner) {
-            process._needTickCallback();
+            _needTickCallback();
             needSpinner = false;
           }
         }
@@ -455,7 +460,7 @@
       // it'll try to keep clearing the queue, since the finally block
       // fires *before* the error hits the top level and is handled.
       if (infoBox[depth] >= process.maxTickDepth)
-        return process._needTickCallback();
+        return _needTickCallback();
 
       if (inTick) return;
       inTick = true;
@@ -494,7 +499,7 @@
       tickDone(0);
     }
 
-    function nextTick(callback) {
+    function _nextTick(callback) {
       // on the way out, don't bother. it won't get fired anyway.
       if (process._exiting)
         return;
@@ -507,7 +512,7 @@
       infoBox[length]++;
 
       if (needSpinner) {
-        process._needTickCallback();
+        _needTickCallback();
         needSpinner = false;
       }
     }
@@ -525,7 +530,7 @@
       infoBox[length]++;
 
       if (needSpinner) {
-        process._needTickCallback();
+        _needTickCallback();
         needSpinner = false;
       }
     }
@@ -826,7 +831,7 @@
     // Called by the "domain" module when switching to domains -- we replace a few functions
     // with others that are domain-aware.
     function usingDomains() {
-      process.nextTick = process._nextDomainTick;
+      process._currenTickHandler = process._nextDomainTick;
       process._tickCallback = process._tickDomainCallback;
       process._submitTick = submitDomainTick;
     }

diff --git a/node10/node10tests/simple/excluded-tests.xml b/node10/node10tests/simple/excluded-tests.xml
@@ -407,10 +407,6 @@
         <Name>test-net-GH-5504.js</Name>
         <Description>This is a complicated console-based test. Figure it out later.</Description>
     </Excluded>
-    <Excluded>
-        <Name>test-http-byteswritten.js</Name>
-        <Description>Doesn't work exactly the same as Node yet.</Description>
-    </Excluded>
     <Excluded>
         <Name>test-http-full-response.js</Name>
         <Description>Relies on ab which is hard to make portable.</Description>
@@ -447,10 +443,6 @@
         <Name>test-error-reporting.js</Name>
         <Description>Depends on specific errors.</Description>
     </Excluded>
-    <Excluded>
-        <Name>test-sys.js</Name>
-        <Description>Depends on fix to Rhino date parsing limitations.</Description>
-    </Excluded>
     <Excluded>
         <Name>test-string-decoder.js</Name>
         <Description>Something weird with UTF-16LE and others</Description>
@@ -511,6 +503,10 @@
         <Name>test-stream-pipe-multi.js</Name>
         <Description>Recursive nextTick -- doesn't work in node either.</Description>
     </Excluded>
+    <Excluded>
+        <Name>test-crypto-dh-odd-key.js</Name>
+        <Description>Not a supported use case in Java.</Description>
+    </Excluded>
 
     <!-- NETTY ADAPTER
          These tests don't work with the Netty adapter
@@ -661,9 +657,6 @@
     <Excluded>
         <Name>test-net-throttle.js</Name>
     </Excluded>
-    <Excluded>
-        <Name>test-net-bytes-stats.js</Name>
-    </Excluded>
     <Excluded>
         <Name>test-http-set-timeout-server.js</Name>
     </Excluded>