-
Notifications
You must be signed in to change notification settings - Fork 27
Open
Description
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 wantedThis 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.
Metadata
Metadata
Assignees
Type
Projects
Milestone
Relationships
Development
Select code repository
Activity
mekras commentedon Feb 16, 2016
I can't reproduce this.
port.php
test.php
Result:
Can you sniff requests with tcpdump or Wireshark?
Ocramius commentedon Feb 16, 2016
Getting following output:
The HTTP server reports:
Note: first 4 requests are from the browser
mekras commentedon Feb 16, 2016
And how about sniffing? I need the dump of the failed HTTP request.
Ocramius commentedon Feb 16, 2016
Here's a wireshark capture (attached)
capture2.zip
mekras commentedon Feb 16, 2016
Hm… I don't see any HTTP traffic in your dump.
Ocramius commentedon Feb 16, 2016
Yeah, that's the weird part. Not sure if it's just ext-curl messing it up a this point...
Ocramius commentedon Feb 16, 2016
Actually, the 9th packet includes some HTTP 1.1 stuff, but that's all there is...
mekras commentedon Feb 16, 2016
So far, I have no ideas…
mekras commentedon Feb 16, 2016
Can you try native PHP cURL functions?
Ocramius commentedon Feb 16, 2016
Native CURL seems to work:
I'll see if I have more time to dig into this at the end of the month - long weeks ahead :(
Ocramius commentedon Mar 5, 2016
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 notGET.@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 commentedon Mar 5, 2016
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 commentedon Mar 5, 2016
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 aRequestMethodVO :-Psagikazarmark commentedon Mar 5, 2016
From PSR-7:
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 commentedon Mar 5, 2016
I agree, but then:
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).
Not validating all of it, but just the weak parts of a request ("stringly-typed" stuff).
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.
This particular one was quite messy to debug.
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 commentedon Mar 7, 2016
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 commentedon Mar 7, 2016
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 commentedon Mar 9, 2016
@sagikazarmark, can you take this issue to yourself? At least until it's time to make changes to the code.
sagikazarmark commentedon Mar 9, 2016
Yes, I will contact the FIG and manage the discussion about this.
Spomky commentedon Jun 4, 2018
Hi,
I have the same issue on my application. I cannot reach any url with a custom port.
Is there any solution?