Throw an exception before invalid response is parsed by getResponse()#1022
Throw an exception before invalid response is parsed by getResponse()#1022piotr-cz wants to merge 6 commits intojoomla:stagingfrom piotr-cz:patch-5
Conversation
to test: JHttpFactory::getHttp()->head('http://blah')
Parsing invalid response (false, "") throws PHP errors
|
In case of JHttpTransportStream, But I'm not sure if it's the right thing to supress the errors Edit: The check for no HTTP response that I added is supposed to handle it anyway see: http://php.net/manual/en/function.fopen.php |
|
There is 1 error in Unit test (http://developer.joomla.org/pulls/pulls/1022.html) but seems like it's triggered by something strange outside this PR Does this PR have any chance to be merged? There are few other issues in JHttp package but I decided to wait so this one can be still merged |
|
JHttpTransportCurl fixed by #1438 |
|
You'll need to update to master because of the curl changes and would you mind putting in some code to check $php_errormsg for the stream handler so that we can get some insight at least there why it failed? |
There was a problem hiding this comment.
What you are doing here is different than what I did in my pull request which relate to the warning from fsockopen() at line 256 similar to what you did with fopen() in stream.
There was a problem hiding this comment.
I've tested all transports against invalid url and var_dump'ed the result:
- for socket it was:
"" - for curl and stream:
false
So I've checked for empty response in the request method to be able to throw an exception in about the same place as in other transports.
Probably emphasizing an convention may not be optimal here as every transport is different.
|
@piotr-cz would you mind rebasing this so we can potentially getting it merged? I'm gonna want to give it a test drive for sure, but I'd love to have consistency here. Can you think of a way we might be able to test it? Note: I'm closing this until it is mergeable again, not because I'm rejecting the work. Please re-open the request when you get it ready to go. |
to test: JHttpFactory::getHttp()->head('http://blah')
Parsing invalid response (false, "") throws PHP errors as getResponse() assumes $content is consists headers and content.