Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (athena.apache.org: local policy)
Date: Wed, 9 Jun 2010 21:55:19 +0200 (CEST)
From: Stefan Fritsch <sf@sfritsch.de>
To: dev@httpd.apache.org
Subject: Re: svn commit: r951896 - in /httpd/httpd/trunk/modules/ssl:
 ssl_engine_io.c ssl_engine_kernel.c
In-Reply-To: <4C0FD556.9080904@apache.org>
Message-ID: <alpine.DEB.1.10.1006092136530.7651@eru.sfritsch.de>
References: <20100606170130.5519923888CB@eris.apache.org>
 <4C0FD556.9080904@apache.org>
User-Agent: Alpine 1.10 (DEB 962 2008-03-14)
MIME-Version: 1.0
Content-Type: MULTIPART/MIXED; BOUNDARY="683461219-331636985-1276113319=:7651"

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--683461219-331636985-1276113319=:7651
Content-Type: TEXT/PLAIN; charset=utf-8; format=flowed
Content-Transfer-Encoding: QUOTED-PRINTABLE

On Wed, 9 Jun 2010, Ruediger Pluem wrote:
> On 06/06/2010 07:01 PM, sf@apache.org wrote:
>> Author: sf
>> Date: Sun Jun  6 17:01:29 2010
>> New Revision: 951896
>>
>> URL: http://svn.apache.org/viewvc?rev=3D951896&view=3Drev
>> Log:
>> Use new loglevel accessor macros to simplify code
>>
>> Modified:
>>     httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
>>     httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c
>>
>> Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_io.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engi=
ne_io.c?rev=3D951896&r1=3D951895&r2=3D951896&view=3Ddiff
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
>> --- httpd/httpd/trunk/modules/ssl/ssl_engine_io.c (original)
>> +++ httpd/httpd/trunk/modules/ssl/ssl_engine_io.c Sun Jun  6 17:01:29 20=
10
>> @@ -1015,7 +1015,7 @@ static void ssl_filter_io_shutdown(ssl_f
>>      SSL_smart_shutdown(ssl);
>>
>>      /* and finally log the fact that we've closed the connection */
>> -    if (mySrvFromConn(c)->loglevel >=3D APLOG_INFO) {
>> +    if (APLOGcinfo(c)) {
>
> IMHO you need to use APLOGinfo(mySrvFromConn(c)) as otherwise you might c=
hoose the base_server
> which might not be what mySrvFromConn(c) returns.

I think it is even more complicated than that and it affects many places=20
in mod_ssl. If c has a loglevel configuration, we should probably use=20
that. If not, we should use mySrvFromConn(c). But in both cases, the log=20
message should be tied to the connection, to ensure that things like the=20
client IP are logged. Maybe we need a ap_log_scerror that accepts both a=20
server_rec and conn_rec.

AIUI, c->base_server is the default virtual host for the relevant IP/port.=
=20
Without SNI, mySrvFromConn(c) is the same but with SNI, mySrvFromConn(c)=20
may be a different name-based virtual host with the same IP/port. So,=20
mySrvFromConn(c) is basically r->server? Is that correct?


>
>>          ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, c,
>>                        "Connection closed to child %ld with %s shutdown =
"
>>                        "(server %s)",
>> @@ -1740,8 +1740,7 @@ void ssl_io_filter_init(conn_rec *c, req
>>      apr_pool_cleanup_register(c->pool, (void*)filter_ctx,
>>                                ssl_io_filter_cleanup, apr_pool_cleanup_n=
ull);
>>
>> -    if ((s->loglevel >=3D APLOG_DEBUG)
>> -         && (sc->ssl_log_level >=3D SSL_LOG_IO)) {
>> +    if (APLOGcdebug(c) && (sc->ssl_log_level >=3D SSL_LOG_IO)) {
>
> Same as above.
>
>>          BIO_set_callback(SSL_get_rbio(ssl), ssl_io_data_cb);
>>          BIO_set_callback_arg(SSL_get_rbio(ssl), (void *)ssl);
>>      }
>>
>
> Regards
>
> R=C3=BCdiger
>
--683461219-331636985-1276113319=:7651--