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

Fix invalid curl_setopt (v2) #614

Merged
merged 5 commits into from
Nov 5, 2020
Merged

Fix invalid curl_setopt (v2) #614

merged 5 commits into from
Nov 5, 2020

Conversation

imjoehaines
Copy link
Contributor

Goal

In PHP 8, curl_setopt will now throw if an undefined option is used. We were using CURL_IPRESOLVE_V4 as an option, when it's actually a value, so all error reports failed before being sent

AFAICT this has always been an error, but earlier PHP version silently ignored it — see the docs from 2013

We need to make significant changes to the test suite to run the tests on newer PHP versions. We haven't don that now as this change is so small and isn't tested in the test suite anyway

Changeset

  • Fixed curl_setop usage
  • Ignore fixtures on StyleCI so it passes
  • Fix installing PHP 5.4/5.5 on Travis by using an older dist
  • Fix HHVM 3.6 on Travis by using an older composer version

Testing

This was tested manually as the test suite doesn't cover it — the changed function is always mocked

This is now a fatal error in PHP 8
These are no longer installable on the default
Newer composer versions hang forever when installing. We also can't
rollback to a specific version because trying to do so throws an error
(CURLOPT_PROTOCOLS isn't defined in HHVM 3.6)
@imjoehaines imjoehaines requested a review from kattrali November 3, 2020 16:58
@imjoehaines imjoehaines merged commit 6c7d6c7 into 2.10 Nov 5, 2020
@kattrali kattrali deleted the fix/curl-setopt-v2 branch November 5, 2020 15:57
@imjoehaines imjoehaines mentioned this pull request Nov 17, 2020
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.

2 participants