On Wed, Jul 3, 2024 at 11:01 AM Yann Ylavic <ylavic....@gmail.com> wrote:
>
> On Wed, Jul 3, 2024 at 10:57 AM Yann Ylavic <ylavic....@gmail.com> wrote:
> >
> > On Wed, Jul 3, 2024 at 8:58 AM Ruediger Pluem <rpl...@apache.org> wrote:
> > >
> > > On 7/3/24 2:59 AM, Yann Ylavic wrote:
> > > > On Tue, Jul 2, 2024 at 10:57 AM Ruediger Pluem <rpl...@apache.org>
> > > > wrote:
> > > >>
> > > >> Updated patch.
> > []
> > > >> const char *dump = "";
> > > >> if (APLOG_CS_IS_LEVEL(c, s, APLOG_TRACE7)) {
> > > >> if (argp != NULL)
> > > >> @@ -2400,23 +2413,28 @@
> > > >> dump = "(Oops, no memory buffer?)";
> > > >> }
> > > >> ap_log_cserror(APLOG_MARK, APLOG_TRACE4, 0, c, s,
> > > >> - "%s: %s %ld/%d bytes %s BIO#%pp [mem: %pp] %s",
> > > >> + "%s: %s %" APR_SIZE_T_FMT "/%" APR_SIZE_T_FMT
> > > >> + " bytes %s BIO#%pp [mem: %pp] %s",
> > > >> MODSSL_LIBRARY_NAME,
> > > >> (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "write" :
> > > >> "read"),
> > > >> - (long)rc, argi, (cmd ==
> > > >> (BIO_CB_WRITE|BIO_CB_RETURN) ? "to" : "from"),
> > > >> + actual_len, requested_len,
> > > >> + (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "to" :
> > > >> "from"),
> > > >> bio, argp, dump);
> > > >> if (*dump != '\0' && argp != NULL)
>
> Here we could check for APLOG_CS_IS_LEVEL(c, s, APLOG_TRACE7) too, to
> save the call when ssl_io_data_dump() is a noop anyway.
In this area, maybe we want to move the check for APLOG_TRACE4 from
the caller of modssl_set_io_callbacks() to the helper itself (which
knows better). Something like the attached eventually, feel free to
use since you are touching this code ;)
Index: modules/ssl/ssl_engine_io.c
===================================================================
--- modules/ssl/ssl_engine_io.c (revision 1918653)
+++ modules/ssl/ssl_engine_io.c (working copy)
@@ -2281,9 +2281,7 @@ apr_status_t ssl_io_filter_init(conn_rec *c, reque
apr_pool_cleanup_register(c->pool, (void*)filter_ctx,
ssl_io_filter_cleanup, apr_pool_cleanup_null);
- if (APLOG_CS_IS_LEVEL(c, mySrvFromConn(c), APLOG_TRACE4)) {
- modssl_set_io_callbacks(ssl);
- }
+ modssl_set_io_callbacks(ssl, c, mySrvFromConn(c));
return APR_SUCCESS;
}
@@ -2429,10 +2450,15 @@ static APR_INLINE void set_bio_callback(BIO *bio,
BIO_set_callback_arg(bio, arg);
}
-void modssl_set_io_callbacks(SSL *ssl)
+void modssl_set_io_callbacks(SSL *ssl, conn_rec *c, server_rec *s)
{
- BIO *rbio = SSL_get_rbio(ssl),
- *wbio = SSL_get_wbio(ssl);
+ BIO *rbio, *wbio;
+
+ if (!APLOG_CS_IS_LEVEL(c, s, APLOG_TRACE4))
+ return;
+
+ rbio = SSL_get_rbio(ssl);
+ wbio = SSL_get_wbio(ssl);
if (rbio) {
set_bio_callback(rbio, ssl);
}
Index: modules/ssl/ssl_engine_kernel.c
===================================================================
--- modules/ssl/ssl_engine_kernel.c (revision 1918653)
+++ modules/ssl/ssl_engine_kernel.c (working copy)
@@ -2607,9 +2607,7 @@ static int ssl_find_vhost(void *servername, conn_r
* (and the first vhost doesn't use APLOG_TRACE4), then
* we need to set that callback here.
*/
- if (APLOGtrace4(s)) {
- modssl_set_io_callbacks(ssl);
- }
+ modssl_set_io_callbacks(ssl, c, s);
return 1;
}
Index: modules/ssl/ssl_private.h
===================================================================
--- modules/ssl/ssl_private.h (revision 1918653)
+++ modules/ssl/ssl_private.h (working copy)
@@ -1053,7 +1053,7 @@ void modssl_callback_keylog(const SSL *ssl
/** I/O */
apr_status_t ssl_io_filter_init(conn_rec *, request_rec *r, SSL *);
void ssl_io_filter_register(apr_pool_t *);
-void modssl_set_io_callbacks(SSL *ssl);
+void modssl_set_io_callbacks(SSL *ssl, conn_rec *c, server_rec *s);
/* ssl_io_buffer_fill fills the setaside buffering of the HTTP request
* to allow an SSL renegotiation to take place. */
