Re: svn commit: r1100200 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS modules/http/http_filters.c server/protocol.c

2011-05-07 Thread William A. Rowe Jr.
On 5/7/2011 12:20 AM, Ruediger Pluem wrote:
 
 
 On 05/06/2011 03:14 PM, cove...@apache.org wrote:
 Author: covener
 Date: Fri May  6 13:14:27 2011
 New Revision: 1100200

 URL: http://svn.apache.org/viewvc?rev=1100200view=rev
 Log:
 Merge r820760, r919323, r937858, r938265 from trunk:

 Reviewed By: sf, trawick, covener

 Modified: httpd/httpd/branches/2.2.x/server/protocol.c
 URL: 
 http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/server/protocol.c?rev=1100200r1=1100199r2=1100200view=diff
 ==
 --- httpd/httpd/branches/2.2.x/server/protocol.c (original)
 +++ httpd/httpd/branches/2.2.x/server/protocol.c Fri May  6 13:14:27 2011
 @@ -608,6 +608,9 @@ static int read_request_line(request_rec
  r-proto_num = HTTP_VERSION(1,0);
  r-protocol  = apr_pstrdup(r-pool, HTTP/1.0);
  }
 +else if (rv == APR_TIMEUP) {

 @@ -691,7 +694,12 @@ AP_DECLARE(void) ap_get_mime_headers_cor
   len, r, 0, bb);
  
  if (rv != APR_SUCCESS) {
 -r-status = HTTP_BAD_REQUEST;
 +if (rv == APR_TIMEUP) {
 
 As mentioned previously APR_STATUS_IS_TIMEUP should be used instead.
 Didn't we have a security issue on Windows and Netware because of this?

Absolutely; +1 to expedite this patch; with a third +1 I'll commit.

Bill


Re: svn commit: r1100200 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS modules/http/http_filters.c server/protocol.c

2011-05-07 Thread Rainer Jung

On 07.05.2011 11:57, William A. Rowe Jr. wrote:

On 5/7/2011 12:20 AM, Ruediger Pluem wrote:



On 05/06/2011 03:14 PM, cove...@apache.org wrote:

Author: covener
Date: Fri May  6 13:14:27 2011
New Revision: 1100200

URL: http://svn.apache.org/viewvc?rev=1100200view=rev
Log:
Merge r820760, r919323, r937858, r938265 from trunk:

Reviewed By: sf, trawick, covener



Modified: httpd/httpd/branches/2.2.x/server/protocol.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/server/protocol.c?rev=1100200r1=1100199r2=1100200view=diff
==
--- httpd/httpd/branches/2.2.x/server/protocol.c (original)
+++ httpd/httpd/branches/2.2.x/server/protocol.c Fri May  6 13:14:27 2011
@@ -608,6 +608,9 @@ static int read_request_line(request_rec
  r-proto_num = HTTP_VERSION(1,0);
  r-protocol  = apr_pstrdup(r-pool, HTTP/1.0);
  }
+else if (rv == APR_TIMEUP) {



@@ -691,7 +694,12 @@ AP_DECLARE(void) ap_get_mime_headers_cor
   len, r, 0, bb);

  if (rv != APR_SUCCESS) {
-r-status = HTTP_BAD_REQUEST;
+if (rv == APR_TIMEUP) {


As mentioned previously APR_STATUS_IS_TIMEUP should be used instead.
Didn't we have a security issue on Windows and Netware because of this?


Absolutely; +1 to expedite this patch; with a third +1 I'll commit.

Bill


+1 to change from comparison with APR_TIMEUP to APR_STATUS_IS_TIMEUP in 
both places in protocol.c. Note this applies to trunk and 2.2.


Two more recent APR_TIMEUP additions are in trunk, Ruediger commented on 
them Re r1092076 on APril 23rd. I'd say they should be fixed as well.


Regards,

Rainer


Re: svn commit: r1100200 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS modules/http/http_filters.c server/protocol.c

2011-05-07 Thread Eric Covener
On Sat, May 7, 2011 at 7:08 AM, Rainer Jung rainer.j...@kippdata.de wrote:
 On 07.05.2011 11:57, William A. Rowe Jr. wrote:

 On 5/7/2011 12:20 AM, Ruediger Pluem wrote:


 On 05/06/2011 03:14 PM, cove...@apache.org wrote:

 Author: covener
 Date: Fri May  6 13:14:27 2011
 New Revision: 1100200

 URL: http://svn.apache.org/viewvc?rev=1100200view=rev
 Log:
 Merge r820760, r919323, r937858, r938265 from trunk:

 Reviewed By: sf, trawick, covener

 Modified: httpd/httpd/branches/2.2.x/server/protocol.c
 URL:
 http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/server/protocol.c?rev=1100200r1=1100199r2=1100200view=diff

 ==
 --- httpd/httpd/branches/2.2.x/server/protocol.c (original)
 +++ httpd/httpd/branches/2.2.x/server/protocol.c Fri May  6 13:14:27
 2011
 @@ -608,6 +608,9 @@ static int read_request_line(request_rec
                  r-proto_num = HTTP_VERSION(1,0);
                  r-protocol  = apr_pstrdup(r-pool, HTTP/1.0);
              }
 +            else if (rv == APR_TIMEUP) {

 @@ -691,7 +694,12 @@ AP_DECLARE(void) ap_get_mime_headers_cor
                           len, r, 0, bb);

          if (rv != APR_SUCCESS) {
 -            r-status = HTTP_BAD_REQUEST;
 +            if (rv == APR_TIMEUP) {

 As mentioned previously APR_STATUS_IS_TIMEUP should be used instead.
 Didn't we have a security issue on Windows and Netware because of this?

 Absolutely; +1 to expedite this patch; with a third +1 I'll commit.

 Bill

 +1 to change from comparison with APR_TIMEUP to APR_STATUS_IS_TIMEUP in both
 places in protocol.c. Note this applies to trunk and 2.2.

 Two more recent APR_TIMEUP additions are in trunk, Ruediger commented on
 them Re r1092076 on APril 23rd. I'd say they should be fixed as well.

 Regards,

 Rainer


third +1 and will backport and link to this thread.

-- 
Eric Covener
cove...@gmail.com


Re: svn commit: r1098105 - /httpd/httpd/branches/2.2.x/STATUS

2011-05-07 Thread Graham Leggett

On 30 Apr 2011, at 2:22 PM, traw...@apache.org wrote:


  * mod_cache: Realign the cache_quick_handler() to behave identically
to the default_handler() when reacting to errors when writing to  
the

@@ -132,6 +132,8 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
Trunk patches: http://svn.apache.org/viewvc?view=revisionrevision=1003913
2.2.x patch: 
http://people.apache.org/~minfrin/httpd-mod_cache-errorfix-22.patch
+1: minfrin
+trawick: any reason it shouldn't be completely aligned with  
default_handler's

+ choice to return OK vs. 500?


Hmmm...

In the cache, we care about AP_FILTER_ERROR, and pass that back if  
present. In the default handler, we ignore AP_FILTER_ERROR completely,  
and instead check for this following:


if (status == APR_SUCCESS
|| r-status != HTTP_OK
|| c-aborted) {
return OK;
}

I'm not sure whether merging these is safe enough to backport?

The problem the original patch solves as that an APR error is  
appearing in the access_log, and that has caused much confusion for  
some people: http://www.google.co.uk/search?q=http+error+103


Ideally, this should be fixed at the very least, and we can then look  
at aligning the behaviour to be closer matched. Does this make sense,  
or am I being excessively paranoid?


Regards,
Graham
--





Re: is anyone planning to TR httpd 2.2.18 real soon?

2011-05-07 Thread Jim Jagielski
I am, once the APR releases happen...

On May 6, 2011, at 8:47 AM, Jeff Trawick wrote:

 lots of fixes ready in httpd; also expecting apr 1.4.3/apr-util 1.3.11
 to be pushed to the mirrors later today
 
 (oh, and 3 items in STATUS that need one more review)
 



Re: svn commit: r1100443 - /httpd/httpd/trunk/server/mpm/winnt/service.c

2011-05-07 Thread Jeff Trawick
On Sat, May 7, 2011 at 12:15 AM,  wr...@apache.org wrote:
 Author: wrowe
 Date: Sat May  7 04:15:01 2011
 New Revision: 1100443

 URL: http://svn.apache.org/viewvc?rev=1100443view=rev
 Log:
 Not possible; you don't declare a variable const and then
 maniuplate it.

 Modified:
    httpd/httpd/trunk/server/mpm/winnt/service.c

 Modified: httpd/httpd/trunk/server/mpm/winnt/service.c
 URL: 
 http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/winnt/service.c?rev=1100443r1=1100442r2=1100443view=diff
 ==
 --- httpd/httpd/trunk/server/mpm/winnt/service.c (original)
 +++ httpd/httpd/trunk/server/mpm/winnt/service.c Sat May  7 04:15:01 2011
 @@ -750,7 +750,7 @@ apr_status_t mpm_service_start(apr_pool_
                                const char * const * argv)
  {
     apr_status_t rv;
 -    const CHAR **start_argv;
 +    CHAR **start_argv;

That const was for what start_argv pointed to indirectly, to prevent
start_argv from being used to mangle it (start_argv[0][0] = '!').

Of course the end goal was to shut up the compiler about what
StartService() wanted as the third arg.  I'll look for a solution that
makes both compilers happy.


Re: svn commit: r1100443 - /httpd/httpd/trunk/server/mpm/winnt/service.c

2011-05-07 Thread William A. Rowe Jr.
On 5/7/2011 9:32 AM, Jeff Trawick wrote:
 
 Of course the end goal was to shut up the compiler about what
 StartService() wanted as the third arg.  I'll look for a solution that
 makes both compilers happy.

I suspect that only a cast will accomplish this.  But it should not
be necessary, additive constness should not need to be specified
when passing arguments.  Subtracting constness is another matter.


Re: is anyone planning to TR httpd 2.2.18 real soon?

2011-05-07 Thread William A. Rowe Jr.
On 5/7/2011 8:20 AM, Jim Jagielski wrote:
 I am, once the APR releases happen...

And as the clock rolls over, they are approved.  Although it will
take some time to sync up APR site and dist files, but nothing
preventing a release.  One report raised privately, which caused
me to hold off last night, turns out to be a non-issue.

Please feel free to beat me to it :)  I'm going ahead with a tag
and roll at 0400 GMT if no one else has done so for 2.2.18, and
then for 2.3.11 beta 24 hours after that if its still pending.




Re: is anyone planning to TR httpd 2.2.18 real soon?

2011-05-07 Thread Jim Jagielski

On May 7, 2011, at 6:46 PM, William A. Rowe Jr. wrote:

 On 5/7/2011 8:20 AM, Jim Jagielski wrote:
 I am, once the APR releases happen...
 
 And as the clock rolls over, they are approved.  Although it will
 take some time to sync up APR site and dist files, but nothing
 preventing a release.  One report raised privately, which caused
 me to hold off last night, turns out to be a non-issue.
 
 Please feel free to beat me to it :)  I'm going ahead with a tag
 and roll at 0400 GMT if no one else has done so for 2.2.18, and
 then for 2.3.11 beta 24 hours after that if its still pending.
 

Go ahead and do the 2.2 release and I'll do the 2.3 one...
I have a few outstanding patches to fold into 2.3 before
the release...



Re: Universal setting for APR_HOOK_PROBES_ENABLED

2011-05-07 Thread Jim Jagielski
Here's my idea:

 o create ap_hooks.h which moves the AP_IMPLEMENT_HOOKS* from
   ap_config.h to itself.
 o Fold in the APR_HOOK_PROBES_ENABLED logic as
   described below.
 o change configure to accept --enable-hooks-probes=path
   which points to the location of the ap_hooks_probes.h
   file.
 o Adjust all files to include ap_hooks.h instead
   of apr_hooks.h

I'll start coding this tomorrow; my hope is to have this
in for the next beta (as RM, I'm gonna hold off a TR
until then) since I think it's something that we want
to really promote as a cool 2.4 feature.

On May 6, 2011, at 8:38 PM, Jim Jagielski wrote:

 APR_HOOK_PROBES_ENABLED is pretty nice, the rub is that, esp
 in httpd, we have lots of modules which go ahead and include
 apr_hooks.h on their own, making the setup of universal
 probe hooks difficult.
 
 Anyone have ideas on how to make it easier, either in
 APR directly or in httpd? Maybe making some kind of
 httpd_hooks.h file that includes some local local_hooks_probes.h
 file (depending on some config-time setting) assuming
 APR_HOOK_PROBES_ENABLED is set (ie: --enable-hook-probes
 sets APR_HOOK_PROBES_ENABLED and httpd_hooks.h is
 basically:
 
  #ifdef APR_HOOK_PROBES_ENABLED
  #include local_hooks_probes.h
  #endif
  #include apr_hooks.h
 
 and no httpd files include apr_hooks.h directly...
 
 That seems a very rough way to do it... :/
 



Re: [PATCH] lingering close and event

2011-05-07 Thread Greg Ames
hey I like the concept a lot!  a quick peek at
http://apache.org/server-status shows 15 Cs for threads tied up in lingering
close, something like 50 Keepalive threads, and only 13 threads actually
reading or writing.

On Mon, Apr 25, 2011 at 8:53 PM, Jeff Trawick traw...@gmail.com wrote:

 has anyone played with this before?  I've seen it mentioned, and joe s
 had a patch to create a linger thread for worker back in 2004

 +apr_thread_mutex_lock(timeout_mutex);
+APR_RING_INSERT_TAIL(recv_fin_timeout_head, cs,
conn_state_t, timeout_list);
+apr_thread_mutex_unlock(timeout_mutex);

I see where the cs is removed from the ring for the timeout flow.  but what
about for the normal non-timeout flow?

 rv = apr_pollset_create(event_pollset,
-threads_per_child,
+threads_per_child, /* XXX don't we need
more, to handle
+* connections in K-A
or lingering
+* close?
+*/

IIRC the second arg to apr_pollset_create determines the size of the revents
array used to report ready file descriptors.  If there are ever more ready
fds than slots in the array, it's no big deal.  They get reported as ready
on the next apr_pollset_poll call.  So using threads_per_child is just
picking a number out of the air which happens to go up automatically as the
worker process is configured for higher workloads.

Greg