Skip to content

sectransp: add support for HTTP/2 in gcc builds #16581

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

Closed
wants to merge 4 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Mar 5, 2025

Before this patch --http2 did not work in gcc builds with Secure
Transport, because ALPN relied on a compiler supporting the
HAVE_BUILTIN_AVAILABLE aka __builtin_available() feature. This
is clang-specific and missing from gcc (as of gcc v14).

Add support for ALPN and HTTP/2 when this compiler feature is missing.

Also drop test exceptions from GHA/macos in CI.

Follow-up to 092f681
Ref: c349bd6 #14097 (issue 15.)
Ref: #4314


@vszakats vszakats added HTTP/2 appleOS specific to an Apple operating system labels Mar 5, 2025
@github-actions github-actions bot added the CI Continuous Integration label Mar 5, 2025
@vszakats vszakats changed the title sectransp: allow HTTP/2 in gcc builds sectransp: add support for HTTP/2 in gcc builds Mar 6, 2025
@vszakats vszakats closed this in 2d94439 Mar 6, 2025
@vszakats vszakats deleted the sectrs-gcc-h2 branch March 6, 2025 19:34
@EricFromCanada
Copy link

This breaks building curl on macOS versions before High Sierra, because it ends up in the new #else block that's intended for GCC, which fails because Secure Transport in Sierra and earlier lacks ALPN support:

libtool: compile:  clang -DHAVE_CONFIG_H -I../include -I../lib -I../lib -DBUILDING_LIBCURL -DCURL_STATICLIB -DUNITTESTS -isystem /usr/local/opt/zlib/include -isystem /usr/local/Cellar/openssl@3/3.4.1/include -isystem /usr/local/Cellar/libssh2/1.11.1/include -isystem /usr/local/Cellar/openssl@3/3.4.1/include -isystem /usr/local/opt/zlib/include -isystem /usr/local/Cellar/rtmpdump/2.4-20151223_3/include -isystem /usr/local/Cellar/openssl@3/3.4.1/include -isystem /usr/local/Cellar/libidn2/2.3.8/include -isystem /usr/local/Cellar/libnghttp2/1.65.0/include -Qunused-arguments -Werror-implicit-function-declaration -O2 -Werror=partial-availability -c vtls/vtls_scache.c  -fno-common -DPIC -o vtls/.libs/libcurlu_la-vtls_scache.o
vtls/sectransp.c:1099:9: error: use of undeclared identifier 'SSLSetALPNProtocols'
    if(&SSLSetALPNProtocols && &SSLCopyALPNProtocols) {
        ^
vtls/sectransp.c:1099:33: error: use of undeclared identifier 'SSLCopyALPNProtocols'
    if(&SSLSetALPNProtocols && &SSLCopyALPNProtocols) {
                                ^
libtool: compile:  clang -DHAVE_CONFIG_H -I../include -I../lib -I../lib -DBUILDING_LIBCURL -DCURL_STATICLIB -DUNITTESTS -isystem /usr/local/opt/zlib/include -isystem /usr/local/Cellar/openssl@3/3.4.1/include -isystem /usr/local/Cellar/libssh2/1.11.1/include -isystem /usr/local/Cellar/openssl@3/3.4.1/include -isystem /usr/local/opt/zlib/include -isystem /usr/local/Cellar/rtmpdump/2.4-20151223_3/include -isystem /usr/local/Cellar/openssl@3/3.4.1/include -isystem /usr/local/Cellar/libidn2/2.3.8/include -isystem /usr/local/Cellar/libnghttp2/1.65.0/include -Qunused-arguments -Werror-implicit-function-declaration -O2 -Werror=partial-availability -c vtls/vtls_scache.c -o vtls/libcurlu_la-vtls_scache.o >/dev/null 2>&1
vtls/sectransp.c:2099:35: error: use of undeclared identifier 'SSLCopyALPNProtocols'; did you mean 'SSLSetALPNProtocols'?
      if(&SSLSetALPNProtocols && &SSLCopyALPNProtocols) {
                                  ^~~~~~~~~~~~~~~~~~~~
                                  SSLSetALPNProtocols
vtls/sectransp.c:1114:13: note: 'SSLSetALPNProtocols' declared here
      err = SSLSetALPNProtocols(backend->ssl_ctx, alpnArr);
            ^
3 errors generated.
make[1]: *** [vtls/libcurlu_la-sectransp.lo] Error 1

@vszakats
Copy link
Member Author

@EricFromCanada Does this patch fix it for you?

diff --git a/lib/vtls/sectransp.c b/lib/vtls/sectransp.c
index f0094df584..3e491adf37 100644
--- a/lib/vtls/sectransp.c
+++ b/lib/vtls/sectransp.c
@@ -1092,8 +1092,8 @@ static CURLcode sectransp_connect_step1(struct Curl_cfilter *cf,
     return result;
 
   if(connssl->alpn) {
-#if (CURL_BUILD_MAC_10_13 || CURL_BUILD_IOS_11) && \
-    defined(HAVE_BUILTIN_AVAILABLE)
+#if CURL_BUILD_MAC_10_13 || CURL_BUILD_IOS_11
+#ifdef HAVE_BUILTIN_AVAILABLE
     if(__builtin_available(macOS 10.13.4, iOS 11, tvOS 11, *)) {
 #else
     if(&SSLSetALPNProtocols && &SSLCopyALPNProtocols) {
@@ -1119,6 +1119,7 @@ static CURLcode sectransp_connect_step1(struct Curl_cfilter *cf,
       Curl_alpn_to_proto_str(&proto, connssl->alpn);
       infof(data, VTLS_INFOF_ALPN_OFFER_1STR, proto.data);
     }
+#endif /* CURL_BUILD_MAC_10_13 || CURL_BUILD_IOS_11 */
   }
 
   if(ssl_config->key) {
@@ -2092,8 +2093,8 @@ check_handshake:
     }
 
     if(connssl->alpn) {
-#if (CURL_BUILD_MAC_10_13 || CURL_BUILD_IOS_11) && \
-    defined(HAVE_BUILTIN_AVAILABLE)
+#if CURL_BUILD_MAC_10_13 || CURL_BUILD_IOS_11
+#ifdef HAVE_BUILTIN_AVAILABLE
       if(__builtin_available(macOS 10.13.4, iOS 11, tvOS 11, *)) {
 #else
       if(&SSLSetALPNProtocols && &SSLCopyALPNProtocols) {
@@ -2124,6 +2125,7 @@ check_handshake:
         if(alpnArr)
           CFRelease(alpnArr);
       }
+#endif /* CURL_BUILD_MAC_10_13 || CURL_BUILD_IOS_11 */
     }
 
     return CURLE_OK;

vszakats added a commit to vszakats/curl that referenced this pull request Apr 25, 2025
Reported-by: Eric Knibbe
Bug: curl#16581 (comment)
Regression from 2d94439 curl#16581
@EricFromCanada
Copy link

@vszakats Yes it does; tested on 10.11 and 10.12 via Homebrew.

@vszakats
Copy link
Member Author

Thanks Eric! Planning to merge this soon via #17193.

vszakats added a commit that referenced this pull request Apr 25, 2025
Reported-by: Eric Knibbe
Bug: #16581 (comment)
Regression from 2d94439 #16581

Closes #17193
nbaws pushed a commit to nbaws/curl that referenced this pull request Apr 26, 2025
pps83 pushed a commit to pps83/curl that referenced this pull request Apr 26, 2025
Before this patch `--http2` did not work in gcc builds with Secure
Transport, because ALPN relied on a compiler supporting the
`HAVE_BUILTIN_AVAILABLE` aka `__builtin_available()` feature. This
is clang-specific and missing from gcc (as of gcc v14).

Add support for ALPN and HTTP/2 when this compiler feature is missing.

Also drop test exceptions from GHA/macos in CI.

Follow-up to 092f681
Ref: c349bd6 curl#14097 (issue 15.)
Ref: curl#4314

Closes curl#16581
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
appleOS specific to an Apple operating system CI Continuous Integration HTTP/2
Development

Successfully merging this pull request may close these issues.

2 participants