Skip to content

Conversation

@x2018
Copy link
Contributor

@x2018 x2018 commented Nov 4, 2025

This PR does the following things:

  1. Update the description (based on gnutls_global_init()) of gtls_init() and properly check its status in gtls_client_init().

gtls_init() should only be called in curl_global_init()->Curl_ssl_init() and the calling in gtls_client_init() itself is redundant?

  1. In gtls_client_init(), check the invaild SSLVERSION at first. Note that this part refactors the duplicate/incompatible checks and removes the current useless local variable sni.
  2. Check the return value of gnutls_ocsp_resp_init(). Although the original code is safe because gnutls_ocsp_resp_import() will check the validity of ocsp_resp, it is better to catch the error in time and record the proper message to output log.

@github-actions github-actions bot added the TLS label Nov 4, 2025
@x2018 x2018 marked this pull request as draft November 4, 2025 18:45
This commit does the following things:
1. Update the description of gtls_init() and properly check its status
   in gtls_client_init().
2. In gtls_client_init(), check the invaild SSLVERSION at first. Note
   that this part refactors the duplicate/incompatible checks and removes
   the useless local variable `sni`.
3. Check the return value of gnutls_ocsp_resp_init(). Although the
   original code is safe because gnutls_ocsp_resp_import() will check
   the validity of `ocsp_resp`, it is better to catch the error in time
   and record the proper message to output log.
@x2018 x2018 marked this pull request as ready for review November 4, 2025 19:38
@bagder
Copy link
Member

bagder commented Nov 4, 2025

Note that clang-tidy still complains:

/home/runner/work/curl/curl/bld/lib/../../lib/hmac.c:171:3: error: Initialized va_list is leaked [clang-analyzer-valist.Unterminated,-warnings-as-errors]
  171 |   return CURLE_OK;
      |   ^
/home/runner/work/curl/curl/bld/lib/../../lib/hmac.c:162:6: note: Assuming 'ctxt' is non-null
  162 |   if(!ctxt)
      |      ^~~~~
/home/runner/work/curl/curl/bld/lib/../../lib/hmac.c:162:3: note: Taking false branch
  162 |   if(!ctxt)
      |   ^
/home/runner/work/curl/curl/bld/lib/../../lib/hmac.c:169:3: note: Initialized va_list
  169 |   Curl_HMAC_final(ctxt, output);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/curl/curl/bld/lib/../../lib/hmac.c:171:3: note: Initialized va_list is leaked
  171 |   return CURLE_OK;
      |   ^

@x2018
Copy link
Contributor Author

x2018 commented Nov 5, 2025

It should be a false positive because 066f571 is ok and 5dfaedc just cleanup a piece of outdated note.
Let me find a typo or something similar to try the check again.

@bagder bagder closed this in 2db36f1 Nov 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants