[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: svn commit: r1844001 - in /httpd/httpd/trunk/modules/ssl: mod_ssl.c ssl_engine_kernel.c ssl_private.h

On 17/10/2018 12:44, Joe Orton wrote:
> On Tue, Oct 16, 2018 at 07:21:54PM +0200, Ruediger Pluem wrote:
>> On 10/16/2018 02:53 PM, jfclere@xxxxxxxxxx wrote:
>>> Author: jfclere
>>> Date: Tue Oct 16 12:53:18 2018
>>> New Revision: 1844001
>>> URL: http://svn.apache.org/viewvc?rev=1844001&view=rev
>>> Log:
>>> And a way to custom modules to guess and extract ssl variable.
>>> See https://github.com/jfclere/JBCSP-17 for example...
> ...
>> 1. Instead of using literals you should use defines for them in mod_ssl.h. I understand that you want to use this
>>    in other modules. So it needs to become part of the mod_ssl API.
>> 2. Also the provider seems to provide only one method it should be stored in a clearly defined struct like e.g.
>>    authz_provider in mod_auth.h
>> 3. Why using the provider interface at all? You have only one method and it doesn't look like that other providers
>>    should be implemented. Looks like a perfect case for an optional function to me which we have various in mod_ssl,
>>    e.g. ssl_proxy_enable
> Doesn't make sense to expose that structure at all IMO, it's an 
> implementation detail.  Might make sense to have a new optional function 
> which is passed an apr_table_t * and populate it exactly as +StdEnvars 
> does, i.e. factor that code out:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_kernel.c?revision=1844001&view=markup#l1549
> actually make it equivalent to ssl_hook_Fixup() as a whole maybe.

One of the customer complains is that having the variables exposed like
in StdEnvars has a huge performances cost (everything is exposed for
each request) , he wants to check one variable and then decide in his
code what are the other he needs to access and yes he wants to know what
we can expose...

I see that just exposing, like StdEnvars the ones he may need already
helps, but that isn't what he wants.