Skip to content

fix(http/client): prefer CURLOPT_PROTOCOLS_STR over deprecated bitmask constants#272

Merged
phil-davis merged 3 commits intosabre-io:masterfrom
ralflang:fix/curlopt-protocols-str
Apr 26, 2026
Merged

fix(http/client): prefer CURLOPT_PROTOCOLS_STR over deprecated bitmask constants#272
phil-davis merged 3 commits intosabre-io:masterfrom
ralflang:fix/curlopt-protocols-str

Conversation

@ralflang
Copy link
Copy Markdown
Contributor

CURLOPT_PROTOCOLS and CURLOPT_REDIR_PROTOCOLS are deprecated at the
libcurl level. When PHP eventually removes the bitmask constants the defined() guards will silently skip protocol restrictions.
This could once become a security issue. It's not an issue right now.
Prefer the string-based constants introduced in PHP 8.3 with fallback to bitmask for older versions.

This is not an urgent fix and I think it's BC

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.21%. Comparing base (fd137b7) to head (5c9a800).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #272      +/-   ##
============================================
+ Coverage     94.19%   94.21%   +0.01%     
- Complexity      262      264       +2     
============================================
  Files            15       15              
  Lines           879      881       +2     
============================================
+ Hits            828      830       +2     
  Misses           51       51              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…k constants

CURLOPT_PROTOCOLS and CURLOPT_REDIR_PROTOCOLS are deprecated at the
libcurl level. When PHP eventually removes the bitmask constants the defined() guards will silently skip protocol restrictions.
This could once become a security issue. It's not an issue right now.
Prefer the string-based constants introduced in PHP 8.3 with fallback to bitmask for older versions.

This is not an urgent fix and I think it's BC
@phil-davis phil-davis force-pushed the fix/curlopt-protocols-str branch from 9afb2d7 to d04997d Compare April 26, 2026 03:09
Comment thread lib/Client.php
// bitmask constants for older PHP versions. When PHP eventually
// removes the bitmask constants the string variant keeps protocol
// restrictions in place.
if (defined('CURLOPT_PROTOCOLS_STR') && defined('CURLOPT_REDIR_PROTOCOLS_STR')) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: phpstan doesn't know/understand/believe that these 2 constants always go together. So I added the 2nd "defined" so that the code explicitly checks for both existing.

Similar for line 420 below, and for code in ClientTest protocolSettings() method.

It does no harm, and keeps code analysis happy.

Copy link
Copy Markdown
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

LGTM. This will be useful for anyone who keeps using the releases that support PHP 7.4 through 8.*.

When we change the PHP support to 8.3+ then this can be refactored again.

@phil-davis phil-davis merged commit 25ea3af into sabre-io:master Apr 26, 2026
9 checks passed
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