Repository: reactphp/http (php)

Issue:
Request body incorrectly parsed
Hello there,

I have implemented a [Mercure Hub](https://github.com/bpolaszek/mercure-php-hub) with ReactPHP and I'm having troubles with request bodies. 


I'm currently trying to achieve this:

```http
POST /.well-known/mercure
Content-Type: application/x-www-form-urlencoded

topic=/foo&topic=/bar&data=foobar
```

Indeed, the [Mercure protocol specification](https://mercure.rocks/spec) states that a topic `/foo` can have alternate IRIs such as `/bar`. The payload to be sent is not in the PHP format `topic[]=foo&topic[]=bar` but `topic=/foo&topic=/bar` which means that PHP, by default, considers that `topic` is a string with the only value `/bar`, which is not what I want. 
And so is the `React\Http\Middleware\RequestBodyParserMiddleware` which internally relies on PHP's `parse_str` function.

So, I'd like to implement my own logic to [parse the body payload by myself](https://github.com/bpolaszek/mercure-php-hub/blob/master/src/Helpers/QueryStringParser.php), but:
- It looks like ReactPHP removes any trace of the original body at some point (`$request->getBody()` returns an empty string), when `Content-Type` is `application/x-www-form-urlencoded` (could not reproduce with a random content type)
- It looks like the only way to short-circuit the `RequestBodyParserMiddleware` to introduce my own is to set an empty value for [enable_post_data_reading](https://github.com/reactphp/http/blob/master/src/Server.php#L228) in `php.ini`, but I cannot assume people who will use my project have this tricky tweak in their configuration.

Could we either:
- Not remove the original body of the request
- Introduce an interface for `RequestBodyParserMiddleware` so that we could optionally replace it in `Server`'s constructor
- Introduce an `array $config` of options into `Server`'s constructor so that we could pass an option to prevent parsing request bodies and let our own middlewares take care of this?

Thank you,
Ben

Pull request:
Keep request body in memory also after consuming request body
The `Server` will always buffer the incoming request body (unless the `StreamingRequestMiddleware` is used). Previously, this  buffer would be cleared when the request body has been parsed successfully. This means that the request body was only present for some requests.

This changeset ensures we keep the request body in memory also after consuming the request body. This means consumers can now always access the complete request body as detailed in the documentation. This allows building custom parsers and more advanced processing models without having to mess with the default parsers.

This shouldn't have a significant effect on memory consumption because the default buffer size is limited as per #371.

This also makes us less dependent on the underlying PSR-7 implementation which allows us to possibly change this in the future (#331).

Resolves #386, #387, #390 and others.

Patch:
diff --git a/src/Io/BufferedBody.php b/src/Io/BufferedBody.php
new file mode 100644
index 0000000..2f81bc5
--- /dev/null
+++ b/src/Io/BufferedBody.php
@@ -0,0 +1,176 @@
+<?php
+
+namespace React\Http\Io;
+
+use Psr\Http\Message\StreamInterface;
+
+/**
+ * [Internal] PSR-7 message body implementation using an in-memory buffer
+ *
+ * @internal
+ */
+class BufferedBody implements StreamInterface
+{
+    private $buffer = '';
+    private $position = 0;
+    private $closed = false;
+
+    public function __construct($buffer)
+    {
+        $this->buffer = $buffer;
+    }
+
+    public function __toString()
+    {
+        if ($this->closed) {
+            return '';
+        }
+
+        $this->seek(0);
+
+        return $this->getContents();
+    }
+
+    public function close()
+    {
+        $this->buffer = '';
+        $this->position = 0;
+        $this->closed = true;
+    }
+
+    public function detach()
+    {
+        $this->close();
+
+        return null;
+    }
+
+    public function getSize()
+    {
+        return $this->closed ? null : \strlen($this->buffer);
+    }
+
+    public function tell()
+    {
+        if ($this->closed) {
+            throw new \RuntimeException('Unable to tell position of closed stream');
+        }
+
+        return $this->position;
+    }
+
+    public function eof()
+    {
+        return $this->position >= \strlen($this->buffer);
+    }
+
+    public function isSeekable()
+    {
+        return !$this->closed;
+    }
+
+    public function seek($offset, $whence = \SEEK_SET)
+    {
+        if ($this->closed) {
+            throw new \RuntimeException('Unable to seek on closed stream');
+        }
+
+        $old = $this->position;
+
+        if ($whence === \SEEK_SET) {
+            $this->position = $offset;
+        } elseif ($whence === \SEEK_CUR) {
+            $this->position += $offset;
+        } elseif ($whence === \SEEK_END) {
+            $this->position = \strlen($this->buffer) + $offset;
+        } else {
+            throw new \InvalidArgumentException('Invalid seek mode given');
+        }
+
+        if (!\is_int($this->position) || $this->position < 0) {
+            $this->position = $old;
+            throw new \RuntimeException('Unable to seek to position');
+        }
+    }
+
+    public function rewind()
+    {
+        $this->seek(0);
+    }
+
+    public function isWritable()
+    {
+        return !$this->closed;
+    }
+
+    public function write($string)
+    {
+        if ($this->closed) {
+            throw new \RuntimeException('Unable to write to closed stream');
+        }
+
+        if ($string === '') {
+            return 0;
+        }
+
+        if ($this->position > 0 && !isset($this->buffer[$this->position - 1])) {
+            $this->buffer = \str_pad($this->buffer, $this->position, "\0");
+        }
+
+        $len = \strlen($string);
+        $this->buffer = \substr($this->buffer, 0, $this->position) . $string . \substr($this->buffer, $this->position + $len);
+        $this->position += $len;
+
+        return $len;
+    }
+
+    public function isReadable()
+    {
+        return !$this->closed;
+    }
+
+    public function read($length)
+    {
+        if ($this->closed) {
+            throw new \RuntimeException('Unable to read from closed stream');
+        }
+
+        if ($length < 1) {
+            throw new \InvalidArgumentException('Invalid read length given');
+        }
+
+        if ($this->position + $length > \strlen($this->buffer)) {
+            $length = \strlen($this->buffer) - $this->position;
+        }
+
+        if (!isset($this->buffer[$this->position])) {
+            return '';
+        }
+
+        $pos = $this->position;
+        $this->position += $length;
+
+        return \substr($this->buffer, $pos, $length);
+    }
+
+    public function getContents()
+    {
+        if ($this->closed) {
+            throw new \RuntimeException('Unable to read from closed stream');
+        }
+
+        if (!isset($this->buffer[$this->position])) {
+            return '';
+        }
+
+        $pos = $this->position;
+        $this->position = \strlen($this->buffer);
+
+        return \substr($this->buffer, $pos);
+    }
+
+    public function getMetadata($key = null)
+    {
+        return $key === null ? array() : null;
+    }
+}
diff --git a/src/Middleware/RequestBodyBufferMiddleware.php b/src/Middleware/RequestBodyBufferMiddleware.php
index 0e6f514..c13a5de 100644
--- a/src/Middleware/RequestBodyBufferMiddleware.php
+++ b/src/Middleware/RequestBodyBufferMiddleware.php
@@ -4,10 +4,10 @@ namespace React\Http\Middleware;
 
 use OverflowException;
 use Psr\Http\Message\ServerRequestInterface;
+use React\Http\Io\BufferedBody;
 use React\Http\Io\IniUtil;
 use React\Promise\Stream;
 use React\Stream\ReadableStreamInterface;
-use RingCentral\Psr7\BufferStream;
 
 final class RequestBodyBufferMiddleware
 {
@@ -38,7 +38,7 @@ final class RequestBodyBufferMiddleware
         if ($size === 0 || !$body instanceof ReadableStreamInterface) {
             // replace with empty body if body is streaming (or buffered size exceeds limit)
             if ($body instanceof ReadableStreamInterface || $size > $this->sizeLimit) {
-                $request = $request->withBody(new BufferStream(0));
+                $request = $request->withBody(new BufferedBody(''));
             }
 
             return $stack($request);
@@ -51,9 +51,7 @@ final class RequestBodyBufferMiddleware
         }
 
         return Stream\buffer($body, $sizeLimit)->then(function ($buffer) use ($request, $stack) {
-            $stream = new BufferStream(\strlen($buffer));
-            $stream->write($buffer);
-            $request = $request->withBody($stream);
+            $request = $request->withBody(new BufferedBody($buffer));
 
             return $stack($request);
         }, function ($error) use ($stack, $request, $body) {


Test patch:
diff --git a/tests/Io/BufferedBodyTest.php b/tests/Io/BufferedBodyTest.php
new file mode 100644
index 0000000..01154e7
--- /dev/null
+++ b/tests/Io/BufferedBodyTest.php
@@ -0,0 +1,300 @@
+<?php
+
+namespace React\Tests\Http\Io;
+
+use React\Tests\Http\TestCase;
+use React\Http\Io\BufferedBody;
+
+class BufferedBodyTest extends TestCase
+{
+    public function testEmpty()
+    {
+        $stream = new BufferedBody('');
+
+        $this->assertTrue($stream->isReadable());
+        $this->assertTrue($stream->isWritable());
+        $this->assertTrue($stream->isSeekable());
+        $this->assertSame(0, $stream->getSize());
+        $this->assertSame('', $stream->getContents());
+        $this->assertSame('', (string) $stream);
+    }
+
+    public function testClose()
+    {
+        $stream = new BufferedBody('hello');
+        $stream->close();
+
+        $this->assertFalse($stream->isReadable());
+        $this->assertFalse($stream->isWritable());
+        $this->assertFalse($stream->isSeekable());
+        $this->assertTrue($stream->eof());
+        $this->assertNull($stream->getSize());
+        $this->assertSame('', (string) $stream);
+    }
+
+    public function testDetachReturnsNullAndCloses()
+    {
+        $stream = new BufferedBody('hello');
+        $this->assertNull($stream->detach());
+
+        $this->assertFalse($stream->isReadable());
+        $this->assertFalse($stream->isWritable());
+        $this->assertFalse($stream->isSeekable());
+        $this->assertTrue($stream->eof());
+        $this->assertNull($stream->getSize());
+        $this->assertSame('', (string) $stream);
+    }
+
+    public function testSeekAndTellPosition()
+    {
+        $stream = new BufferedBody('hello');
+
+        $this->assertSame(0, $stream->tell());
+        $this->assertFalse($stream->eof());
+
+        $stream->seek(1);
+        $this->assertSame(1, $stream->tell());
+        $this->assertFalse($stream->eof());
+
+        $stream->seek(2, SEEK_CUR);
+        $this->assertSame(3, $stream->tell());
+        $this->assertFalse($stream->eof());
+
+        $stream->seek(-1, SEEK_END);
+        $this->assertSame(4, $stream->tell());
+        $this->assertFalse($stream->eof());
+
+        $stream->seek(0, SEEK_END);
+        $this->assertSame(5, $stream->tell());
+        $this->assertTrue($stream->eof());
+    }
+
+    public function testSeekAfterEndIsPermitted()
+    {
+        $stream = new BufferedBody('hello');
+
+        $stream->seek(1000);
+        $this->assertSame(1000, $stream->tell());
+        $this->assertTrue($stream->eof());
+
+        $stream->seek(0, SEEK_END);
+        $this->assertSame(5, $stream->tell());
+        $this->assertTrue($stream->eof());
+    }
+
+    public function testSeekBeforeStartThrows()
+    {
+        $stream = new BufferedBody('hello');
+
+        try {
+            $stream->seek(-10, SEEK_CUR);
+        } catch (\RuntimeException $e) {
+            $this->assertSame(0, $stream->tell());
+
+            $this->setExpectedException('RuntimeException');
+            throw $e;
+        }
+    }
+
+    public function testSeekWithInvalidModeThrows()
+    {
+        $stream = new BufferedBody('hello');
+
+        $this->setExpectedException('InvalidArgumentException');
+        $stream->seek(1, 12345);
+    }
+
+    public function testSeekAfterCloseThrows()
+    {
+        $stream = new BufferedBody('hello');
+        $stream->close();
+
+        $this->setExpectedException('RuntimeException');
+        $stream->seek(0);
+    }
+
+    public function testTellAfterCloseThrows()
+    {
+        $stream = new BufferedBody('hello');
+        $stream->close();
+
+        $this->setExpectedException('RuntimeException');
+        $stream->tell();
+    }
+
+    public function testRewindSeeksToStartPosition()
+    {
+        $stream = new BufferedBody('hello');
+
+        $stream->seek(1);
+        $stream->rewind();
+        $this->assertSame(0, $stream->tell());
+    }
+
+    public function testRewindAfterCloseThrows()
+    {
+        $stream = new BufferedBody('hello');
+        $stream->close();
+
+        $this->setExpectedException('RuntimeException');
+        $stream->rewind();
+    }
+
+    public function testGetContentsMultipleTimesReturnsBodyOnlyOnce()
+    {
+        $stream = new BufferedBody('hello');
+
+        $this->assertSame(5, $stream->getSize());
+        $this->assertSame('hello', $stream->getContents());
+        $this->assertSame('', $stream->getContents());
+    }
+
+    public function testReadReturnsChunkAndAdvancesPosition()
+    {
+        $stream = new BufferedBody('hello');
+
+        $this->assertSame('he', $stream->read(2));
+        $this->assertSame(2, $stream->tell());
+
+        $this->assertSame('ll', $stream->read(2));
+        $this->assertSame(4, $stream->tell());
+
+        $this->assertSame('o', $stream->read(2));
+        $this->assertSame(5, $stream->tell());
+
+        $this->assertSame('', $stream->read(2));
+        $this->assertSame(5, $stream->tell());
+    }
+
+    public function testReadAfterEndReturnsEmptyStringWithoutChangingPosition()
+    {
+        $stream = new BufferedBody('hello');
+
+        $stream->seek(1000);
+
+        $this->assertSame('', $stream->read(2));
+        $this->assertSame(1000, $stream->tell());
+    }
+
+    public function testReadZeroThrows()
+    {
+        $stream = new BufferedBody('hello');
+
+        $this->setExpectedException('InvalidArgumentException');
+        $stream->read(0);
+    }
+
+    public function testReadAfterCloseThrows()
+    {
+        $stream = new BufferedBody('hello');
+        $stream->close();
+
+        $this->setExpectedException('RuntimeException');
+        $stream->read(10);
+    }
+
+    public function testGetContentsReturnsWholeBufferAndAdvancesPositionToEof()
+    {
+        $stream = new BufferedBody('hello');
+
+        $this->assertSame('hello', $stream->getContents());
+        $this->assertSame(5, $stream->tell());
+        $this->assertTrue($stream->eof());
+    }
+
+    public function testGetContentsAfterEndsReturnsEmptyStringWithoutChangingPosition()
+    {
+        $stream = new BufferedBody('hello');
+
+        $stream->seek(100);
+
+        $this->assertSame('', $stream->getContents());
+        $this->assertSame(100, $stream->tell());
+        $this->assertTrue($stream->eof());
+    }
+
+    public function testGetContentsAfterCloseThrows()
+    {
+        $stream = new BufferedBody('hello');
+        $stream->close();
+
+        $this->setExpectedException('RuntimeException');
+        $stream->getContents();
+    }
+
+    public function testWriteAdvancesPosition()
+    {
+        $stream = new BufferedBody('');
+
+        $this->assertSame(2, $stream->write('he'));
+        $this->assertSame(2, $stream->tell());
+
+        $this->assertSame(2, $stream->write('ll'));
+        $this->assertSame(4, $stream->tell());
+
+        $this->assertSame(1, $stream->write('o'));
+        $this->assertSame(5, $stream->tell());
+
+        $this->assertSame(0, $stream->write(''));
+        $this->assertSame(5, $stream->tell());
+    }
+
+    public function testWriteInMiddleOfBufferOverwrites()
+    {
+        $stream = new BufferedBody('hello');
+
+        $stream->seek(1);
+        $this->assertSame(1, $stream->write('a'));
+
+        $this->assertSame(2, $stream->tell());
+        $this->assertsame(5, $stream->getSize());
+        $this->assertSame('hallo', (string) $stream);
+    }
+
+    public function testWriteOverEndOverwritesAndAppends()
+    {
+        $stream = new BufferedBody('hello');
+
+        $stream->seek(4);
+        $this->assertSame(2, $stream->write('au'));
+
+        $this->assertSame(6, $stream->tell());
+        $this->assertsame(6, $stream->getSize());
+        $this->assertSame('hellau', (string) $stream);
+    }
+
+    public function testWriteAfterEndAppendsAndFillsWithNullBytes()
+    {
+        $stream = new BufferedBody('hello');
+
+        $stream->seek(6);
+        $this->assertSame(6, $stream->write('binary'));
+
+        $this->assertSame(12, $stream->tell());
+        $this->assertsame(12, $stream->getSize());
+        $this->assertSame('hello' . "\0" . 'binary', (string) $stream);
+    }
+
+    public function testWriteAfterCloseThrows()
+    {
+        $stream = new BufferedBody('hello');
+        $stream->close();
+
+        $this->setExpectedException('RuntimeException');
+        $stream->write('foo');
+    }
+
+    public function testGetMetadataWithoutKeyReturnsEmptyArray()
+    {
+        $stream = new BufferedBody('hello');
+
+        $this->assertEquals(array(), $stream->getMetadata());
+    }
+
+    public function testGetMetadataWithKeyReturnsNull()
+    {
+        $stream = new BufferedBody('hello');
+
+        $this->assertNull($stream->getMetadata('key'));
+    }
+}
diff --git a/tests/ServerTest.php b/tests/ServerTest.php
index ce19dda..84d93eb 100644
--- a/tests/ServerTest.php
+++ b/tests/ServerTest.php
@@ -110,6 +110,35 @@ final class ServerTest extends TestCase
         $this->assertSame('beforeokafter', $called);
     }
 
+    public function testPostFormData()
+    {
+        $loop = Factory::create();
+        $deferred = new Deferred();
+        $server = new Server($loop, function (ServerRequestInterface $request) use ($deferred) {
+            $deferred->resolve($request);
+        });
+
+        $server->listen($this->socket);
+        $this->socket->emit('connection', array($this->connection));
+        $this->connection->emit('data', array("POST / HTTP/1.0\r\nContent-Type: application/x-www-form-urlencoded\r\nContent-Length: 7\r\n\r\nfoo=bar"));
+
+        $request = Block\await($deferred->promise(), $loop);
+        assert($request instanceof ServerRequestInterface);
+
+        $form = $request->getParsedBody();
+
+        $this->assertTrue(isset($form['foo']));
+        $this->assertEquals('bar', $form['foo']);
+
+        $this->assertEquals(array(), $request->getUploadedFiles());
+
+        $body = $request->getBody();
+
+        $this->assertSame(7, $body->getSize());
+        $this->assertSame(7, $body->tell());
+        $this->assertSame('foo=bar', (string) $body);
+    }
+
     public function testPostFileUpload()
     {
         $loop = Factory::create();
@@ -132,11 +161,14 @@ final class ServerTest extends TestCase
             }
         });
 
-        $parsedRequest = Block\await($deferred->promise(), $loop);
-        $this->assertNotEmpty($parsedRequest->getUploadedFiles());
-        $this->assertEmpty($parsedRequest->getParsedBody());
+        $request = Block\await($deferred->promise(), $loop);
+        assert($request instanceof ServerRequestInterface);
+
+        $this->assertEmpty($request->getParsedBody());
+
+        $this->assertNotEmpty($request->getUploadedFiles());
 
-        $files = $parsedRequest->getUploadedFiles();
+        $files = $request->getUploadedFiles();
 
         $this->assertTrue(isset($files['file']));
         $this->assertCount(1, $files);
@@ -144,6 +176,37 @@ final class ServerTest extends TestCase
         $this->assertSame('hello.txt', $files['file']->getClientFilename());
         $this->assertSame('text/plain', $files['file']->getClientMediaType());
         $this->assertSame("hello\r\n", (string)$files['file']->getStream());
+
+        $body = $request->getBody();
+
+        $this->assertSame(220, $body->getSize());
+        $this->assertSame(220, $body->tell());
+    }
+
+    public function testPostJsonWillNotBeParsedByDefault()
+    {
+        $loop = Factory::create();
+        $deferred = new Deferred();
+        $server = new Server($loop, function (ServerRequestInterface $request) use ($deferred) {
+            $deferred->resolve($request);
+        });
+
+        $server->listen($this->socket);
+        $this->socket->emit('connection', array($this->connection));
+        $this->connection->emit('data', array("POST / HTTP/1.0\r\nContent-Type: application/json\r\nContent-Length: 6\r\n\r\n[true]"));
+
+        $request = Block\await($deferred->promise(), $loop);
+        assert($request instanceof ServerRequestInterface);
+
+        $this->assertNull($request->getParsedBody());
+
+        $this->assertSame(array(), $request->getUploadedFiles());
+
+        $body = $request->getBody();
+
+        $this->assertSame(6, $body->getSize());
+        $this->assertSame(0, $body->tell());
+        $this->assertSame('[true]', (string) $body);
     }
 
     public function testServerReceivesBufferedRequestByDefault()