You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@serf.apache.org by ds...@apache.org on 2025/07/16 19:34:43 UTC
svn commit: r1927273 - /serf/trunk/buckets/ssl_buckets.c
Author: dsahlberg
Date: Wed Jul 16 19:34:43 2025
New Revision: 1927273
URL: http://svn.apache.org/viewvc?rev=1927273&view=rev
Log:
Followup to r1926972, tighten up the code slightly.
* buckets/ssl_buckets.c
(log_ssl_error): Skip an unnecessary copy of the error message to the stack
- the consumer anyway has to make a copy in the callback.
(ssl_need_client_cert): Decrease scope for err variable.
Review by: gstein (see GitHub PR#9)
Modified:
serf/trunk/buckets/ssl_buckets.c
Modified: serf/trunk/buckets/ssl_buckets.c
URL: http://svn.apache.org/viewvc/serf/trunk/buckets/ssl_buckets.c?rev=1927273&r1=1927272&r2=1927273&view=diff
==============================================================================
--- serf/trunk/buckets/ssl_buckets.c (original)
+++ serf/trunk/buckets/ssl_buckets.c Wed Jul 16 19:34:43 2025
@@ -362,9 +362,8 @@ static void log_ssl_error(serf_ssl_conte
while ((err = ERR_get_error())) {
if (err && ctx->error_callback) {
- char ebuf[256];
- ERR_error_string_n(err, ebuf, sizeof(ebuf));
- ctx->error_callback(ctx->error_baton, ctx->fatal_err, ebuf);
+ char *errstr = ERR_error_string(err, NULL);
+ ctx->error_callback(ctx->error_baton, ctx->fatal_err, errstr);
}
}
@@ -1608,7 +1607,6 @@ static int ssl_pass_cb(UI *ui, UI_STRING
static int ssl_need_client_cert(SSL *ssl, X509 **cert, EVP_PKEY **pkey)
{
serf_ssl_context_t *ctx = SSL_get_app_data(ssl);
- unsigned long err = 0;
#if defined(SERF_HAVE_OSSL_STORE_OPEN_EX)
STACK_OF(X509) *leaves;
STACK_OF(X509) *intermediates;
@@ -1919,7 +1917,7 @@ static int ssl_need_client_cert(SSL *ssl
return 1;
}
else {
- err = ERR_get_error();
+ unsigned long err = ERR_get_error();
ERR_clear_error();
if (ERR_GET_LIB(err) == ERR_LIB_PKCS12 &&
ERR_GET_REASON(err) == PKCS12_R_MAC_VERIFY_FAILURE) {
Re: svn commit: r1927273 - /serf/trunk/buckets/ssl_buckets.c
Posted by Branko Čibej <br...@apache.org>.
On 16. 7. 25 21:34, dsahlberg@apache.org wrote:
> Author: dsahlberg
> Date: Wed Jul 16 19:34:43 2025
> New Revision: 1927273
>
> URL:http://svn.apache.org/viewvc?rev=1927273&view=rev
> Log:
> Followup to r1926972, tighten up the code slightly.
>
> * buckets/ssl_buckets.c
> (log_ssl_error): Skip an unnecessary copy of the error message to the stack
> - the consumer anyway has to make a copy in the callback.
> (ssl_need_client_cert): Decrease scope for err variable.
>
> Review by: gstein (see GitHub PR#9)
>
>
> Modified:
> serf/trunk/buckets/ssl_buckets.c
>
> Modified: serf/trunk/buckets/ssl_buckets.c
> URL:http://svn.apache.org/viewvc/serf/trunk/buckets/ssl_buckets.c?rev=1927273&r1=1927272&r2=1927273&view=diff
> ==============================================================================
> --- serf/trunk/buckets/ssl_buckets.c (original)
> +++ serf/trunk/buckets/ssl_buckets.c Wed Jul 16 19:34:43 2025
> @@ -362,9 +362,8 @@ static void log_ssl_error(serf_ssl_conte
> while ((err = ERR_get_error())) {
>
> if (err && ctx->error_callback) {
> - char ebuf[256];
> - ERR_error_string_n(err, ebuf, sizeof(ebuf));
> - ctx->error_callback(ctx->error_baton, ctx->fatal_err, ebuf);
> + char *errstr = ERR_error_string(err, NULL);
> + ctx->error_callback(ctx->error_baton, ctx->fatal_err, errstr);
> }
It would appear that this copy wasn't unnecessary after all.
https://docs.openssl.org/master/man3/ERR_error_string/
> ERR_error_string() generates a human-readable string representing the
> error code/e/, and places it at/buf/./buf/must be at least 256 bytes
> long. If/buf/is*NULL*, the error string is placed in a static buffer.
> Note that this function is not thread-safe and does no checks on the
> size of the buffer; use ERR_error_string_n() instead.
"Not thread safe static buffer" is a non-starter. This hunk should be
reverted.
-- Brane