Re: httpd test framework svn repo borked?

2022-01-14 Thread William A Rowe Jr
On Fri, Jan 14, 2022 at 4:08 AM Stefan Sperling  wrote:
>
> On Thu, Jan 13, 2022 at 07:17:27PM -0600, William A Rowe Jr wrote:
> > Thanks Stefan, it's attempting [locally] to replace a file, which was
> > just created during the
> > checkout (which might even be open).
>
> Hmm. I assume SVN would close such files based on APR pool lifetime.
>
> Handling of the pristine installation step has been changed in SVN trunk:
> https://svn.apache.org/r1886490
> In particular:
> """
> On Windows, it uses the existing mechanism of installing the files
> by handle, without having to reopen them.
> """

As indicated, this is in the ubuntu 18.04 SFU on windows, the msys2-64
current distribution, as well as Windows "native". Very consistent.

> If you are running 1.14 or an older release, perhaps try a client
> built from trunk to see if this issue has already been fixed for
> a future SVN 1.15 release?

I'm happy to have a look next week.

> > So it looks like some hash
> > collision or other
> > duplication issue where there are two attempts to emplace the same
> > hash svn-base image,
> > and linux is letting this happen without complaint, while windows refuses.
>
> Does the same file content (ignoring expanded keywords, if any) exist at
> multiple paths? If so, such paths would naturally share the same pristine
> based on the hash of file contents.


Re: Possible apr 1.7.1 showstopper from httpd test framework

2022-01-14 Thread William A Rowe Jr
On Fri, Jan 14, 2022 at 5:44 AM Joe Orton  wrote:
>
> On Fri, Jan 14, 2022 at 11:37:35AM +0100, Ruediger Pluem wrote:
> >
> > On 1/14/22 6:47 AM, William A Rowe Jr wrote:
> > > In addition to a broken release of brotli (where enc/dec don't specify
> > > -lbrotlicommon,
> > > even on trunk, for openssl and other consumers to ferret out that 
> > > binding), and
> > > lots of fun changes to build flags in curl 7.81 minor release (who does 
> > > that?)
> > > there appears to be one test failure with date formatting in httpd 2.4.x 
> > > branch
> > > (including release 2.4.52) and apr 1.7.x branch (including release 1.7.0);
> > >
> > > t/modules/include.t . 56/98 # Failed test 64 in
> > > t/modules/include.t at line 373
> > >
> > > Have not had time to investigate whether this is a change in perl 
> > > behavior, or
> > > possibly a regression caused by apr datetime handling in 1.7.x itself., 
> > > but any
> > > release apr-side should hold off just a bit to resolve this question.
> >
> > I cannot reproduce this with APR 1.7.x on RedHat 8 and our Travis builds at 
> > least for some builds
> > on Ubuntu use APR 1.7 as well and do not fail.
> > Is this probably a Windows specific issue? Can anyone reproduce on Windows?

Fun guess. Not on Windows at the moment. This is on ubuntu 18.04
running as a vmw
workstation guest vm with the following snapshot revisions (all just a
bit beyond the
current releases, including apr 1.7;

apr_ver=1.7.x-1896808
aprutil_ver=1.7.x-1894932
brotli_rev=e83c7b8
brotli_ver=master
curl_rev=d4492b6d1
curl_ver=master
expat_rev=9c42ebdd
expat_ver=master
httpd_ver=2.4.x-1896743
httpdtest_rev=763e0201
httpdtest_ver=trunk
jansson_rev=addeeef
jansson_ver=master
libxml2_rev=dea91c97
libxml2_ver=master
lua_ver=5.4.3
nghttp2_rev=02e6cad1
nghttp2_ver=master
openssl_rev=2080da84a4
openssl_ver=master
pcre_rev=81d3729
pcre_ver=master
zlib_rev=cacf7f1
zlib_ver=master

> IIRC there is some test case which can be sensitive to filesystems, and
> e.g. sometimes fails if NFS mounted?  I may be mixing it up with another
> test. The output of "./t/TEST -v t/modules/include.t" should help
> diagnose.

I will dig deeper and check some other linux flavors.

> That phrase "including release 1.7.0" implies it's not a 1.7.x
> regression if it also failed with 1.7.0, Bill? i.e. no reason to hold up
> a release?

It's a good point, I just don't care to tag a release when we have been making
changes to apr time logic as part of the next release. Hopefully it is something
interesting in the local deployment and perhaps a bad assumption in the test?
Will report back.


Re: [Regression in httpd 2.4.52] mod_dav: Potentially unbounded memory usage in PROPFIND with dav_get_props() and dav_propfind_walker()

2022-01-14 Thread Ruediger Pluem



On 1/14/22 1:57 PM, Evgeny Kotkov wrote:
> Hi,
> 
> I might have stumbled across a regression in httpd 2.4.52 where mod_dav was
> changed in a way where dav_get_props() now allocates data in resource->pool.
> 
> I think that r1879889 [1] is the change that is causing the new behavior.
> This change has been backported to 2.4.x in r1895893 [2].
> 
> Consider the new part of the dav_get_props() function:
> 
> DAV_DECLARE(dav_get_props_result) dav_get_props()
> {
>…
>element = apr_pcalloc(propdb->resource->pool, sizeof(dav_liveprop_elem));
>element->doc = doc;
>…
> 
> This code unconditionally allocates data in resource->pool (this is the only
> such place, other allocations performed by this function happen in propdb->p).
> 
> The dav_get_props() is called by dav_propfind_walker().  The walker function
> is driven by a specific provider.  Both of the two implementations known to
> me, mod_dav_fs and mod_dav_svn, happen to use a long-living pool as the
> resource->pool during the walk.
> 
> I think that with this change, the PROPFIND walks are going to make O(N)
> allocations for O(N) walked items — which is an unbounded memory usage
> and a regression, compared to 2.4.51.
> 
> [1] https://svn.apache.org/r1879889
> [2] https://svn.apache.org/r1895893

Can you please check if the below patch fixes your issue?

Index: modules/dav/main/props.c
===
--- modules/dav/main/props.c(revision 1896810)
+++ modules/dav/main/props.c(working copy)
@@ -805,9 +805,15 @@
 /* we lose both the document and the element when calling (insert_prop),
  * make these available in the pool.
  */
-element = apr_pcalloc(propdb->resource->pool, sizeof(dav_liveprop_elem));
+element = dav_get_liveprop_element(propdb->resource);
+if (!element) {
+element = apr_pcalloc(propdb->resource->pool, 
sizeof(dav_liveprop_elem));
+apr_pool_userdata_setn(element, DAV_PROP_ELEMENT, NULL, 
propdb->resource->pool);
+}
+else {
+memset(element, 0, sizeof(dav_liveprop_elem));
+}
 element->doc = doc;
-apr_pool_userdata_setn(element, DAV_PROP_ELEMENT, NULL, 
propdb->resource->pool);

 /* ### NOTE: we should pass in TWO buffers -- one for keys, one for
the marks */


Regards

Rüdiger



[Regression in httpd 2.4.52] mod_dav: Potentially unbounded memory usage in PROPFIND with dav_get_props() and dav_propfind_walker()

2022-01-14 Thread Evgeny Kotkov
Hi,

I might have stumbled across a regression in httpd 2.4.52 where mod_dav was
changed in a way where dav_get_props() now allocates data in resource->pool.

I think that r1879889 [1] is the change that is causing the new behavior.
This change has been backported to 2.4.x in r1895893 [2].

Consider the new part of the dav_get_props() function:

DAV_DECLARE(dav_get_props_result) dav_get_props()
{
   …
   element = apr_pcalloc(propdb->resource->pool, sizeof(dav_liveprop_elem));
   element->doc = doc;
   …

This code unconditionally allocates data in resource->pool (this is the only
such place, other allocations performed by this function happen in propdb->p).

The dav_get_props() is called by dav_propfind_walker().  The walker function
is driven by a specific provider.  Both of the two implementations known to
me, mod_dav_fs and mod_dav_svn, happen to use a long-living pool as the
resource->pool during the walk.

I think that with this change, the PROPFIND walks are going to make O(N)
allocations for O(N) walked items — which is an unbounded memory usage
and a regression, compared to 2.4.51.

[1] https://svn.apache.org/r1879889
[2] https://svn.apache.org/r1895893


Thanks,
Evgeny Kotkov


Re: Possible apr 1.7.1 showstopper from httpd test framework

2022-01-14 Thread Joe Orton
On Fri, Jan 14, 2022 at 11:37:35AM +0100, Ruediger Pluem wrote:
> 
> 
> On 1/14/22 6:47 AM, William A Rowe Jr wrote:
> > In addition to a broken release of brotli (where enc/dec don't specify
> > -lbrotlicommon,
> > even on trunk, for openssl and other consumers to ferret out that binding), 
> > and
> > lots of fun changes to build flags in curl 7.81 minor release (who does 
> > that?)
> > there appears to be one test failure with date formatting in httpd 2.4.x 
> > branch
> > (including release 2.4.52) and apr 1.7.x branch (including release 1.7.0);
> > 
> > t/modules/include.t . 56/98 # Failed test 64 in
> > t/modules/include.t at line 373
> > 
> > Have not had time to investigate whether this is a change in perl behavior, 
> > or
> > possibly a regression caused by apr datetime handling in 1.7.x itself., but 
> > any
> > release apr-side should hold off just a bit to resolve this question.
> 
> I cannot reproduce this with APR 1.7.x on RedHat 8 and our Travis builds at 
> least for some builds
> on Ubuntu use APR 1.7 as well and do not fail.
> Is this probably a Windows specific issue? Can anyone reproduce on Windows?

IIRC there is some test case which can be sensitive to filesystems, and 
e.g. sometimes fails if NFS mounted?  I may be mixing it up with another 
test. The output of "./t/TEST -v t/modules/include.t" should help 
diagnose.

That phrase "including release 1.7.0" implies it's not a 1.7.x 
regression if it also failed with 1.7.0, Bill? i.e. no reason to hold up 
a release?

Regards, Joe



Re: Possible apr 1.7.1 showstopper from httpd test framework

2022-01-14 Thread Ruediger Pluem



On 1/14/22 6:47 AM, William A Rowe Jr wrote:
> In addition to a broken release of brotli (where enc/dec don't specify
> -lbrotlicommon,
> even on trunk, for openssl and other consumers to ferret out that binding), 
> and
> lots of fun changes to build flags in curl 7.81 minor release (who does that?)
> there appears to be one test failure with date formatting in httpd 2.4.x 
> branch
> (including release 2.4.52) and apr 1.7.x branch (including release 1.7.0);
> 
> t/modules/include.t . 56/98 # Failed test 64 in
> t/modules/include.t at line 373
> 
> Have not had time to investigate whether this is a change in perl behavior, or
> possibly a regression caused by apr datetime handling in 1.7.x itself., but 
> any
> release apr-side should hold off just a bit to resolve this question.

I cannot reproduce this with APR 1.7.x on RedHat 8 and our Travis builds at 
least for some builds
on Ubuntu use APR 1.7 as well and do not fail.
Is this probably a Windows specific issue? Can anyone reproduce on Windows?

Regards

Rüdiger



Re: httpd test framework svn repo borked?

2022-01-14 Thread Stefan Sperling
On Thu, Jan 13, 2022 at 07:17:27PM -0600, William A Rowe Jr wrote:
> Thanks Stefan, it's attempting [locally] to replace a file, which was
> just created during the
> checkout (which might even be open).

Hmm. I assume SVN would close such files based on APR pool lifetime.

Handling of the pristine installation step has been changed in SVN trunk:
https://svn.apache.org/r1886490
In particular:
"""
On Windows, it uses the existing mechanism of installing the files
by handle, without having to reopen them.
"""

If you are running 1.14 or an older release, perhaps try a client
built from trunk to see if this issue has already been fixed for
a future SVN 1.15 release?

> So it looks like some hash
> collision or other
> duplication issue where there are two attempts to emplace the same
> hash svn-base image,
> and linux is letting this happen without complaint, while windows refuses.

Does the same file content (ignoring expanded keywords, if any) exist at
multiple paths? If so, such paths would naturally share the same pristine
based on the hash of file contents.