zend-diactoros icon indicating copy to clipboard operation
zend-diactoros copied to clipboard

Request HTTP method defaults to empty string

Open Ocramius opened this issue 9 years ago • 4 comments
trafficstars

Discovered while digging in https://github.com/php-http/curl-client/issues/14

Apparently, diactoros defaults the HTTP method when building a new Request('http://example.com') to '' (empty string). As far as I know, an empty string is not a valid HTTP method (not sure if that assumption is reflected in the HTTP spec), and therefore the initial state of a diactoros HTTP request is invalid, and should lead to an exception.

Ocramius avatar Mar 05 '16 02:03 Ocramius

What would you consider a valid default? GET? HEAD? OPTIONS?

Additionally, IIRC, somebody presented a use case for allowing a nullable method, so we'll need to see if those needs are still valid, and how to handle that with the concept of a default. On Mar 4, 2016 8:30 PM, "Marco Pivetta" [email protected] wrote:

Discovered while digging in php-http/curl-client#14 https://github.com/php-http/curl-client/issues/14

Apparently, diactoros defaults the HTTP method when building a new Request('http://example.com') to '' (empty string). As far as I know, an empty string is not a valid HTTP method (not sure if that assumption is reflected in the HTTP spec), and therefore the initial state of a diactoros HTTP request is invalid, and should lead to an exception.

— Reply to this email directly or view it on GitHub https://github.com/zendframework/zend-diactoros/issues/150.

weierophinney avatar Mar 05 '16 04:03 weierophinney

What would you consider a valid default? GET? HEAD? OPTIONS?

That is a good question, but I'm fairly sure that 90% of the web traffic is just GET, so going with that is a quite decent choice. That would just be the default value, but the idea is to simply reject anything that isn't a valid HTTP method. For example, HTTP methods with invalid characters should also be rejected (spaces are one simple case that can be handled).

Overall, this logic can be encapsulated in a tiny HttpMethod value object, which doesn't need to be exposed to userland.

Ocramius avatar Mar 05 '16 17:03 Ocramius

I've just remembered that i implemented psr-7 starting from phly/http and added a default method in the constructor ('GET') and a simple http-method filtering method (mwop would have nameed it marhallMethod). Juts to get an idea form ths code fragment

//...
    protected static $validMethods = [
        'OPTIONS'  => true,
        'GET'      => true,
        'HEAD'     => true,
        'POST'     => true,
        'PUT'      => true,
        'DELETE'   => true,
        'TRACE'    => true,
        'CONNECT'  => true,
        'PATCH'    => true,
        'PROPFIND' => true,
    ];

    /**
     * Array of possible CSRF Header names
     * @var array
     */
    protected static $csrfHeaderNames = [
        'X-CSRF-Token',
        'X-CSRFToken',
        'X-XSRF-TOKEN',
    ];

    /**
     * Constructor
     * @param UriInterface $uri
     * @param string $method
     * @param array $headers
     * @param Stream|resource|string $body
     * @param string $protocolVersion
     * @throws InvalidArgumentExceptions
     */
    public function __construct(
        $uri = null,
        $method = 'GET',
        $headers = [],
        $body = 'php://temp',
        $protocolVersion = '1.1'
    ) {
        parent::__construct($protocolVersion, $headers, $body);

        $this->method = $this->filterMethod($method);

        // Initialize uri from constructor argument or build uri from request
        // environment
        if (null === $uri) {
            $this->uri = new Uri('');
        } else if (is_string($uri)) {
            $this->uri = new Uri($uri);
        } elseif($uri instanceof UriInterface) {
            $this->uri = $uri;
        } else {
            throw new InvalidArgumentException(
                'The constructor $uri must be a string, an instance of UriInterface or null'
            );
        }
    }
//...
    /**
     * Validate the HTTP method
     *
     * @param null|string $method
     * @throws InvalidArgumentException on invalid HTTP method.
     */
    protected function filterMethod($method)
    {
        if (null === $method) {
            return 'GET';
        }

        if (! is_string($method)) {
            throw new InvalidArgumentException(
                'The HTTP method must be a string'
            );
        }

        $method = strtoupper($method);

        if (! isset(static::$validMethods[$method])) {
            throw new InvalidArgumentException(sprintf(
                'Unsupported HTTP method "%s"',
                $method
            ));
        }

        return $method;
    }

pine3ree avatar May 30 '16 23:05 pine3ree

This repository has been closed and moved to laminas/laminas-diactoros; a new issue has been opened at https://github.com/laminas/laminas-diactoros/issues/26.

weierophinney avatar Dec 31 '19 22:12 weierophinney