Request HTTP method defaults to empty string

[weierophinney]

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.

---

Originally posted by @Ocramius at zendframework/zend-diactoros#150

[weierophinney]

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
zendframework/zend-diactoros#150.

---

Originally posted by @weierophinney at zendframework/zend-diactoros#150 (comment)

[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.

---

Originally posted by @Ocramius at zendframework/zend-diactoros#150 (comment)

[weierophinney]

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

---

Originally posted by @pine3ree at zendframework/zend-diactoros#150 (comment)

[Xerkus]

@Ocramius is this still relevant?

[Ocramius]

Haven't used it in a while, but I don't remember us validating this input anywhere.

[Xerkus]

@Ocramius behavior is unchanged since the issue was opened. I was rather asking if it is a behavior that you still think needs to be changed.

[Ocramius]

Yeah, I'd say that an empty HTTP method is not viable, so we'd need some default or some exception

[Xerkus]

Nevermind. I tested it wrong and missed default method value in the trait.

laminas-diactoros/src/RequestTrait.php

Line 34 in 800f287

private $method = 'GET';

This is already handled.