Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Request HTTP method defaults to empty string #150

Open
Ocramius opened this issue Mar 5, 2016 · 4 comments
Open

Request HTTP method defaults to empty string #150

Ocramius opened this issue Mar 5, 2016 · 4 comments
Labels

Comments

@Ocramius
Copy link
Member

Ocramius commented Mar 5, 2016

Discovered while digging in php-http/curl-client#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 Ocramius added the bug label Mar 5, 2016
@weierophinney
Copy link
Member

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" notifications@github.com wrote:

Discovered while digging in php-http/curl-client#14
php-http/curl-client#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
#150.

@Ocramius
Copy link
Member Author

Ocramius commented Mar 5, 2016

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.

@pine3ree
Copy link
Contributor

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;
    }

@weierophinney
Copy link
Member

This repository has been closed and moved to laminas/laminas-diactoros; a new issue has been opened at laminas/laminas-diactoros#26.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants