tuupola / http-factory

Lightweight autodiscovering PSR-17 HTTP factories
MIT License
17 stars 6 forks source link

Guzzles Server Request never contains POST data? #11

Open burzum opened 5 years ago

burzum commented 5 years ago

I'm sending a POST request via Guzzle ("guzzlehttp/guzzle": "^6.3") but whatever I do the body is always NULL.

<?php
require_once 'vendor/autoload.php';

use GuzzleHttp\Client;
use GuzzleHttp\Psr7\Request;

$client = new Client(['base_uri' => 'http://email-service.test']);

$response = $client->request('POST', '/emails', [
    'form_params' => [
        'field_name' => 'abc',
        'other_field' => '123',
        'nested_field' => [
            'nested' => 'hello'
        ]
    ]
]);

var_dump($response->getBody()->read(1000));

On the receiving side of the server I'm just doing this

var_dump($request->getParsedBody());

And the output is always NULL.

If I use my own factory I'm getting the right data, so I assume something is wrong with the way this lib is creating the guzzle server request object.

<?php
declare(strict_types=1);

namespace App\Infrastructure\Http;

use GuzzleHttp\Psr7\ServerRequest;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestFactoryInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Message\UriInterface;

/**
 * ServerRequestFactory
 */
class ServerRequestFactory implements ServerRequestFactoryInterface
{
    /**
     * Create a new server request.
     * Note that server-params are taken precisely as given - no parsing/processing
     * of the given values is performed, and, in particular, no attempt is made to
     * determine the HTTP method or URI, which must be provided explicitly.
     *
     * @param string $method The HTTP method associated with the request.
     * @param UriInterface|string $uri The URI associated with the request. If
     *     the value is a string, the factory MUST create a UriInterface
     *     instance based on it.
     * @param array $serverParams Array of SAPI parameters with which to seed
     *     the generated request instance.
     * @return ServerRequestInterface
     */
    public function createServerRequest(string $method, $uri, array $serverParams = []): ServerRequestInterface
    {
        return ServerRequest::fromGlobals();
    }
}
tuupola commented 5 years ago

On the receiving side, how are you creating the $request object? What framework is used?

burzum commented 5 years ago

@tuupola I'm not using any framework at all, we're moving away from CakePHP to a framework-less approach with CQRS.

I think the problem is in the factory:

        if (class_exists(GuzzleServerRequest::class)) {
            return new GuzzleServerRequest($method, $uri, [], null, "1.1", $serverParams);
        }

If you look at the Guzzle ServerRequests fromGlobals() method you'll see how it constructs the body, while body (4th arg) is null in your implementation and it also adds uploaded files later on. Also the 3rd arg, the headers aren't passed either.

    public static function fromGlobals()
    {
        $method = isset($_SERVER['REQUEST_METHOD']) ? $_SERVER['REQUEST_METHOD'] : 'GET';
        $headers = getallheaders();
        $uri = self::getUriFromGlobals();
        $body = new CachingStream(new LazyOpenStream('php://input', 'r+'));
        $protocol = isset($_SERVER['SERVER_PROTOCOL']) ? str_replace('HTTP/', '', $_SERVER['SERVER_PROTOCOL']) : '1.1';

        $serverRequest = new ServerRequest($method, $uri, $headers, $body, $protocol, $_SERVER);

        return $serverRequest
            ->withCookieParams($_COOKIE)
            ->withQueryParams($_GET)
            ->withParsedBody($_POST)
            ->withUploadedFiles(self::normalizeFiles($_FILES));
    }

See https://github.com/guzzle/psr7/blob/master/src/ServerRequest.php#L167

So either copy the code from their method or simply use "fromGlobals"?

I'm also not sure if the other implementations might not lack the same kind of code, that attaches the files and body. The question is if one wants to intentionally not add these things and leave it up to the developer or not. In my opinion it's not a good idea because it will require me to do it and then I'm ending up creating my own factory any way.

tuupola commented 5 years ago

Leaving body out was unintentional. Have not noticed the problem before since I do not use Guzzle myself and there was no unit test for this. Thanks for the heads up!

tuupola commented 5 years ago

Actually now I remember. Creating a fully functional ServerRequest object from globals was intentionally left out from the PSR. Guzzle, Diactoros and Slim 4 provide a non PSR fromGlobals() method. Slim 3 and Nyholm do not provide such method.

There is an excellent nyholm/psr7-server though which can be used for creating fully fledged ServerRequest objects. I am not sure yet if this library should provide something similar or just follow the PSR standard.

burzum commented 5 years ago

@tuupola yes, I know that this is not part of PSR, but your lib is a wrapper / auto-detector around other libs. :) And how you construct a request is also not defined. If you don't like to simply call the fromGlobals() method you could copy the body of that method, I think it uses only PSR methods.

I've made a few changes here https://github.com/burzum/http-factory/blob/refactor/src/ServerRequestFactory.php to "complete" the information available to the request objects and also added two ways to detect and instantiate Nyhols Request Objects.