Re: a patch for a ap_ssl_is_https()
Hi Stefan,
Günter is dead at 2020.
Please cancel all Email..
mfg.
Am 2021-02-23 15:18, schrieb Stefan Eissing:
Am 23.02.2021 um 12:18 schrieb Ruediger Pluem :
On 2/23/21 11:26 AM, Stefan Eissing wrote:
Am 23.02.2021 um 11:14 schrieb Joe Orton :
On Mon, Feb 22, 2021 at 05:28:03PM +0100, Stefan Eissing wrote:
Regarding my proposal to add SSL related inquiry functions to our
core server, here
is a patch for the "ssl_is_https()" function. This allows:
a) anyone to inquire about a connections SSLiness without the
optional function retrieval.
It will itself call such a function provided by a module. So this
should make anyone
using the new ap_ssl_is_ssl(c) remain compatible to existing ssl
modules.
This makes sense to me except, obviously, I will start a fight to
bikeshed the naming, since "SSL is SSL" scans quite weirdly?
ap_is_https() or ap_conn_is_{ssl,tls}() or something would be better
IMO?
Was ping-pong in this as well. But we need to extend this for other
'ssl' optional functions and I thought keeping a comming 'ap_ssl_'
prefix is overall better to parse. But I am not strong opinioned on
this.
Maybe ap_ssl_conn_is_{ssl,tls}() is a middle ground with respect to
the need for further ap_ssl_ functions?
Fine with me!
Re: a patch for a ap_ssl_is_https()
> Am 23.02.2021 um 12:18 schrieb Ruediger Pluem :
>
>
>
> On 2/23/21 11:26 AM, Stefan Eissing wrote:
>>
>>
>>> Am 23.02.2021 um 11:14 schrieb Joe Orton :
>>>
>>> On Mon, Feb 22, 2021 at 05:28:03PM +0100, Stefan Eissing wrote:
Regarding my proposal to add SSL related inquiry functions to our core
server, here
is a patch for the "ssl_is_https()" function. This allows:
a) anyone to inquire about a connections SSLiness without the optional
function retrieval.
It will itself call such a function provided by a module. So this should
make anyone
using the new ap_ssl_is_ssl(c) remain compatible to existing ssl modules.
>>>
>>> This makes sense to me except, obviously, I will start a fight to
>>> bikeshed the naming, since "SSL is SSL" scans quite weirdly?
>>> ap_is_https() or ap_conn_is_{ssl,tls}() or something would be better
>>> IMO?
>>
>> Was ping-pong in this as well. But we need to extend this for other 'ssl'
>> optional functions and I thought keeping a comming 'ap_ssl_' prefix is
>> overall better to parse. But I am not strong opinioned on this.
>
> Maybe ap_ssl_conn_is_{ssl,tls}() is a middle ground with respect to the need
> for further ap_ssl_ functions?
Fine with me!
Re: a patch for a ap_ssl_is_https()
On 2/23/21 11:26 AM, Stefan Eissing wrote:
>
>
>> Am 23.02.2021 um 11:14 schrieb Joe Orton :
>>
>> On Mon, Feb 22, 2021 at 05:28:03PM +0100, Stefan Eissing wrote:
>>> Regarding my proposal to add SSL related inquiry functions to our core
>>> server, here
>>> is a patch for the "ssl_is_https()" function. This allows:
>>>
>>> a) anyone to inquire about a connections SSLiness without the optional
>>> function retrieval.
>>> It will itself call such a function provided by a module. So this should
>>> make anyone
>>> using the new ap_ssl_is_ssl(c) remain compatible to existing ssl modules.
>>
>> This makes sense to me except, obviously, I will start a fight to
>> bikeshed the naming, since "SSL is SSL" scans quite weirdly?
>> ap_is_https() or ap_conn_is_{ssl,tls}() or something would be better
>> IMO?
>
> Was ping-pong in this as well. But we need to extend this for other 'ssl'
> optional functions and I thought keeping a comming 'ap_ssl_' prefix is
> overall better to parse. But I am not strong opinioned on this.
Maybe ap_ssl_conn_is_{ssl,tls}() is a middle ground with respect to the need
for further ap_ssl_ functions?
Regards
Rüdiger
Re: a patch for a ap_ssl_is_https()
On 2/23/21 11:14 AM, Joe Orton wrote:
> On Mon, Feb 22, 2021 at 05:28:03PM +0100, Stefan Eissing wrote:
>> Regarding my proposal to add SSL related inquiry functions to our core
>> server, here
>> is a patch for the "ssl_is_https()" function. This allows:
>>
>> a) anyone to inquire about a connections SSLiness without the optional
>> function retrieval.
>>It will itself call such a function provided by a module. So this should
>> make anyone
>>using the new ap_ssl_is_ssl(c) remain compatible to existing ssl modules.
>
> This makes sense to me except, obviously, I will start a fight to
> bikeshed the naming, since "SSL is SSL" scans quite weirdly?
> ap_is_https() or ap_conn_is_{ssl,tls}() or something would be better
> IMO?
>
+1 to any of both proposals
Regards
Rüdiger
Re: a patch for a ap_ssl_is_https()
On 2/22/21 6:00 PM, Eric Covener wrote: > On Mon, Feb 22, 2021 at 11:28 AM Stefan Eissing > wrote: >> >> Regarding my proposal to add SSL related inquiry functions to our core >> server, here >> is a patch for the "ssl_is_https()" function. This allows: >> >> a) anyone to inquire about a connections SSLiness without the optional >> function retrieval. >>It will itself call such a function provided by a module. So this should >> make anyone >>using the new ap_ssl_is_ssl(c) remain compatible to existing ssl modules. >> b) provide a hook to ssl modules where they can register to inform about >> connections they manage. >> c) allow old modules that use the existing optional functions to work when >> everyone uses the new hook. >> >> If I got this right, of course. Feedback very much appreciated. > > +1, maybe a comment somewhere that normal optional functions are setup > earlier in register_hooks() so early post-config will be the right > time to shadow them. That question came up to me either. Hence +1 on the patch but also +1 to mention that in a comment in post-config to understand this better. Regards Rüdiger
Re: a patch for a ap_ssl_is_https()
> Am 23.02.2021 um 11:14 schrieb Joe Orton :
>
> On Mon, Feb 22, 2021 at 05:28:03PM +0100, Stefan Eissing wrote:
>> Regarding my proposal to add SSL related inquiry functions to our core
>> server, here
>> is a patch for the "ssl_is_https()" function. This allows:
>>
>> a) anyone to inquire about a connections SSLiness without the optional
>> function retrieval.
>> It will itself call such a function provided by a module. So this should
>> make anyone
>> using the new ap_ssl_is_ssl(c) remain compatible to existing ssl modules.
>
> This makes sense to me except, obviously, I will start a fight to
> bikeshed the naming, since "SSL is SSL" scans quite weirdly?
> ap_is_https() or ap_conn_is_{ssl,tls}() or something would be better
> IMO?
Was ping-pong in this as well. But we need to extend this for other 'ssl'
optional functions and I thought keeping a comming 'ap_ssl_' prefix is overall
better to parse. But I am not strong opinioned on this.
>
>> b) provide a hook to ssl modules where they can register to inform about
>> connections they manage.
>> c) allow old modules that use the existing optional functions to work when
>> everyone uses the new hook.
>>
>> If I got this right, of course. Feedback very much appreciated.
>
> Looks like the right design otherwise to me. And all the modules which
> do the dance to retrieve ssl_is_https currently, can be changed over to
> this new API? A nice simplification.
\o/
>
> FWIW we briefly tried in RHEL supporting loading mod_ssl & mod_nss into
> httpd simultaneously, patching both to juggle the optional functions,
> and it was a bit painful/stupid. So, this is definitely much better.
> (We dropped mod_nss from RHEL8 onwards anyway)
>
> Regards, Joe
>
>
Re: a patch for a ap_ssl_is_https()
On Mon, Feb 22, 2021 at 05:28:03PM +0100, Stefan Eissing wrote:
> Regarding my proposal to add SSL related inquiry functions to our core
> server, here
> is a patch for the "ssl_is_https()" function. This allows:
>
> a) anyone to inquire about a connections SSLiness without the optional
> function retrieval.
>It will itself call such a function provided by a module. So this should
> make anyone
>using the new ap_ssl_is_ssl(c) remain compatible to existing ssl modules.
This makes sense to me except, obviously, I will start a fight to
bikeshed the naming, since "SSL is SSL" scans quite weirdly?
ap_is_https() or ap_conn_is_{ssl,tls}() or something would be better
IMO?
> b) provide a hook to ssl modules where they can register to inform about
> connections they manage.
> c) allow old modules that use the existing optional functions to work when
> everyone uses the new hook.
>
> If I got this right, of course. Feedback very much appreciated.
Looks like the right design otherwise to me. And all the modules which
do the dance to retrieve ssl_is_https currently, can be changed over to
this new API? A nice simplification.
FWIW we briefly tried in RHEL supporting loading mod_ssl & mod_nss into
httpd simultaneously, patching both to juggle the optional functions,
and it was a bit painful/stupid. So, this is definitely much better.
(We dropped mod_nss from RHEL8 onwards anyway)
Regards, Joe
Re: a patch for a ap_ssl_is_https()
On Mon, Feb 22, 2021 at 11:28 AM Stefan Eissing wrote: > > Regarding my proposal to add SSL related inquiry functions to our core > server, here > is a patch for the "ssl_is_https()" function. This allows: > > a) anyone to inquire about a connections SSLiness without the optional > function retrieval. >It will itself call such a function provided by a module. So this should > make anyone >using the new ap_ssl_is_ssl(c) remain compatible to existing ssl modules. > b) provide a hook to ssl modules where they can register to inform about > connections they manage. > c) allow old modules that use the existing optional functions to work when > everyone uses the new hook. > > If I got this right, of course. Feedback very much appreciated. +1, maybe a comment somewhere that normal optional functions are setup earlier in register_hooks() so early post-config will be the right time to shadow them.
a patch for a ap_ssl_is_https()
Regarding my proposal to add SSL related inquiry functions to our core server, here is a patch for the "ssl_is_https()" function. This allows: a) anyone to inquire about a connections SSLiness without the optional function retrieval. It will itself call such a function provided by a module. So this should make anyone using the new ap_ssl_is_ssl(c) remain compatible to existing ssl modules. b) provide a hook to ssl modules where they can register to inform about connections they manage. c) allow old modules that use the existing optional functions to work when everyone uses the new hook. If I got this right, of course. Feedback very much appreciated. Cheers, Stefan is_ssl.patch Description: Binary data
