Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CurlFactory] Prevent undefined offset when using array for ssl_key options #2348

Merged
merged 4 commits into from Oct 24, 2019

Conversation

andrewnclark
Copy link

Hopefully resolves #2339 where the ssl_key options are provided using the array format, but without a password as the second item in the array.

Still waiting conformation that this was the cause of the warnings, but this will avoid them going forward.

As always, feedback is appreciated.

@andrewnclark andrewnclark changed the title Test & changes to CurlFactory [CurlFactory] Prevent undefined offset when using array for ssl_key options Aug 8, 2019
@andrewnclark
Copy link
Author

Has since been confirmed that the cause was options defined as such:

$options = [
    'ssl_key' => ['/some/file/path']
]

This PR will prevent further warnings but the alternative is to validate properly that the options are both provided in the array or use a single string, thus making the above invalid and prompting a new release and update instructions.

@sagikazarmark
Copy link
Member

For maximum compatibility, I think it should be possible to use the array format without a key (even if it's not intended to be supported). We can deprecate that behavior though, and remove it in the next major release.

@andrewnclark are you up for making that change?

@sagikazarmark
Copy link
Member

Also, can you please rebase your branch from master?

@Nyholm Nyholm added this to the 6.5.0 milestone Oct 20, 2019
Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed the bug discovered by the test and I rebased the PR.

Copy link
Member

@sagikazarmark sagikazarmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Nyholm
Copy link
Member

Nyholm commented Oct 24, 2019

Thank you for the reviews and for the fix

@Nyholm Nyholm merged commit ac157c5 into guzzle:master Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Undefined offset: 1 in CurlFactory.php
3 participants