Re: svn commit: r1877954 - in /httpd/httpd/trunk: docs/log-message-tags/ include/ modules/apreq/ modules/cache/ modules/dav/main/ modules/filters/ modules/http/ modules/mappers/ modules/proxy/ server/

2020-05-20 Thread Ruediger Pluem



On 5/20/20 5:58 PM, Yann Ylavic wrote:
> On Wed, May 20, 2020 at 4:44 PM Ruediger Pluem  wrote:
>>
>>> +return (!r->header_only
>>> +&& (r->kept_body
>>> +|| apr_table_get(r->headers_in, "Transfer-Encoding")
>>> +|| ((cls = apr_table_get(r->headers_in, "Content-Length"))
>>> +&& ap_parse_strict_length(, cls) && cl > 0)));
>>
>> Are we sure that cls is not NULL here? ap_parse_strict_length is not NULL 
>> safe :-)
> 
> Hmm, I suppose that if cls is NULL the next "&& ap_parse.." is not evaluated 
> :)

My bad :-p. cls = apr_table_get(r->headers_in, "Content-Length") already does 
that NULL check already.
All good.

Regards

Rüdiger


Re: svn commit: r1877954 - in /httpd/httpd/trunk: docs/log-message-tags/ include/ modules/apreq/ modules/cache/ modules/dav/main/ modules/filters/ modules/http/ modules/mappers/ modules/proxy/ server/

2020-05-20 Thread Yann Ylavic
On Wed, May 20, 2020 at 4:44 PM Ruediger Pluem  wrote:
>
> > +return (!r->header_only
> > +&& (r->kept_body
> > +|| apr_table_get(r->headers_in, "Transfer-Encoding")
> > +|| ((cls = apr_table_get(r->headers_in, "Content-Length"))
> > +&& ap_parse_strict_length(, cls) && cl > 0)));
>
> Are we sure that cls is not NULL here? ap_parse_strict_length is not NULL 
> safe :-)

Hmm, I suppose that if cls is NULL the next "&& ap_parse.." is not evaluated :)

Regards;
Yann.


Re: svn commit: r1877954 - in /httpd/httpd/trunk: docs/log-message-tags/ include/ modules/apreq/ modules/cache/ modules/dav/main/ modules/filters/ modules/http/ modules/mappers/ modules/proxy/ server/

2020-05-20 Thread Ruediger Pluem



On 5/20/20 4:01 PM, yla...@apache.org wrote:
> Author: ylavic
> Date: Wed May 20 14:01:17 2020
> New Revision: 1877954
> 
> URL: http://svn.apache.org/viewvc?rev=1877954=rev
> Log:
> core,modules: provide/use ap_parse_strict_length() helper.
> 
> It helps simplifying a lot of duplicated code based on apr_strtoff(), while
> also rejecting leading plus/minus signs which are dissalowed in Content-Length
> and (Content-)Range headers.
> 
> Modified:
> httpd/httpd/trunk/docs/log-message-tags/next-number
> httpd/httpd/trunk/include/ap_mmn.h
> httpd/httpd/trunk/include/httpd.h
> httpd/httpd/trunk/modules/apreq/filter.c
> httpd/httpd/trunk/modules/cache/mod_cache.c
> httpd/httpd/trunk/modules/cache/mod_cache_disk.c
> httpd/httpd/trunk/modules/cache/mod_cache_socache.c
> httpd/httpd/trunk/modules/dav/main/mod_dav.c
> httpd/httpd/trunk/modules/filters/mod_data.c
> httpd/httpd/trunk/modules/filters/mod_reflector.c
> httpd/httpd/trunk/modules/filters/mod_request.c
> httpd/httpd/trunk/modules/http/byterange_filter.c
> httpd/httpd/trunk/modules/http/http_filters.c
> httpd/httpd/trunk/modules/mappers/mod_negotiation.c
> httpd/httpd/trunk/modules/proxy/mod_proxy.c
> httpd/httpd/trunk/modules/proxy/mod_proxy_ajp.c
> httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> httpd/httpd/trunk/server/apreq_module_cgi.c
> httpd/httpd/trunk/server/util.c
> 

> Modified: httpd/httpd/trunk/server/util.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1877954=1877953=1877954=diff
> ==
> --- httpd/httpd/trunk/server/util.c (original)
> +++ httpd/httpd/trunk/server/util.c Wed May 20 14:01:17 2020
> @@ -2673,6 +2673,15 @@ AP_DECLARE(apr_status_t) ap_timeout_para
>  return APR_SUCCESS;
>  }
>  
> +AP_DECLARE(int) ap_parse_strict_length(apr_off_t *len, const char *str)
> +{
> +char *end;
> +
> +return (apr_isdigit(*str)
> +&& apr_strtoff(len, str, , 10) == APR_SUCCESS
> +&& *end == '\0');
> +}
> +
>  /**
>   * Determine if a request has a request body or not.
>   *
> @@ -2682,20 +2691,13 @@ AP_DECLARE(apr_status_t) ap_timeout_para
>  AP_DECLARE(int) ap_request_has_body(request_rec *r)
>  {
>  apr_off_t cl;
> -char *estr;
>  const char *cls;
> -int has_body;
>  
> -has_body = (!r->header_only
> -&& (r->kept_body
> -|| apr_table_get(r->headers_in, "Transfer-Encoding")
> -|| ( (cls = apr_table_get(r->headers_in, 
> "Content-Length"))
> -&& (apr_strtoff(, cls, , 10) == APR_SUCCESS)
> -&& (!*estr)
> -&& (cl > 0) )
> -)
> -);
> -return has_body;
> +return (!r->header_only
> +&& (r->kept_body
> +|| apr_table_get(r->headers_in, "Transfer-Encoding")
> +|| ((cls = apr_table_get(r->headers_in, "Content-Length"))
> +&& ap_parse_strict_length(, cls) && cl > 0)));

Are we sure that cls is not NULL here? ap_parse_strict_length is not NULL safe 
:-)

Regards

Rüdiger



Re: Trunk :: crashes libhttpd.dll (scoreboard.c)

2020-05-20 Thread Ruediger Pluem



On 5/20/20 4:20 PM, Steffen Land wrote:
> 
> Running production. Get crashes, mostly few times a day:
> 
> Error log:
> [mpm_winnt:notice] [pid 11936:tid 744] AH00428: Parent: child process 4980 
> exited with status 3221225477 -- Restarting
> 
> The thread tried to read from or write to a virtual address for which it does 
> not have the appropriate access.

Thanks for the details. Does the following patch make them go away?

Index: server/scoreboard.c
===
--- server/scoreboard.c (revision 1877790)
+++ server/scoreboard.c (working copy)
@@ -381,7 +381,7 @@
 if (pfn_ap_logio_get_last_bytes != NULL) {
 bytes = pfn_ap_logio_get_last_bytes(r->connection);
 }
-else if (r->method_number == M_GET && r->method[0] == 'H') {
+else if (r->method_number == M_GET && r->method && r->method[0] == 'H') {
 bytes = 0;
 }
 else {


Regards

Rüdiger


Trunk :: crashes libhttpd.dll (scoreboard.c)

2020-05-20 Thread Steffen Land


Running production. Get crashes, mostly few times a day:

Error log:
[mpm_winnt:notice] [pid 11936:tid 744] AH00428: Parent: child process 
4980 exited with status 3221225477 -- Restarting


The thread tried to read from or write to a virtual address for which 
it does not have the appropriate access.


Attached

scoreboard.png
callstack.png
locals.png


Steffen