On Wed, Jul 3, 2024 at 10:57 AM Yann Ylavic <[email protected]> wrote:
>
> On Wed, Jul 3, 2024 at 8:58 AM Ruediger Pluem <[email protected]> wrote:
> >
> > On 7/3/24 2:59 AM, Yann Ylavic wrote:
> > > On Tue, Jul 2, 2024 at 10:57 AM Ruediger Pluem <[email protected]> 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.
> > >> - ssl_io_data_dump(c, s, argp, rc);
> > >> + ssl_io_data_dump(c, s, argp, actual_len);
> > > And here:
> > > long dump_len = (actual_len >= APR_UINT16_MAX
> > > ? APR_UINT16_MAX
> > > : actual_len);
> >
> > Hm. As you point out: SSL records should be of limited length anyway, but
> > if we really truncate the output shouldn't we log this?
> > A log message like: Dump truncated to first XXX bytes?
>
> Something like this to initialize "dump" above?
>
> if (APLOG_CS_IS_LEVEL(c, s, APLOG_TRACE7)) {
> if (argp == NULL)
> dump = "(Oops, no memory buffer?)";
> else if (actual_len > MODSSL_IO_DUMP_MAX)
> dump = "(BIO dump follows, truncated to "
> APR_STRINGIFY(MODSSL_IO_DUMP_MAX) ")";
> else
> dump = "(BIO dump follows)";
> }
>
> where we'd "#define MODSSL_IO_DUMP_MAX APR_UINT16_MAX" and use it
> everywhere eventually.
>
> >
> > > ssl_io_data_dump(c, s, argp, dump_len);
> > >> }
> > >> else {
> > >> ap_log_cserror(APLOG_MARK, APLOG_TRACE4, 0, c, s,
> > >> - "%s: I/O error, %d bytes expected to %s on BIO#%pp
> > >> [mem: %pp]",
> > >> - MODSSL_LIBRARY_NAME, argi,
> > >> + "%s: I/O error, %" APR_SIZE_T_FMT
> > >> + " bytes expected to %s on BIO#%pp [mem: %pp]",
> > >> + MODSSL_LIBRARY_NAME, requested_len,
> > >> (cmd == (BIO_CB_WRITE|BIO_CB_RETURN) ? "write" :
> > >> "read"),
> > >> bio, argp);
> > >> }
> > >> }
> > >> return rc;
> > >> +#undef requested_len
> > >> +#undef actual_len
> > >> }
> > > ?
>
>
> Cheers;
> Yann.
