Requests with a custom port seem to be ignored?

[Ocramius]

I'm not sure where this gets swallowed, but I'm trying following:

use Http\Client\Curl\Client;
use Http\Client\HttpClient;
use Http\Message\MessageFactory\DiactorosMessageFactory;
use Http\Message\StreamFactory\DiactorosStreamFactory;
use Zend\Diactoros\Request;

$client = new Client(new DiactorosMessageFactory(), new DiactorosStreamFactory());

$url = 'http://localhost:8085/api/v2/messages?start=0&limit=50';
$response = $this->client->sendRequest(
    (new Request($url))->withAddedHeader('Accept', 'application/json')
);

var_dump((string) $response->getBody()); // 404 page not found
file_get_contents($url); // the JSON I wanted

This is while interacting with MailHog.

While debugging, it seems like the curl options are as following:

$options = [
    42 => true,
    19913 => true,
    52 => false,
    84 => 2,
    10002 => 'http://localhost:8025/api/v2/messages?start=0&limit=50',
    11036 => '',
    10023 => [
        0 => 'Host: localhost:8025',
    ],
];

Translated in curl options constants, that would be:

$options = [
    CURLOPT_HEADER => true,
    CURLOPT_RETURNTRANSFER => true,
    CURLOPT_FOLLOWLOCATION => false,
    CURLOPT_HTTP_VERSION => 2,
    CURLOPT_URL => 'http://localhost:8025/api/v2/messages?start=0&limit=50',
    CURLOPT_CUSTOMREQUEST => '',
    CURLOPT_HTTPHEADER => [
        0 => 'Host: localhost:8025',
    ],
];

Does anybody have a clue on why this behavior may happen? Note that requests seem to go to port 80.

[mekras]

I can't reproduce this.

port.php

<?php
echo $_SERVER['HTTP_HOST'];

test.php

<?php
use Http\Client\Curl\Client;
use Http\Message\MessageFactory\DiactorosMessageFactory;
use Http\Message\StreamFactory\DiactorosStreamFactory;
use Zend\Diactoros\Request;

$client = new Client(new DiactorosMessageFactory(), new DiactorosStreamFactory());
$url = 'http://localhost:8080/port.php';
$response = $client->sendRequest(new Request($url));

var_dump((string) $response->getBody());
var_dump(file_get_contents($url));

Result:

string(14) "localhost:8080"
string(14) "localhost:8080"

Can you sniff requests with tcpdump or Wireshark?

[Ocramius]

Getting following output:

Fatal error: Uncaught Http\Client\Exception\RequestException: Empty reply from server in vendor/php-http/curl-client/src/Client.php on line 116

Http\Client\Exception\RequestException: Empty reply from server in vendor/php-http/curl-client/src/Client.php on line 116

Call Stack:
    0.0002     365320   1. {main}() foo.php:0
    0.0047    1091552   2. Http\Client\Curl\Client->sendRequest() foo.php:12

The HTTP server reports:

php -S 0.0.0.0:8080 -t .
PHP 7.0.3 Development Server started at Tue Feb 16 09:13:10 2016
Listening on http://0.0.0.0:8080
Document root is /yadda/yadda
Press Ctrl-C to quit.
[Tue Feb 16 09:13:14 2016] 127.0.0.1:60588 [200]: /bar.php
[Tue Feb 16 09:13:15 2016] 127.0.0.1:60589 [200]: /bar.php
[Tue Feb 16 09:13:16 2016] 127.0.0.1:60590 [200]: /bar.php
[Tue Feb 16 09:13:16 2016] 127.0.0.1:60591 [200]: /bar.php
[Tue Feb 16 09:13:21 2016] 127.0.0.1:60593 Invalid request (Malformed HTTP request)
[Tue Feb 16 09:13:46 2016] 127.0.0.1:60599 Invalid request (Malformed HTTP request)
[Tue Feb 16 09:13:55 2016] 127.0.0.1:60602 Invalid request (Malformed HTTP request)

Note: first 4 requests are from the browser

[mekras]

And how about sniffing? I need the dump of the failed HTTP request.

[Ocramius]

Here's a wireshark capture (attached)

capture2.zip

[mekras]

Hm… I don't see any HTTP traffic in your dump.

[Ocramius]

Yeah, that's the weird part. Not sure if it's just ext-curl messing it up a this point...

[Ocramius]

Actually, the 9th packet includes some HTTP 1.1 stuff, but that's all there is...

[mekras]

So far, I have no ideas…

[mekras]

Can you try native PHP cURL functions?

[Ocramius]

Native CURL seems to work:

$ch = curl_init();
curl_setopt($ch, CURLOPT_URL, 'http://localhost:8080/foo.php');
curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
echo curl_exec($ch);

I'll see if I have more time to dig into this at the end of the month - long weeks ahead :(

[Ocramius]

I accidentally found the cause of the problem. It is possibly caused by Diactoros, and not this library. Specifically, when no HTTP method is specified, the HTTP method defaults to '' (empty string) in Diactoros.

I'm not sure whether this should be considered a bug in Diactoros or in this library. This lib is simply putting '' (empty string) as HTTP method, since the HTTP method is not GET.

@mekras what do you suggest here? Does the HTTP spec allow an empty method? If so, then both Diactoros and this library should throw an exception in that case, what do you think?

[sagikazarmark]

I am not sure whether it should be handled here or in DIactoros. Looking at Guzzle PSR-7 implementation, it does not defaults to, but also allows an empty string (no validation).

I checked the RFCs PSR-7 is built around:

http://tools.ietf.org/html/rfc7230#section-3.1.1

For me, nothing indicates that the method can be omitted.

Also, based on PSR-7, the request should throw an invalid argument exception upon an invalid method passed to withMethod. Based on this I would say it's the Request's responsibility to validate the input data, even if it is injected into the constructor.

[Ocramius]

Opened zendframework/zend-diactoros#150 to track it there first - it would make sense for any PSR-7 implementation to reject invalid values, but as you can see, '' is indeed a valid string (from a consumer of the interface point of view), so I think re-validating here will also become necessary, unless somebody changes PSR-7 to have a RequestMethod VO :-P

[sagikazarmark]

but as you can see, '' is indeed a valid string (from a consumer of the interface point of view)

From PSR-7:

    /**
     * Return an instance with the provided HTTP method.
     *
     * While HTTP method names are typically all uppercase characters, HTTP
     * method names are case-sensitive and thus implementations SHOULD NOT
     * modify the given string.
     *
     * This method MUST be implemented in such a way as to retain the
     * immutability of the message, and MUST return an instance that has the
     * changed request method.
     *
     * @param string $method Case-sensitive method.
     * @return self
     * @throws \InvalidArgumentException for invalid HTTP methods.
     */
    public function withMethod($method);

Since the method is required (and for me empty string is not a valid method), validation should happen in requests. I think validating the request in ALL client implementations is kind of impossible.

What I could imagine, is that the invalid request gets rejected by the server. Not sure if in other clients it returns a human readable error message, in this case it clearly does not which we might improve. But again: adding the validation responsibility to ALL PSR-7 clients is not a viable way IMO.

[Ocramius]

validation should happen in requests

I agree, but then:

    /**
     * Retrieves the HTTP method of the request.
     *
     * @return string Returns the request method.
     */
    public function getMethod();

As you can see, no guarantees here, except for the value being a string. This is a weakness in the interface, and is a part of it that isn't really trustworthy (by "trusting" the return value, we lower the safety of the consumer as well).

I think validating the request in ALL client implementations is kind of impossible.

Not validating all of it, but just the weak parts of a request ("stringly-typed" stuff).

What I could imagine, is that the invalid request gets rejected by the server.

They will, but the request was invalid in-memory at this point, before even hitting a server. That is still to be solved in zendframework/zend-diactoros#150 in this case, but the lack of a strong type for a request method will lead to this sort of problem with any kind of PSR7 implementation at some point, due to the "stringly-typed" interface.

Not sure if in other clients it returns a human readable error message, in this case it clearly does not which we might improve.

This particular one was quite messy to debug.

is not a viable way IMO.

It might not be nice, but it would actually improve usability by a lot, since the interface itself doesn't defend users from this sort of mistake itself (limitation in the interface, I'm not blaming php-http here). I tend to be paranoid about anything that isn't a well-defined "value" type.

[sagikazarmark]

This is a weakness in the interface, and is a part of it that isn't really trustworthy (by "trusting" the return value, we lower the safety of the consumer as well).

You are right. I think we should also involve the FIG as we might not be the only ones who face this issue and would be better to have an "official" recommendation.

HTTPlug interfaces allow to throw invalid argument exceptions, so it might after all be the solution to handle these weekly typed stuff in clients. However, most of the client implementations are adapters, but a very low number of them wrap PSR-7 clients. Probably only Guzzle 6 for the moment. In these cases, I think it should rather solved be in the wrapped client, not the adapter itself.

Before doing any actions in this project, after(/if) we get an "official" statement from the FIG, we should apply the final proposed solution for every clients and should be discussed in the httplug repo as well.

[joelwurtz]

Or it could be in a plugin, we have in mind to have a ValidationPlugin, to validate a request on HTTP 1.0 or 1.1 standard. (like having the correct content-length header, transfer-encoding, and other stuffs like the method)

[mekras]

@sagikazarmark, can you take this issue to yourself? At least until it's time to make changes to the code.

[sagikazarmark]

Yes, I will contact the FIG and manage the discussion about this.

[Spomky]

Hi,

I have the same issue on my application. I cannot reach any url with a custom port.
Is there any solution?