Re: svn commit: r1685052 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

2015-06-12 Thread Christophe JAILLET 

Hi,

should this warning at startup be an issue, why not just remove it in 
2.4.x and keep it in trunk?
Having the depreciation written in doc (and in migration note for 2.4 - 
2.6/3.0 one day) could be enough, no?



BTW, the apr_pstrdup(ap_pglobal, 1) could be just 1.

CJ

Le 12/06/2015 11:07, yla...@apache.org a écrit :

Author: ylavic
Date: Fri Jun 12 09:07:34 2015
New Revision: 1685052

URL: http://svn.apache.org/r1685052
Log:
mod_ssl: Warn about deprecated SSLCertificateChainFile once at startup,
on first usage only.

Modified:
 httpd/httpd/trunk/CHANGES
 httpd/httpd/trunk/modules/ssl/ssl_engine_config.c

Modified: httpd/httpd/trunk/CHANGES
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1685052r1=1685051r2=1685052view=diff
==
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Jun 12 09:07:34 2015
@@ -1,6 +1,9 @@
   -*- coding: utf-8 -*-
  Changes with Apache 2.5.0
  
+  *) mod_ssl: Warn about deprecated SSLCertificateChainFile once at startup,

+ on first usage only.  [Yann Ylavic]
+
*) mod_substitute: Fix configuraton merge order.
   PR 57641 [Marc.Stern approach.be]
  


Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_config.c?rev=1685052r1=1685051r2=1685052view=diff
==
--- httpd/httpd/trunk/modules/ssl/ssl_engine_config.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_engine_config.c Fri Jun 12 09:07:34 2015
@@ -839,13 +839,22 @@ const char *ssl_cmd_SSLCertificateChainF
  const char *arg)
  {
  SSLSrvConfigRec *sc = mySrvConfig(cmd-server);
+void *once = NULL;
  const char *err;
  
-ap_log_error(APLOG_MARK, APLOG_WARNING|APLOG_STARTUP, 0, NULL,

- APLOGNO(02559)
- The SSLCertificateChainFile directive (%s:%d) is deprecated, 

- SSLCertificateFile should be used instead,
- cmd-directive-filename, cmd-directive-line_num);
+apr_pool_userdata_get(once, ssl_cmd_SSLCertificateChainFile,
+  ap_pglobal);
+if (!once) {
+ap_log_error(APLOG_MARK, APLOG_WARNING|APLOG_STARTUP, 0, NULL,
+ APLOGNO(02559)
+ The SSLCertificateChainFile directive (%s:%d) is 
+ deprecated, SSLCertificateFile should be used instead,
+ cmd-directive-filename, cmd-directive-line_num);
+
+apr_pool_userdata_set(ssl_cmd_SSLCertificateChainFile,
+  apr_pstrdup(ap_pglobal, 1), NULL,
+  ap_pglobal);
+}
  
  if ((err = ssl_cmd_check_file(cmd, arg))) {

  return err;

Re: svn commit: r1685052 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

2015-06-12 Thread Yann Ylavic 
Hi Christophe,

On Fri, Jun 12, 2015 at 1:26 PM, Christophe JAILLET
christophe.jail...@wanadoo.fr wrote:

 should this warning at startup be an issue, why not just remove it in 2.4.x
 and keep it in trunk?
 Having the depreciation written in doc (and in migration note for 2.4 -
 2.6/3.0 one day) could be enough, no?

Probably, but the (only one) warning per startup can possibly
encourage users to look at the doc :)
Let's see what others say.



 BTW, the apr_pstrdup(ap_pglobal, 1) could be just 1.

I'm not sure, if mod_ssl is loaded dynamically, 1 could possibly be
un/reloaded with it (.text section)...

Regards,
Yann.

Re: svn commit: r1685052 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

2015-06-12 Thread Jeff Trawick 
On Fri, Jun 12, 2015 at 5:07 AM, yla...@apache.org wrote:

 Author: ylavic
 Date: Fri Jun 12 09:07:34 2015
 New Revision: 1685052

 URL: http://svn.apache.org/r1685052
 Log:
 mod_ssl: Warn about deprecated SSLCertificateChainFile once at startup,
 on first usage only.

 Modified:
 httpd/httpd/trunk/CHANGES
 httpd/httpd/trunk/modules/ssl/ssl_engine_config.c

 Modified: httpd/httpd/trunk/CHANGES
 URL:
 http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1685052r1=1685051r2=1685052view=diff

 ==
 --- httpd/httpd/trunk/CHANGES [utf-8] (original)
 +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Jun 12 09:07:34 2015
 @@ -1,6 +1,9 @@
   -*- coding:
 utf-8 -*-
  Changes with Apache 2.5.0

 +  *) mod_ssl: Warn about deprecated SSLCertificateChainFile once at
 startup,
 + on first usage only.  [Yann Ylavic]
 +
*) mod_substitute: Fix configuraton merge order.
   PR 57641 [Marc.Stern approach.be]


 Modified: httpd/httpd/trunk/modules/ssl/ssl_engine_config.c
 URL:
 http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_engine_config.c?rev=1685052r1=1685051r2=1685052view=diff

 ==
 --- httpd/httpd/trunk/modules/ssl/ssl_engine_config.c (original)
 +++ httpd/httpd/trunk/modules/ssl/ssl_engine_config.c Fri Jun 12 09:07:34
 2015
 @@ -839,13 +839,22 @@ const char *ssl_cmd_SSLCertificateChainF
  const char *arg)
  {
  SSLSrvConfigRec *sc = mySrvConfig(cmd-server);
 +void *once = NULL;
  const char *err;

 -ap_log_error(APLOG_MARK, APLOG_WARNING|APLOG_STARTUP, 0, NULL,
 - APLOGNO(02559)
 - The SSLCertificateChainFile directive (%s:%d) is
 deprecated, 
 - SSLCertificateFile should be used instead,
 - cmd-directive-filename, cmd-directive-line_num);
 +apr_pool_userdata_get(once, ssl_cmd_SSLCertificateChainFile,
 +  ap_pglobal);
 +if (!once) {
 +ap_log_error(APLOG_MARK, APLOG_WARNING|APLOG_STARTUP, 0, NULL,
 + APLOGNO(02559)
 + The SSLCertificateChainFile directive (%s:%d) is 
 + deprecated, SSLCertificateFile should be used
 instead,
 + cmd-directive-filename, cmd-directive-line_num);
 +
 +apr_pool_userdata_set(ssl_cmd_SSLCertificateChainFile,
 +  apr_pstrdup(ap_pglobal, 1), NULL,
 +  ap_pglobal);
 +}

 IMHO the  ap_retained_data_get/create APIs make this nicer than this
older pattern.

retained = ap_retained_data_get(userdata_key);
if (!retained) {
retained = ap_retained_data_create(userdata_key, sizeof(*retained))



 if ((err = ssl_cmd_check_file(cmd, arg))) {
  return err;





-- 
Born in Roswell... married an alien...
http://emptyhammock.com/

Re: svn commit: r1685052 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

2015-06-12 Thread Yann Ylavic 
On Fri, Jun 12, 2015 at 2:11 PM, Jeff Trawick traw...@gmail.com wrote:

 IMHO the  ap_retained_data_get/create APIs make this nicer than this older
 pattern.

Ah yes, good catch, I just did not think about it.


 retained = ap_retained_data_get(userdata_key);
 if (!retained) {
 retained = ap_retained_data_create(userdata_key, sizeof(*retained))

Will change to use that instead in a follow up.
Unless the warning should be removed completely...

Re: svn commit: r1685052 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

2015-06-12 Thread Rainer Jung 

Am 12.06.2015 um 13:49 schrieb Yann Ylavic:

Hi Christophe,

On Fri, Jun 12, 2015 at 1:26 PM, Christophe JAILLET
christophe.jail...@wanadoo.fr wrote:


should this warning at startup be an issue, why not just remove it in 2.4.x
and keep it in trunk?
Having the depreciation written in doc (and in migration note for 2.4 -
2.6/3.0 one day) could be enough, no?


Probably, but the (only one) warning per startup can possibly
encourage users to look at the doc :)
Let's see what others say.


I don't have a strong opinion, but if we want to remove the directive in 
the next version, then it would help to keep the deprecation logging to 
make people aware and reduce the migration complains a bit.


Logging only once instead of for each certificate is IMHO enough and 
much better.


As to if the current situation in 2.4.14 is a problem or should be 
tolerated (lots of log output for people with huge numbers of 
certificates) I'm quite undecided.


Regards,

Rainer

Re: svn commit: r1685052 - in /httpd/httpd/trunk: CHANGES modules/ssl/ssl_engine_config.c

2015-06-12 Thread William A Rowe Jr 
On Fri, Jun 12, 2015 at 8:36 AM, Rainer Jung rainer.j...@kippdata.de
wrote:

 Am 12.06.2015 um 13:49 schrieb Yann Ylavic:

 Hi Christophe,

 On Fri, Jun 12, 2015 at 1:26 PM, Christophe JAILLET
 christophe.jail...@wanadoo.fr wrote:


 should this warning at startup be an issue, why not just remove it in
 2.4.x
 and keep it in trunk?
 Having the depreciation written in doc (and in migration note for 2.4 -
 2.6/3.0 one day) could be enough, no?


 Probably, but the (only one) warning per startup can possibly
 encourage users to look at the doc :)
 Let's see what others say.


 I don't have a strong opinion, but if we want to remove the directive in
 the next version, then it would help to keep the deprecation logging to
 make people aware and reduce the migration complains a bit.

 Logging only once instead of for each certificate is IMHO enough and much
 better.


Agreed, although this is never a [warn], it's [info] at strongest.


 As to if the current situation in 2.4.14 is a problem or should be
 tolerated (lots of log output for people with huge numbers of certificates)
 I'm quite undecided.


Trivial patch, easily applied by the tiny few who would be affected, so I
don't think it has any impact on the 2.4.14 readiness for GA.

Bill

Patch: message received and acknowledged;

--- modules/ssl/ssl_engine_config.c (revision 8078)
+++ modules/ssl/ssl_engine_config.c (working copy)
@@ -862,7 +862,7 @@
 SSLSrvConfigRec *sc = mySrvConfig(cmd-server);
 const char *err;

-ap_log_error(APLOG_MARK, APLOG_WARNING|APLOG_STARTUP, 0, NULL,
+ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_STARTUP, 0, NULL,
  APLOGNO(02559)
  The SSLCertificateChainFile directive (%s:%d) is
deprecated, 
  SSLCertificateFile should be used instead,