asika32764 / http

The PSR7 Http Implementation. (PHP 5.3 Compatible)
https://packagist.org/packages/asika/http
26 stars 2 forks source link

Psr incompatibility in AbstractTransport Uri handling #1

Closed kaiwa closed 9 years ago

kaiwa commented 9 years ago

Hi,

NULL is not allowed as argument to UriInterface::withPath(), UriInterface::withQuery() and UriInterface::withFragment() according to https://github.com/php-fig/http-message/blob/master/src/UriInterface.php and implemented in AbstractTransport.php:

        $uri = $request->getUri()
            ->withPath(null)
            ->withQuery(null)
            ->withFragment(null);

I noticed this actually breaks when trying to send a RequestInterface implementation created with guzzlehttp/psr7 via your HttpClient, because they are checking the argument with is_string() inside their Uri implementation. https://github.com/guzzle/psr7/blob/master/src/Uri.php#L383

asika32764 commented 9 years ago

I'm not sure. See withHost() in Guzzle and phly

https://github.com/guzzle/psr7/blob/master/src/Uri.php#L357

https://github.com/phly/http/blob/master/src/Uri.php

And PSR interface: https://github.com/php-fig/http-message/blob/master/src/UriInterface.php#L221

I think the is_string() condition was implemented by the author of phly, and guzzle forked it.

Some of the params in PSR interface are string|null but some are only string, and the docblock did not say NULL is not allowed.

I guess it is a fuzzy zone that PSR did not defined clearly. But I'll still add the is_string() condition to make my package compatibility to Guzzle.

Thank you for reporting it.

asika32764 commented 9 years ago

Also, see withFragment() in phly https://github.com/phly/http/blob/master/src/Uri.php#L368

https://github.com/phly/http/blob/master/src/Uri.php#L565

You will notice that NULL is allowed in phly Uri::withFragment(), so I think the is_string condition in withPath() and withQuery() is a misunderstand of Psr7.

kaiwa commented 9 years ago

Thank you for your quick reply, let's see what they're saying over at zendframework/zend-diactoros.