Skip to content

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

Merged
Nyholm merged 4 commits intoguzzle:masterfrom
andrewnclark:issue-2339
Oct 24, 2019
Merged

[CurlFactory] Prevent undefined offset when using array for ssl_key options#2348
Nyholm merged 4 commits intoguzzle:masterfrom
andrewnclark:issue-2339

Conversation

@andrewnclark
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

@Nyholm Nyholm requested a review from sagikazarmark October 23, 2019 19:56
Copy link
Copy Markdown
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
Copy Markdown
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