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, update and prettify cURL information: phpinfo() and curl_version() #17848

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

nono303
Copy link
Contributor

@nono303 nono303 commented Feb 18, 2025

  • Remove empty, null & deprecated values
  • Ensure displaying human readable values
  • Fix typo (like ares version was displayed as ZLib)
  • Fix non-working condition with CURLVERSION_NOW using age
  • Use feature_names if available instead of const feats[] and display features as list (like for protocols)
  • Update (legacy) duplicated const feats[] with CURL_VERSION_CURLDEBUG, CURL_VERSION_THREADSAFE
  • Display SSL supported backends
  • Add new supported protocols (Brotli, Nghttp2, QUIC, Zstd, Hyper, GNU SASL, RTMP)
  • Return protocols module versions in an array for curl_version()

Result with same cURL 8.12.1 and PHP 8.4.4:

phpinfo()

before

phpinfo_before

after

phpinfo_after

curl_version()

before

[version_number] => 527361  
[age] => 11  
[features] => 1575833501  
[feature_list] => Array  
(  
    [AsynchDNS] => 1  
    [CharConv] =>  
    [Debug] =>  
    [GSS-Negotiate] =>  
    [IDN] => 1  
    [IPv6] => 1  
    [krb4] =>  
    [Largefile] => 1  
    [libz] => 1  
    [NTLM] => 1  
    [NTLMWB] =>  
    [SPNEGO] => 1  
    [SSL] => 1  
    [SSPI] => 1  
    [TLS-SRP] => 1  
    [HTTP2] => 1  
    [GSSAPI] =>  
    [KERBEROS5] => 1  
    [UNIX_SOCKETS] => 1  
    [PSL] =>  
    [HTTPS_PROXY] => 1  
    [MULTI_SSL] => 1  
    [BROTLI] => 1  
    [ALTSVC] => 1  
    [HTTP3] =>  
    [UNICODE] => 1  
    [ZSTD] => 1  
    [HSTS] => 1  
    [GSASL] =>  
)  
  
[ssl_version_number] => 0  
[version] => 8.12.1-DEV  
[host] => Windows  
[ssl_version] => (OpenSSL/3.4.1) Schannel  
[libz_version] => 1.3.1  
[protocols] => Array  
(  
    [0] => dict  
    [1] => file  
    [2] => ftp  
    [3] => ftps  
    [4] => gopher  
    [5] => gophers  
    [6] => http  
    [7] => https  
    [8] => imap  
    [9] => imaps  
    [10] => ldap  
    [11] => ldaps  
    [12] => mqtt  
    [13] => pop3  
    [14] => pop3s  
    [15] => rtsp  
    [16] => scp  
    [17] => sftp  
    [18] => smb  
    [19] => smbs  
    [20] => smtp  
    [21] => smtps  
    [22] => telnet  
    [23] => tftp  
    [24] => ws  
    [25] => wss  
)  
  
[ares] => 1.34.4  
[ares_num] => 74244  
[libidn] =>  
[iconv_ver_num] => 0  
[libssh_version] => libssh2/1.11.1_DEV  
[brotli_ver_num] => 16781312  
[brotli_version] => brotli/1.1.0  

after

[version] => 8.12.1-DEV  
[version_number] => 527361  
[age] => 11  
[host] => Windows  
[features] => 1575833501  
[feature_list] => Array  
(  
    [0] => alt-svc  
    [1] => AsynchDNS  
    [2] => brotli  
    [3] => HSTS  
    [4] => HTTP2  
    [5] => HTTPS-proxy  
    [6] => HTTPSRR  
    [7] => IDN  
    [8] => IPv6  
    [9] => Kerberos  
    [10] => Largefile  
    [11] => libz  
    [12] => MultiSSL  
    [13] => NTLM  
    [14] => SPNEGO  
    [15] => SSL  
    [16] => SSLS-EXPORT  
    [17] => SSPI  
    [18] => threadsafe  
    [19] => TLS-SRP  
    [20] => Unicode  
    [21] => UnixSockets  
    [22] => zstd  
)  
  
[protocols] => Array  
(  
    [0] => dict  
    [1] => file  
    [2] => ftp  
    [3] => ftps  
    [4] => gopher  
    [5] => gophers  
    [6] => http  
    [7] => https  
    [8] => imap  
    [9] => imaps  
    [10] => ldap  
    [11] => ldaps  
    [12] => mqtt  
    [13] => pop3  
    [14] => pop3s  
    [15] => rtsp  
    [16] => scp  
    [17] => sftp  
    [18] => smb  
    [19] => smbs  
    [20] => smtp  
    [21] => smtps  
    [22] => telnet  
    [23] => tftp  
    [24] => ws  
    [25] => wss  
)  
  
[ssl_backends] => Array  
(  
    [0] => openssl  
    [1] => schannel  
)  
  
[feature_version] => Array  
(  
    [ssl] => OpenSSL/3.4.1 (Schannel)  
    [libz] => 1.3.1  
    [ares] => 1.34.4  
    [libssh] => libssh2/1.11.1_DEV  
    [brotli] => brotli/1.1.0  
    [nghttp2] => 1.64.0  
    [zstd] => zstd/1.5.6  
)  

- put back long: version_number & features
- keep original key: feature_list
@cmb69
Copy link
Member

cmb69 commented Feb 18, 2025

Thank you for the PR! I'm generally in favor of these changes, but we need to cater to BC, so ideally all tests should pass without needing to be modified. It might also make sense to explicitly check the changes against libcurl 7.61.0 which is the oldest version we're supporting (I presume that none of the CI jobs runs against such an old version).

Minor issue: it might be reasonable to not introduce unrelated whitespace changes (these can be done in a separate PR, if at all).

@cmb69
Copy link
Member

cmb69 commented Feb 18, 2025

cc @Ayesh

@nono303
Copy link
Contributor Author

nono303 commented Feb 18, 2025

Thx @cmb69 for your feedback!

ll tests should pass without needing to be modified

I'll have a look on what failed

It might also make sense to explicitly check the changes against libcurl 7.61.0

Hmmmm... actually and out of my PR with php 8.4.4, libcurl 7.61.0 doesn't work

  • curl_easy_upkeep required by ext/curl are is not present (at least exposed by shared libcurl) so it'll be difficult for me to do this check
curl 7.61.0-DEV (Windows) libcurl/7.61.0-DEV OpenSSL/3.4.0a (WinSSL) zlib/1.3.1 brotli/1.1.0 c-ares/1.34.4 libssh2/1.11.1_DEV nghttp2/1.64.0
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher http https imap imaps ldap pop3 pop3s rtsp scp sftp smtp smtps telnet tftp
Features: AsynchDNS IPv6 Largefile SSPI Kerberos SPNEGO NTLM SSL libz brotli HTTP2 HTTPS-proxy MultiSSL

not introduce unrelated whitespace changes

I does it for my change (only space after \n...) but as I saw some other few inconsistencies, I've just fixed them afterwards... but I can revert it if needed

@cmb69
Copy link
Member

cmb69 commented Feb 18, 2025

I'll have a look on what failed

Thanks!

curl_easy_upkeep required by ext/curl are is not present (at least exposed by shared libcurl) so it'll be difficult for me to do this check

The code using curl_easy_upkeep() is not supposed to be compiled when built against libcurl < 7.62.0. Anyway, it's not strictly necessary that you do this check; it's just that somebody better should double-check it (I don't think we can bump the cURL requirement further for now). If need be, I think I can do that.

I does it for my change (only space after \n...) but as I saw some other few inconsistencies, I've just fixed them afterwards... but I can revert it if needed

I, personally, would prefer if you revert the unrelated whitespace changes (keeps the diff a bit smaller, and more focused). But maybe wait a while what others think about that.

@nono303
Copy link
Contributor Author

nono303 commented Feb 19, 2025

... I just temporary commented part of curl_easy_upkeep() to check against 7.61 ;)

Looking to the failing test, I would have to rollback Fix non-working condition with CURLVERSION_NOW using age curl_version_info_data->age to CURLVERSION_XXX testing but as mentionned ,their is an issue:
only CURLVERSION_NOW is defined. All other (CURLVERSION_FIRST...) are well declared in curl.h CURLversion enum but not defined.

🔴 This is why all test #if defined(CURLVERSION_XXX) && CURLVERSION_NOW >= CURLVERSION_XXX never worked and module version were not displayed in both phpinfo() and curl_version()

#if defined(CURLVERSION_NOW)
	#pragma message ("*** CURLVERSION_NOW defined")
#else
	#pragma message ("*** CURLVERSION_NOW NOT defined")
#endif
#if defined(CURLVERSION_SECOND)
	#pragma message ("*** CURLVERSION_SECOND defined")
#else
	#pragma message ("*** CURLVERSION_SECOND NOT defined")
#endif
2025-02-19_09-26-51 ####### BEGIN BUILD MODULE 'php_curl' vs17 x64 avx2 ###########################
curl_file.c
interface.c
multi.c
share.c
*** CURLVERSION_NOW defined
*** CURLVERSION_SECOND NOT defined
   Creating library C:\Program Files\php\php_curl.lib and object C:\Program Files\php\php_curl.exp
Generating code
Finished generating code

Does I missed something??

nono303 added 5 commits February 19, 2025 12:43
BC:
 - remove ssl_version_number: not used anymore, always 0 https://github.com/curl/curl/blob/master/include/curl/curl.h#L3089
 - ["ssl_version"] & ["libz_version"] moved to ["features_version"]["ssl"] & ["features_version"]["libz"]
@nono303
Copy link
Contributor Author

nono303 commented Feb 20, 2025

❓ Would adding this to curl_private.h make sense (cf. #17848 (comment))

#define PHP_CURLVERSION_TWELFTH=	LIBCURL_VERSION_NUM >= 0x080800 /* Available since 8.8.0	*/	
#define PHP_CURLVERSION_ELEVENTH =	LIBCURL_VERSION_NUM >= 0x075700 /* Available since 7.87.0	*/	
#define PHP_CURLVERSION_TENTH =		LIBCURL_VERSION_NUM >= 0x074d00 /* Available since 7.77.0	*/	
#define PHP_CURLVERSION_NINTH =		LIBCURL_VERSION_NUM >= 0x074b00 /* Available since 7.75.0	*/	
#define PHP_CURLVERSION_EIGHTH =	LIBCURL_VERSION_NUM >= 0x074800 /* Available since 7.72.0	*/	
#define PHP_CURLVERSION_SEVENTH  =	LIBCURL_VERSION_NUM >= 0x074600 /* Available since 7.71.0	*/	
#define PHP_CURLVERSION_SIXTH =		LIBCURL_VERSION_NUM >= 0x074200 /* Available since 7.66.0	*/	
#define PHP_CURLVERSION_FIFTH =		LIBCURL_VERSION_NUM >= 0x073900 /* Available since 7.57.0	*/	
#define PHP_CURLVERSION_FOURTH =	LIBCURL_VERSION_NUM >= 0x071001 /* Available since 7.16.1	*/	
#define PHP_CURLVERSION_THIRD =		LIBCURL_VERSION_NUM >= 0x070C00 /* Available since 7.12.0	*/	
#define PHP_CURLVERSION_SECOND =	LIBCURL_VERSION_NUM >= 0x070B01 /* Available since 7.11.1	*/	
#define PHP_CURLVERSION_FIRST =		LIBCURL_VERSION_NUM >= 0x070A00 /* Available since 7.10		*/

@cmb69
Copy link
Member

cmb69 commented Feb 20, 2025

Would adding this to curl_private.h make sense

While I don't find the hexadecimal numeric cURL version number not super readable, CURLVERSION_* is incomprehensable. I would stick with the hard-coded numbers.

@nono303
Copy link
Contributor Author

nono303 commented Feb 25, 2025

Hi @cmb69 @adoy
I think I’ve come to the end of this PR concerning fix, upgrade, improve & testing (including cURL 7.61.0 regression test validated).
So now, could you please give me your opinion on the validity of the “breaking” changes made
Tests had been updated to fit this state, in summary:

phpinfo()

  • cURL >= 7.87.0 All linked libcurl available features are displayed in a single line, comma separated
    • cURL < 7.87.0 keep the original hardcoded features set displaying each one by line with "enabled cell Yes/no"

curl_version()

  • cURL >= 7.87.0 only linked libcurl features are appended as key in feature_list array, value is always true
    • cURL < 7.87.0 keep the original hardcoded features set, appending each one as key with value true / false)
  • ssl_version_number removed as it’s always 0 (cf. https://github.com/curl/curl/blob/master/include/curl/curl.h#L3086)
  • all feature version listed at root of returned array are now appended in a sub-array feature_version with standard lowercase feature name as key and human readable version as value.

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.

2 participants