Re: svn commit: r1100200 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS modules/http/http_filters.c server/protocol.c
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
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
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
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?
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
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
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?
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?
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
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
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