Refactored processRequest method on ConnectionHandler

Author: AndrewCarterUK

src/ConnectionHandler/ConnectionHandler.php

@@ -169,25 +169,24 @@ private function processRecord(array $record)
 
         $content = 0 === $record['contentLength'] ? null : $record['contentData'];
 
-        switch ($record['type']) {
-            case DaemonInterface::FCGI_BEGIN_REQUEST:
-                $this->processBeginRequestRecord($requestId, $content);
-                break;
-
-            case DaemonInterface::FCGI_PARAMS:
-                $this->processParamsRecord($requestId, $content);
-                break;
-
-            case DaemonInterface::FCGI_STDIN:
-                $this->processStdinRecord($requestId, $content);
-                break;
-
-            case DaemonInterface::FCGI_ABORT_REQUEST:
-                $this->processAbortRequestRecord($requestId);
-                break;
-
-            default:
-                throw new ProtocolException('Unexpected packet of unkown type: '.$record['type']);
+        if (DaemonInterface::FCGI_BEGIN_REQUEST === $record['type']) {
+            $this->processBeginRequestRecord($requestId, $content);
+        } elseif (!isset($this->requests[$requestId])) {
+            throw new ProtocolException('Invalid request id for record of type: ' . $record['type']);
+        } elseif (DaemonInterface::FCGI_PARAMS === $record['type']) {
+            while (strlen($content) > 0) {
+                $this->readNameValuePair($requestId, $content);
+            }
+        } elseif (DaemonInterface::FCGI_STDIN === $record['type']) {
+            if (null !== $content) {
+                fwrite($this->requests[$requestId]['stdin'], $content);
+            } else {
+                $this->dispatchRequest($requestId);
+            }
+        } elseif (DaemonInterface::FCGI_ABORT_REQUEST === $record['type']) {
+            $this->endRequest($requestId);
+        } else {
+            throw new ProtocolException('Unexpected packet of type: '.$record['type']);
         }
     }
 
@@ -228,29 +227,6 @@ private function processBeginRequestRecord($requestId, $contentData)
         }
     }
 
-    /**
-     * Process a FCGI_PARAMS record.
-     *
-     * @param int    $requestId   The request id sending the record
-     * @param string $contentData The content of the record
-     *
-     * @throws ProtocolException If the FCGI_PARAMS record is unexpected
-     */
-    private function processParamsRecord($requestId, $contentData)
-    {
-        if (!isset($this->requests[$requestId])) {
-            throw new ProtocolException('Unexpected FCGI_PARAMS record');
-        }
-
-        if (null === $contentData) {
-            return;
-        }
-
-        do {
-            $this->readNameValuePair($requestId, $contentData);
-        } while (strlen($contentData) > 0);
-    }
-
     /**
      * Read a FastCGI name-value pair from a buffer and add it to the request params
      * 
@@ -303,45 +279,6 @@ private function readFieldLength(&$buffer)
         return $length;
     }
 
-    /**
-     * Process a FCGI_STDIN record.
-     *
-     * @param int    $requestId   The request id sending the record
-     * @param string $contentData The content of the record
-     *
-     * @throws ProtocolException If the FCGI_STDIN record is unexpected
-     */
-    private function processStdinRecord($requestId, $contentData)
-    {
-        if (!isset($this->requests[$requestId])) {
-            throw new ProtocolException('Unexpected FCGI_STDIN record');
-        }
-
-        if (null === $contentData) {
-            $this->dispatchRequest($requestId);
-
-            return;
-        }
-
-        fwrite($this->requests[$requestId]['stdin'], $contentData);
-    }
-
-    /**
-     * Process a FCGI_ABORT_REQUEST record.
-     *
-     * @param int $requestId The request id sending the record
-     *
-     * @throws ProtocolException If the FCGI_ABORT_REQUEST record is unexpected
-     */
-    private function processAbortRequestRecord($requestId)
-    {
-        if (!isset($this->requests[$requestId])) {
-            throw new ProtocolException('Unexpected FCG_ABORT_REQUEST record');
-        }
-
-        $this->endRequest($requestId);
-    }
-    
     /**
      * End the request by writing an FCGI_END_REQUEST record and then removing
      * the request from memory and closing the connection if necessary.

test/ConnectionHandler/ConnectionHandlerTest.php

@@ -220,9 +220,10 @@ public function testAbortRequestRecord()
     }
 
     /**
-     * Test that the daemon correctly handles an unexpected abort request record.
+     * Test that the daemon correctly handles an abort request record with a
+     * request id that hasn't been initiated.
      */
-    public function testUnexpectedAbortRequestRecord()
+    public function testInvalidRequestId()
     {
         $context = $this->createTestingContext();
 
@@ -239,7 +240,7 @@ public function testUnexpectedAbortRequestRecord()
         $expectedLogMessages = [
             [
                 'level'   => 'error',
-                'message' => 'Unexpected FCG_ABORT_REQUEST record',
+                'message' => 'Invalid request id for record of type: '.DaemonInterface::FCGI_ABORT_REQUEST,
                 'context' => [],
             ],
         ];
@@ -257,6 +258,7 @@ public function testUnexpectedRecord()
         $context = $this->createTestingContext();
 
         // Write a record with a type that makes no sense (application doesn't receive stdout)
+        $context['clientWrapper']->writeBeginRequestRecord(0, DaemonInterface::FCGI_RESPONDER, 0);
         $context['clientWrapper']->writeRecord(DaemonInterface::FCGI_STDOUT, 0);
 
         // Run the handler
@@ -269,7 +271,7 @@ public function testUnexpectedRecord()
         $expectedLogMessages = [
             [
                 'level'   => 'error',
-                'message' => 'Unexpected packet of unkown type: '.DaemonInterface::FCGI_STDOUT,
+                'message' => 'Unexpected packet of type: '.DaemonInterface::FCGI_STDOUT,
                 'context' => [],
             ],
         ];
@@ -312,68 +314,6 @@ public function testUnexpectedBeginRequestRecord()
         fclose($context['sockets'][0]);
     }
 
-    /**
-     * Test that the daemon throws an error if a stdin record is received for
-     * an unknown request id.
-     */
-    public function testUnexpectedStdinRecord()
-    {
-        $context = $this->createTestingContext();
-
-        // Write an unexpected record and close the connection
-        $context['clientWrapper']->writeStdinRecord(0);
-
-        // Run the handler
-        $context['handler']->ready();
-
-        // Check daemon has closed server side connection
-        $this->assertTrue($context['connection']->isClosed());
-
-        // Check the logging has worked
-        $expectedLogMessages = [
-            [
-                'level'   => 'error',
-                'message' => 'Unexpected FCGI_STDIN record',
-                'context' => [],
-            ],
-        ];
-        $this->assertEquals($expectedLogMessages, $context['logger']->getMessages());
-
-        // Close the client side socket
-        fclose($context['sockets'][0]);
-    }
-
-    /**
-     * Test that the daemon throws an error if a params record is received for
-     * an unknown request id.
-     */
-    public function testUnexpectedParamsRecord()
-    {
-        $context = $this->createTestingContext();
-
-        // Write an unexpected record and close the connection
-        $context['clientWrapper']->writeParamsRecord(0);
-
-        // Run the handler
-        $context['handler']->ready();
-
-        // Check daemon has closed server side connection
-        $this->assertTrue($context['connection']->isClosed());
-
-        // Check the logging has worked
-        $expectedLogMessages = [
-            [
-                'level'   => 'error',
-                'message' => 'Unexpected FCGI_PARAMS record',
-                'context' => [],
-            ],
-        ];
-        $this->assertEquals($expectedLogMessages, $context['logger']->getMessages());
-
-        // Close the client side socket
-        fclose($context['sockets'][0]);
-    }
-
     /**
      * Test that the daemon responds correctly to a non-responder role.
      */