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

Respect provided verify option in Utils\http_request #5507

Conversation

aaemnnosttv
Copy link
Contributor

This PR fixes #5463 in that it respects the given value of $options['verify'] rather than overwriting it with the path to the default CA cert.

It does this partly by extracting the default CA cert detection/lookup into a new utility: get_default_cacert.

The tests added also include a very simple Mock_Requests_Transport class to make testing the http_request call much cleaner and simpler, but is only used in the added test. Regarding the way this class is loaded, normally I would expect it to be autoloaded but there didn't seem to be any existing configuration for this yet so I left that out for now for simplicity.

@aaemnnosttv aaemnnosttv marked this pull request as ready for review March 21, 2021 14:15
@aaemnnosttv aaemnnosttv requested a review from a team as a code owner March 21, 2021 14:15
@schlessera schlessera force-pushed the fix/5463-utils-http-request-verify-option branch from 8bca8ac to 398de67 Compare May 5, 2021 08:36
@schlessera
Copy link
Member

I rebased and adapted this PR to take the new default behavior from #5514 into account.

The main change is that WP-CLI will now default to the system certificate and only fall back to the bundled one if the system-wide certificate failed.

@schlessera schlessera merged commit 84ddf49 into wp-cli:master May 5, 2021
@schlessera schlessera changed the title Fix 5463 Utils\http_request verify option Respect provided verify option in Utils\http_request May 5, 2021
@schlessera
Copy link
Member

Thanks for the PR, @aaemnnosttv !

@aaemnnosttv aaemnnosttv deleted the fix/5463-utils-http-request-verify-option branch May 5, 2021 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

http_request utility ignores verify option
2 participants