subject:"\[PATCH\] split ap_get_server_version\(\) into two functions..."

Re: [PATCH] split ap_get_server_version() into two functions...

2006-09-01 Thread Joe Orton

On Wed, Aug 30, 2006 at 09:18:02AM -0400, Jeff Trawick wrote:
 ... so that ServerTokens doesn't affect what gets logged to the error
 log at startup (or any other place where we want the description of
 the server instead of the banner to be written over the network).
 
 This patch axes ap_get_server_version() so that third-party modules
 will be forced to make a choice in post-2.2.x versions -- call
 ap_get_server_banner() to retrieve the string suitable for sending
 over the network (as controlled by ServerTokens) or call
 ap_get_server_description() to retrieve the string that describes the
 server version and certain plug-in modules.

Looks good to me.

 I haven't actually changed the returned strings with this patch, but
 instead would like to see any comments on the direction.  The two
 functions are present and all httpd code has been modified to call the
 proper function, which is also subject to a useful review.  What about
 status pages?  Is the server version and module configuration somehow
 more private than the status page itself, such that they should use
 ap_get_server_banner() as in the current patch?

I'd agree with Sebastian here, the mod_status output should just use the 
more descriptive version, if you are revealing the server state in such 
detail you certainly have nothing to hide.

Regards,

joe

Re: [PATCH] split ap_get_server_version() into two functions...

2006-09-01 Thread Jeff Trawick


On 9/1/06, Joe Orton [EMAIL PROTECTED] wrote:

On Wed, Aug 30, 2006 at 09:18:02AM -0400, Jeff Trawick wrote:
 ... so that ServerTokens doesn't affect what gets logged to the error
 log at startup (or any other place where we want the description of
 the server instead of the banner to be written over the network).

 This patch axes ap_get_server_version() so that third-party modules
 will be forced to make a choice in post-2.2.x versions -- call
 ap_get_server_banner() to retrieve the string suitable for sending
 over the network (as controlled by ServerTokens) or call
 ap_get_server_description() to retrieve the string that describes the
 server version and certain plug-in modules.

Looks good to me.

 I haven't actually changed the returned strings with this patch, but
 instead would like to see any comments on the direction.  The two
 functions are present and all httpd code has been modified to call the
 proper function, which is also subject to a useful review.  What about
 status pages?  Is the server version and module configuration somehow
 more private than the status page itself, such that they should use
 ap_get_server_banner() as in the current patch?

I'd agree with Sebastian here, the mod_status output should just use the
more descriptive version, if you are revealing the server state in such
detail you certainly have nothing to hide.


Thanks to both for commenting and to Sebastian for carrying the patch
further.  I'll play with Sebastian's logic to return the proper
strings with the plan to commit (but still no ServerTokens None at
that point).

Re: [PATCH] split ap_get_server_version() into two functions...

2006-08-31 Thread Sebastian Nohn

Jeff Trawick wrote:

 ... so that ServerTokens doesn't affect what gets logged to the error
 log at startup (or any other place where we want the description of
 the server instead of the banner to be written over the network).

As far as I can see, your patch does not distinguish between those yet:

+AP_DECLARE(const char *) ap_get_server_description(void)
+{
+return get_server_version();
+}
+
+AP_DECLARE(const char *) ap_get_server_banner(void)
+{
+return get_server_version();
+}

 I haven't actually changed the returned strings with this patch, but
 instead would like to see any comments on the direction.  The two
 functions are present and all httpd code has been modified to call the

I should have read that ;)

 proper function, which is also subject to a useful review.  What about
 status pages?  Is the server version and module configuration somehow
 more private than the status page itself, such that they should use
 ap_get_server_banner() as in the current patch?

I don't think so. Someone who makes his status page public most likely
would also make his Server version and configured modules public. And
someone who protects his server status somehow does most likely not care
if Server version and module information is published on that page, so
in my opinion, the full information should be displayed on status pages.

Best regards,
  Sebastian Nohn
-- 
Sebastian Nohn · Wolfstraße 29 · 53111 Bonn · Germany
+49-228-4097103 · http://nohn.net/ · [EMAIL PROTECTED]
http://pgpkeys.pca.dfn.de:11371/pks/lookup?op=getfingerprint=onsearch=0xD47D55E0

Re: [PATCH] split ap_get_server_version() into two functions...

2006-08-31 Thread Sebastian Nohn

Sebastian Nohn wrote:
 Jeff Trawick wrote:
 
 ... so that ServerTokens doesn't affect what gets logged to the error
 log at startup (or any other place where we want the description of
 the server instead of the banner to be written over the network).
 
 As far as I can see, your patch does not distinguish between those yet:
 
 +AP_DECLARE(const char *) ap_get_server_description(void)
 +{
 +return get_server_version();
 +}
 +
 +AP_DECLARE(const char *) ap_get_server_banner(void)
 +{
 +return get_server_version();
 +}

Please find attached a patch that implements the desired behaviour in a
very rudimentary, but probably suffiecient manner (and includes my
ServerTokens Off patch, which could be removed easily but is included
for convenience).

Best regards,
  Sebastian Nohn
-- 
Sebastian Nohn · Wolfstraße 29 · 53111 Bonn · Germany
+49-228-4097103 · http://nohn.net/ · [EMAIL PROTECTED]
http://pgpkeys.pca.dfn.de:11371/pks/lookup?op=getfingerprint=onsearch=0xD47D55E0
Index: server/mpm/winnt/service.c
===
--- server/mpm/winnt/service.c  (revision 438824)
+++ server/mpm/winnt/service.c  (working copy)
@@ -436,7 +436,7 @@
 
 /* Time to fix up the description, upon each successful restart
  */
-full_description = ap_get_server_version();
+full_description = ap_get_server_description();
 
 if ((osver.dwPlatformId == VER_PLATFORM_WIN32_NT)
(osver.dwMajorVersion  4)
Index: server/mpm/winnt/mpm_winnt.c
===
--- server/mpm/winnt/mpm_winnt.c(revision 438824)
+++ server/mpm/winnt/mpm_winnt.c(working copy)
@@ -1705,7 +1705,7 @@
 /* A real-honest to goodness parent */
 ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, ap_server_conf,
  %s configured -- resuming normal operations,
- ap_get_server_version());
+ ap_get_server_description());
 ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, ap_server_conf,
  Server built: %s, ap_get_server_built());
 
Index: server/mpm/mpmt_os2/mpmt_os2.c
===
--- server/mpm/mpmt_os2/mpmt_os2.c  (revision 438824)
+++ server/mpm/mpmt_os2/mpmt_os2.c  (working copy)
@@ -207,7 +207,7 @@
 int listener_num, num_listeners, slot;
 ULONG rc;
 
-printf(%s \n, ap_get_server_version());
+printf(%s \n, ap_get_server_description());
 set_signals();
 
 if (ap_setup_listeners(ap_server_conf)  1) {
@@ -270,7 +270,7 @@
 ap_scoreboard_image-global-restart_time = apr_time_now();
 ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, ap_server_conf,
 %s configured -- resuming normal operations,
-ap_get_server_version());
+ap_get_server_description());
 ap_log_error(APLOG_MARK, APLOG_INFO, 0, ap_server_conf,
 Server built: %s, ap_get_server_built());
 #ifdef AP_MPM_WANT_SET_ACCEPT_LOCK_MECH
Index: server/mpm/netware/mpm_netware.c
===
--- server/mpm/netware/mpm_netware.c(revision 438824)
+++ server/mpm/netware/mpm_netware.c(working copy)
@@ -723,7 +723,7 @@
 request_count = 0;
 
 ClearScreen (getscreenhandle());
-printf(%s \n, ap_get_server_version());
+printf(%s \n, ap_get_server_description());
 
 for (i=0;iSERVER_NUM_STATUS;i++) {
 status_array[i] = 0;
@@ -793,7 +793,7 @@
 ap_listen_rec *lr;
 module **m;
 
-printf(%s\n, ap_get_server_version());
+printf(%s\n, ap_get_server_description());
 if (ap_my_addrspace  (ap_my_addrspace[0] != 'O')  (ap_my_addrspace[1] 
!= 'S'))
 printf(   Running in address space %s\n, ap_my_addrspace);
 
@@ -899,7 +899,7 @@
 
 ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, ap_server_conf,
 %s configured -- resuming normal operations,
-ap_get_server_version());
+ap_get_server_description());
 ap_log_error(APLOG_MARK, APLOG_INFO, 0, ap_server_conf,
 Server built: %s, ap_get_server_built());
 #ifdef AP_MPM_WANT_SET_ACCEPT_LOCK_MECH
@@ -1204,7 +1204,7 @@
 restart();
 }
 else if (!strnicmp(VERSION,szcommandLine[iCommandLen],3)) {
-printf(Server version: %s\n, ap_get_server_version());
+printf(Server version: %s\n, ap_get_server_description());
 printf(Server built:   %s\n, ap_get_server_built());
 }
 else if (!strnicmp(MODULES,szcommandLine[iCommandLen],3)) {
Index: server/mpm/beos/beos.c
===
--- server/mpm/beos/beos.c  (revision 438824)
+++ server/mpm/beos/beos.c  (working copy)
@@ -935,7 +935,7 @@
  */
 ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, ap_server_conf,
 %s configured -- resuming normal operations,
-

[PATCH] split ap_get_server_version() into two functions...

2006-08-30 Thread Jeff Trawick


... so that ServerTokens doesn't affect what gets logged to the error
log at startup (or any other place where we want the description of
the server instead of the banner to be written over the network).

This patch axes ap_get_server_version() so that third-party modules
will be forced to make a choice in post-2.2.x versions -- call
ap_get_server_banner() to retrieve the string suitable for sending
over the network (as controlled by ServerTokens) or call
ap_get_server_description() to retrieve the string that describes the
server version and certain plug-in modules.

I haven't actually changed the returned strings with this patch, but
instead would like to see any comments on the direction.  The two
functions are present and all httpd code has been modified to call the
proper function, which is also subject to a useful review.  What about
status pages?  Is the server version and module configuration somehow
more private than the status page itself, such that they should use
ap_get_server_banner() as in the current patch?
Index: server/mpm/winnt/service.c
===
--- server/mpm/winnt/service.c  (revision 438471)
+++ server/mpm/winnt/service.c  (working copy)
@@ -436,7 +436,7 @@
 
 /* Time to fix up the description, upon each successful restart
  */
-full_description = ap_get_server_version();
+full_description = ap_get_server_description();
 
 if ((osver.dwPlatformId == VER_PLATFORM_WIN32_NT)
(osver.dwMajorVersion  4)
Index: server/mpm/winnt/mpm_winnt.c
===
--- server/mpm/winnt/mpm_winnt.c(revision 438471)
+++ server/mpm/winnt/mpm_winnt.c(working copy)
@@ -1705,7 +1705,7 @@
 /* A real-honest to goodness parent */
 ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, ap_server_conf,
  %s configured -- resuming normal operations,
- ap_get_server_version());
+ ap_get_server_description());
 ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, ap_server_conf,
  Server built: %s, ap_get_server_built());
 
Index: server/mpm/mpmt_os2/mpmt_os2.c
===
--- server/mpm/mpmt_os2/mpmt_os2.c  (revision 438471)
+++ server/mpm/mpmt_os2/mpmt_os2.c  (working copy)
@@ -207,7 +207,7 @@
 int listener_num, num_listeners, slot;
 ULONG rc;
 
-printf(%s \n, ap_get_server_version());
+printf(%s \n, ap_get_server_description());
 set_signals();
 
 if (ap_setup_listeners(ap_server_conf)  1) {
@@ -270,7 +270,7 @@
 ap_scoreboard_image-global-restart_time = apr_time_now();
 ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, ap_server_conf,
 %s configured -- resuming normal operations,
-ap_get_server_version());
+ap_get_server_description());
 ap_log_error(APLOG_MARK, APLOG_INFO, 0, ap_server_conf,
 Server built: %s, ap_get_server_built());
 #ifdef AP_MPM_WANT_SET_ACCEPT_LOCK_MECH
Index: server/mpm/netware/mpm_netware.c
===
--- server/mpm/netware/mpm_netware.c(revision 438471)
+++ server/mpm/netware/mpm_netware.c(working copy)
@@ -723,7 +723,7 @@
 request_count = 0;
 
 ClearScreen (getscreenhandle());
-printf(%s \n, ap_get_server_version());
+printf(%s \n, ap_get_server_description());
 
 for (i=0;iSERVER_NUM_STATUS;i++) {
 status_array[i] = 0;
@@ -793,7 +793,7 @@
 ap_listen_rec *lr;
 module **m;
 
-printf(%s\n, ap_get_server_version());
+printf(%s\n, ap_get_server_description());
 if (ap_my_addrspace  (ap_my_addrspace[0] != 'O')  (ap_my_addrspace[1] 
!= 'S'))
 printf(   Running in address space %s\n, ap_my_addrspace);
 
@@ -899,7 +899,7 @@
 
 ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, ap_server_conf,
 %s configured -- resuming normal operations,
-ap_get_server_version());
+ap_get_server_description());
 ap_log_error(APLOG_MARK, APLOG_INFO, 0, ap_server_conf,
 Server built: %s, ap_get_server_built());
 #ifdef AP_MPM_WANT_SET_ACCEPT_LOCK_MECH
@@ -1204,7 +1204,7 @@
 restart();
 }
 else if (!strnicmp(VERSION,szcommandLine[iCommandLen],3)) {
-printf(Server version: %s\n, ap_get_server_version());
+printf(Server version: %s\n, ap_get_server_description());
 printf(Server built:   %s\n, ap_get_server_built());
 }
 else if (!strnicmp(MODULES,szcommandLine[iCommandLen],3)) {
Index: server/mpm/beos/beos.c
===
--- server/mpm/beos/beos.c  (revision 438471)
+++ server/mpm/beos/beos.c  (working copy)
@@ -935,7 +935,7 @@
  */
 ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, ap_server_conf,
 %s

Re: [PATCH] split ap_get_server_version() into two functions...

Re: [PATCH] split ap_get_server_version() into two functions...

Re: [PATCH] split ap_get_server_version() into two functions...

Re: [PATCH] split ap_get_server_version() into two functions...

[PATCH] split ap_get_server_version() into two functions...

5 matches

Site Navigation

Mail list logo

Footer information