Add socket and SSL/TLS context options to connectors #52
Conversation
|
I've just rebased this on the current master now that #46 is in. Ready for review :) |
|
LGTM |
|
I've just rebased this now that #54 is in and requires full HHVM support. |
| @@ -73,4 +74,53 @@ public function gettingEncryptedStuffFromGoogleShouldWork() | |||
| $this->assertTrue($connected); | |||
| $this->assertRegExp('#^HTTP/1\.0#', $response); | |||
| } | |||
|
|
|||
| /** @test */ | |||
| public function testSelfSignedRejectsIfVerificationIsEnabled() | |||
There was a problem hiding this comment.
This test is failing for me on OS X 10.11 with PHP 5.5.30
There was a problem hiding this comment.
This is strange indeed…
Can you try running the following code to verify this is in line with what PHP's stream handlers do?
file_get_contents('https://self-signed.badssl.com/', false, stream_context_create(array('ssl'=>array('verify_peer'=>false))));
// all okay
file_get_contents('https://self-signed.badssl.com/', false, stream_context_create(array('ssl'=>array('verify_peer'=>true))));
// PHP warning: file_get_contents(): SSL operation failed with code 1. OpenSSL Error messages:
// error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed on line 1There was a problem hiding this comment.
Yup. Those worked as you expected.
There was a problem hiding this comment.
Can you give some error output for the failing test?
Unfortunately I do not have a Mac to test against, so how else could I assist you tracking this down?
As an alternative, how about skipping this test on Mac for now (similar to how we skip certain tests for HHVM)?
There was a problem hiding this comment.
I'd rather not just skip because it's failing. HHVM is skipped because the future is not supported in the language.
There was a problem hiding this comment.
After talking to @cboden, we agree that this is probably a bug in certain (older) versions of PHP, which we have yet to triage further.
This test DOES work correctly on ALL PHP versions tested via Travis.
This test does NOT work on PHP 5.5.30 on Mac, but DOES work with PHP 7.? on Mac.
Note that this test failure is not related to this library at all, we're currently looking into providing a test script which highlights this issue in the underlying SSL stream wrapper.
Let's keep track of this in #59 instead and get this PR in 
|
Hey guys, This should be merge in 0.4.x ASAP as there is no bc break... And it's a crutial missing feature since PHP 5.6. |
This PR builds on top of #46 which does in fact contain a BC break, as such, this will likely land in v0.5.0 soon. See the above remark, other than that I consider this ready to merge |
I think it might make sense to backport some of these fixes to a v0.4.x release, see #60 for more details. Either way, I now consider this ready to merge |
This is an alternative approach to #17 and #45 in order to achieve a better separation of concerns and to leave the current API unchanged. See also the discussion in #4 for some background.
This PR builds on top of #46, so the diff also includes its changes. Look here for changes only related to this PR alone.(no longer applies after rebasing)Fixes #4
Supersedes/closes #45
Supersedes/closes #17