From 6c5a9fdab1bffdda02ee7a720c70f1271178be15 Mon Sep 17 00:00:00 2001 From: Paul Lodge Date: Wed, 7 Oct 2026 12:12:01 +0200 Subject: [PATCH 1/3] Add WebDAV CRUD and locking tests Co-Authored-By: Claude Opus 4.6 --- test/modules/dav/test_001_propdb_ns_limit.py | 4 +- test/modules/dav/test_002_operations.py | 223 +++++++++++++++++++ test/modules/dav/test_003_if_header.py | 57 +++++ 3 files changed, 282 insertions(+), 2 deletions(-) create mode 100644 test/modules/dav/test_002_operations.py create mode 100644 test/modules/dav/test_003_if_header.py diff --git a/test/modules/dav/test_001_propdb_ns_limit.py b/test/modules/dav/test_001_propdb_ns_limit.py index b9d65b8cf19..10fd4035c42 100644 --- a/test/modules/dav/test_001_propdb_ns_limit.py +++ b/test/modules/dav/test_001_propdb_ns_limit.py @@ -49,7 +49,7 @@ def _proppatch(self, env, body: str): '--data-binary', f'@{fpath}']) # a modest number of distinct namespaces is accepted and stored - def test_dav_001_01(self, env): + def test_dav_001_01_simple_distinct_ns(self, env): r = self._proppatch(env, self._propertyupdate(100, 'a')) assert r.response["status"] == 207 assert 'HTTP/1.1 200' in r.stdout @@ -58,7 +58,7 @@ def test_dav_001_01(self, env): # declaring more distinct namespaces than one property database will # record must be refused with 413 rather than accepted, and the resource # must remain usable afterwards - def test_dav_001_02(self, env): + def test_dav_001_02_complex_distinct_ns(self, env): r = self._proppatch(env, self._propertyupdate(2049, 'b')) # the refusal is logged at error level, and writing back the # oversized namespace table on close fails with a warning diff --git a/test/modules/dav/test_002_operations.py b/test/modules/dav/test_002_operations.py new file mode 100644 index 00000000000..616ead7d8c0 --- /dev/null +++ b/test/modules/dav/test_002_operations.py @@ -0,0 +1,223 @@ +import os +import sys +import shutil + +import pytest + +from pyhttpd.conf import HttpdConf + + +class TestDavOperations: + + @pytest.fixture(autouse=True, scope='class') + def _class_scope(self, env): + dav_dir = os.path.join(env.gen_dir, 'dav-ops') + if os.path.exists(dav_dir): + shutil.rmtree(dav_dir) + os.makedirs(dav_dir, exist_ok=True) + import glob + for f in glob.glob(os.path.join(env.gen_dir, 'davlock-ops*')): + os.remove(f) + with open(os.path.join(dav_dir, 'hello.txt'), 'w') as fd: + fd.write('hello') + dav_dir_conf = dav_dir.replace('\\', '/') + lock_db_conf = os.path.join(env.gen_dir, 'davlock-ops').replace('\\', '/') + conf = HttpdConf(env, extras={ + 'base': f""" + DavLockDB "{lock_db_conf}" + """, + f"test1.{env.http_tld}": f""" + Alias /dav-ops "{dav_dir_conf}" + + Dav On + Require all granted + + """, + }) + conf.add_vhost_test1() + conf.install() + assert env.apache_restart() == 0 + + def _dav_url(self, env, path=''): + return env.mkurl("http", "test1", f"/dav-ops/{path}") + + def _write_dav_file(self, env, name, content): + """Write a file directly into the DAV directory on disk.""" + dav_dir = os.path.join(env.gen_dir, 'dav-ops') + fpath = os.path.join(dav_dir, name) + with open(fpath, 'w') as fd: + fd.write(content) + + def _remove_dav_file(self, env, name): + """Remove a file from the DAV directory if it exists.""" + fpath = os.path.join(env.gen_dir, 'dav-ops', name) + if os.path.isfile(fpath): + os.remove(fpath) + + def _write_temp(self, env, name, content): + """Write a temporary file outside the DAV directory for uploads.""" + fpath = os.path.join(env.gen_dir, name) + with open(fpath, 'w') as fd: + fd.write(content) + return fpath + + @staticmethod + def _lock_body(): + return ('' + '' + '' + '' + 'test-owner' + '') + + # HEAD request on a DAV resource returns 200 + def test_dav_002_01(self, env): + self._write_dav_file(env, 'hello.txt', 'hello') + url = self._dav_url(env, 'hello.txt') + r = env.curl_raw(url, options=['-I']) + assert r.response["status"] == 200 + + # GET retrieves the correct file content + def test_dav_002_02(self, env): + self._write_dav_file(env, 'hello.txt', 'hello') + url = self._dav_url(env, 'hello.txt') + r = env.curl_raw(url) + assert r.response["status"] == 200 + assert r.stdout.strip() == 'hello' + + # PUT uploads content, subsequent GET returns the same content + + def test_dav_002_03(self, env): + self._remove_dav_file(env, 'put_test.txt') + content = 'uploaded via put' + fpath = self._write_temp(env, 'put_content.txt', content) + url = self._dav_url(env, 'put_test.txt') + r = env.curl_raw(url, options=['-T', fpath]) + assert r.response["status"] == 201 + # verify with GET + r = env.curl_raw(url) + assert r.response["status"] == 200 + assert r.stdout.strip() == content + + # COPY duplicates a file; the copy has the original content + + def test_dav_002_04(self, env): + self._write_dav_file(env, 'hello.txt', 'hello') + self._remove_dav_file(env, 'hello_copy.txt') + src_url = self._dav_url(env, 'hello.txt') + dest_url = self._dav_url(env, 'hello_copy.txt') + r = env.curl_raw(src_url, options=[ + '-X', 'COPY', + '-H', f'Destination: {dest_url}']) + assert r.response["status"] == 201 + # verify the copy has the original content + r = env.curl_raw(dest_url) + assert r.response["status"] == 200 + assert r.stdout.strip() == 'hello' + + # MOVE transfers a file; the source returns 404 afterwards + def test_dav_002_05(self, env): + self._write_dav_file(env, 'move_src.txt', 'move me') + self._remove_dav_file(env, 'move_dest.txt') + src_url = self._dav_url(env, 'move_src.txt') + dest_url = self._dav_url(env, 'move_dest.txt') + r = env.curl_raw(src_url, options=[ + '-X', 'MOVE', + '-H', f'Destination: {dest_url}']) + assert r.response["status"] == 201 + # source should be gone + r = env.curl_raw(src_url) + assert r.response["status"] == 404 + # destination should have the content + r = env.curl_raw(dest_url) + assert r.response["status"] == 200 + assert r.stdout.strip() == 'move me' + + # DELETE removes a file; subsequent GET returns 404 + def test_dav_002_06(self, env): + self._write_dav_file(env, 'delete_me.txt', 'delete this') + url = self._dav_url(env, 'delete_me.txt') + r = env.curl_raw(url, options=['-X', 'DELETE']) + assert r.response["status"] == 204 + # verify the file is gone + r = env.curl_raw(url) + assert r.response["status"] == 404 + + # PROPFIND with Depth:1 returns a listing that includes file names + + def test_dav_002_07(self, env): + self._write_dav_file(env, 'hello.txt', 'hello') + url = self._dav_url(env) + r = env.curl_raw(url, options=[ + '-X', 'PROPFIND', + '-H', 'Depth: 1']) + assert r.response["status"] == 207 + assert 'hello.txt' in r.stdout + + # MKCOL creates a collection; PROPFIND on the parent shows it + + def test_dav_002_08(self, env): + col_dir = os.path.join(env.gen_dir, 'dav-ops', 'newcol') + shutil.rmtree(col_dir, ignore_errors=True) + url = self._dav_url(env, 'newcol') + r = env.curl_raw(url, options=['-X', 'MKCOL']) + assert r.response["status"] == 201 + # verify the collection appears in the parent listing + parent_url = self._dav_url(env) + r = env.curl_raw(parent_url, options=[ + '-X', 'PROPFIND', + '-H', 'Depth: 1']) + assert r.response["status"] == 207 + assert 'newcol' in r.stdout + + # After LOCK, PUT/DELETE/MOVE fail with 423 Locked but GET still works + + def test_dav_002_09(self, env): + self._write_dav_file(env, 'lock_test.txt', 'lock me') + url = self._dav_url(env, 'lock_test.txt') + fpath = self._write_temp(env, 'lock_body.xml', self._lock_body()) + r = env.curl_raw(url, options=[ + '-X', 'LOCK', + '-H', 'Content-Type: text/xml', + '--data-binary', f'@{fpath}']) + assert r.response["status"] == 200 + # PUT should fail with 423 + put_fpath = self._write_temp(env, 'put_locked.txt', 'overwrite attempt') + r = env.curl_raw(url, options=['-T', put_fpath]) + assert r.response["status"] == 423 + # DELETE should fail with 423 + r = env.curl_raw(url, options=['-X', 'DELETE']) + assert r.response["status"] == 423 + # MOVE should fail with 423 + move_dest = self._dav_url(env, 'lock_moved.txt') + r = env.curl_raw(url, options=[ + '-X', 'MOVE', + '-H', f'Destination: {move_dest}']) + assert r.response["status"] == 423 + # GET should still succeed on a write-locked resource + r = env.curl_raw(url) + assert r.response["status"] == 200 + assert r.stdout.strip() == 'lock me' + env.httpd_error_log.ignore_recent( + matches=[r'.*dav:error.*']) + + # After LOCK then UNLOCK, DELETE succeeds + def test_dav_002_10(self, env): + self._write_dav_file(env, 'unlock_test.txt', 'unlock me') + url = self._dav_url(env, 'unlock_test.txt') + fpath = self._write_temp(env, 'unlock_body.xml', self._lock_body()) + r = env.curl_raw(url, options=[ + '-X', 'LOCK', + '-H', 'Content-Type: text/xml', + '--data-binary', f'@{fpath}']) + assert r.response["status"] == 200 + # extract the lock token from the response header + lock_token = r.response["header"]["lock-token"] + # unlock the resource + r = env.curl_raw(url, options=[ + '-X', 'UNLOCK', + '-H', f'Lock-Token: {lock_token}']) + assert r.response["status"] == 204 + # DELETE should now succeed + r = env.curl_raw(url, options=['-X', 'DELETE']) + assert r.response["status"] == 204 diff --git a/test/modules/dav/test_003_if_header.py b/test/modules/dav/test_003_if_header.py new file mode 100644 index 00000000000..15ac060bc5a --- /dev/null +++ b/test/modules/dav/test_003_if_header.py @@ -0,0 +1,57 @@ +import os +import sys +import pytest + +from pyhttpd.conf import HttpdConf + + +class TestDavIfHeader: + """mod_dav out-of-bounds read/write of zero byte when parsing a + malformed 'Not' token in the If: header. The fix should reject + the malformed header with 400 Bad Request.""" + + @pytest.fixture(autouse=True, scope='class') + def _class_scope(self, env): + dav_dir = os.path.join(env.gen_dir, 'dav-if') + os.makedirs(dav_dir, exist_ok=True) + with open(os.path.join(dav_dir, "hello.txt"), "w") as f: + f.write("hello") + dav_dir_conf = dav_dir.replace('\\', '/') + lock_db_conf = os.path.join(env.gen_dir, 'davlock-if').replace('\\', '/') + conf = HttpdConf(env, extras={ + 'base': f""" + DavLockDB "{lock_db_conf}" + """, + f"test1.{env.http_tld}": f""" + Alias /dav-if "{dav_dir_conf}" + + Dav On + Require all granted + + LogLevel debug + """, + }) + conf.add_vhost_test1() + conf.install() + assert env.apache_restart() == 0 + + @pytest.mark.skipif(sys.platform == 'win32', + reason="crashes Apache child process on Windows") + def test_dav_003_01(self, env): + """COPY with truncated 'Not' in If: header should return 400, + not crash or succeed.""" + url = env.mkurl("http", "test1", "/dav-if/hello.txt") + dest = env.mkurl("http", "test1", "/dav-if/hello2.txt") + r = env.curl_raw(url, options=[ + '-X', 'COPY', + '-H', f'Destination: {dest}', + '-H', 'Overwrite: T', + '-H', 'If: (N)', + '-v', + ]) + assert r.response is not None + assert r.response["status"] == 400, \ + f"Malformed If: header should produce 400, got {r.response['status']}" + env.httpd_error_log.ignore_recent( + matches=[r'.*Invalid.*If.*header.*', r'.*dav:error.*', + r'.*:error\].*']) From 12476f032be11743a4ef6447fd83b5882ad9ce87 Mon Sep 17 00:00:00 2001 From: Paul Lodge Date: Wed, 4 Nov 2026 12:24:10 +0100 Subject: [PATCH 2/3] Fix for crash on windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Crash (STATUS_ACCESS_VIOLATION / 0xC0000005) on all DAV write operations targeting non-existent resources (PUT to new file, COPY/MOVE to new destination, MKCOL): dav_fs_quota_precondition() in modules/dav/fs/quota.c called dav_fs_fname(src/dst) to get the filesystem path dav_fs_fname() returns resource->info->finfo.fname — APR's stat result On Windows, apr_stat() calls fillin_fileinfo() which does memset(finfo, '\0', ...), zeroing finfo->fname = NULL when the file doesn't exist On Linux, apr_stat() sets finfo->fname = fname (the input path) even when stat succeeds, but for non-existent resources Apache pre-assigns r->finfo.fname from r->filename, so it is non-NULL ap_make_dirstr_parent(pool, NULL) → ap_strrchr_c(NULL, '/') → crash The crash has been verified by hand to ensure that the AI produced a correct analysis the associated tests pass as expected, both on Linux which was not affected, and on Windows. Co-Authored-By: BOB --- modules/dav/fs/quota.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/dav/fs/quota.c b/modules/dav/fs/quota.c index 7d239f1c478..19bff2fe2e8 100644 --- a/modules/dav/fs/quota.c +++ b/modules/dav/fs/quota.c @@ -279,9 +279,9 @@ int dav_fs_quota_precondition(request_rec *r, */ if (dst == NULL) goto out; - path = dav_fs_fname(dst); + path = dav_fs_pathname(dst); } else { - path = dav_fs_fname(src); + path = dav_fs_pathname(src); } path = ap_make_dirstr_parent(r->pool, path); From e13b1ca1b104ea43cfee86fe357d31394548eb4b Mon Sep 17 00:00:00 2001 From: Paul Lodge Date: Wed, 4 Nov 2026 12:43:19 +0100 Subject: [PATCH 3/3] Updated test naming --- ...2_operations.py => test_004_operations.py} | 25 ++++++++----------- ...003_if_header.py => test_005_if_header.py} | 4 +-- 2 files changed, 11 insertions(+), 18 deletions(-) rename test/modules/dav/{test_002_operations.py => test_004_operations.py} (94%) rename test/modules/dav/{test_003_if_header.py => test_005_if_header.py} (92%) diff --git a/test/modules/dav/test_002_operations.py b/test/modules/dav/test_004_operations.py similarity index 94% rename from test/modules/dav/test_002_operations.py rename to test/modules/dav/test_004_operations.py index 616ead7d8c0..b011042a9dc 100644 --- a/test/modules/dav/test_002_operations.py +++ b/test/modules/dav/test_004_operations.py @@ -71,14 +71,14 @@ def _lock_body(): '') # HEAD request on a DAV resource returns 200 - def test_dav_002_01(self, env): + def test_dav_004_01_head_req(self, env): self._write_dav_file(env, 'hello.txt', 'hello') url = self._dav_url(env, 'hello.txt') r = env.curl_raw(url, options=['-I']) assert r.response["status"] == 200 # GET retrieves the correct file content - def test_dav_002_02(self, env): + def test_dav_004_02_get_req(self, env): self._write_dav_file(env, 'hello.txt', 'hello') url = self._dav_url(env, 'hello.txt') r = env.curl_raw(url) @@ -86,8 +86,7 @@ def test_dav_002_02(self, env): assert r.stdout.strip() == 'hello' # PUT uploads content, subsequent GET returns the same content - - def test_dav_002_03(self, env): + def test_dav_004_03_put_req(self, env): self._remove_dav_file(env, 'put_test.txt') content = 'uploaded via put' fpath = self._write_temp(env, 'put_content.txt', content) @@ -100,8 +99,7 @@ def test_dav_002_03(self, env): assert r.stdout.strip() == content # COPY duplicates a file; the copy has the original content - - def test_dav_002_04(self, env): + def test_dav_004_04_copy_req(self, env): self._write_dav_file(env, 'hello.txt', 'hello') self._remove_dav_file(env, 'hello_copy.txt') src_url = self._dav_url(env, 'hello.txt') @@ -116,7 +114,7 @@ def test_dav_002_04(self, env): assert r.stdout.strip() == 'hello' # MOVE transfers a file; the source returns 404 afterwards - def test_dav_002_05(self, env): + def test_dav_004_05_move_req(self, env): self._write_dav_file(env, 'move_src.txt', 'move me') self._remove_dav_file(env, 'move_dest.txt') src_url = self._dav_url(env, 'move_src.txt') @@ -134,7 +132,7 @@ def test_dav_002_05(self, env): assert r.stdout.strip() == 'move me' # DELETE removes a file; subsequent GET returns 404 - def test_dav_002_06(self, env): + def test_dav_004_06_delete_req(self, env): self._write_dav_file(env, 'delete_me.txt', 'delete this') url = self._dav_url(env, 'delete_me.txt') r = env.curl_raw(url, options=['-X', 'DELETE']) @@ -144,8 +142,7 @@ def test_dav_002_06(self, env): assert r.response["status"] == 404 # PROPFIND with Depth:1 returns a listing that includes file names - - def test_dav_002_07(self, env): + def test_dav_004_07_prpfind_req(self, env): self._write_dav_file(env, 'hello.txt', 'hello') url = self._dav_url(env) r = env.curl_raw(url, options=[ @@ -155,8 +152,7 @@ def test_dav_002_07(self, env): assert 'hello.txt' in r.stdout # MKCOL creates a collection; PROPFIND on the parent shows it - - def test_dav_002_08(self, env): + def test_dav_004_08_mkcol_req(self, env): col_dir = os.path.join(env.gen_dir, 'dav-ops', 'newcol') shutil.rmtree(col_dir, ignore_errors=True) url = self._dav_url(env, 'newcol') @@ -171,8 +167,7 @@ def test_dav_002_08(self, env): assert 'newcol' in r.stdout # After LOCK, PUT/DELETE/MOVE fail with 423 Locked but GET still works - - def test_dav_002_09(self, env): + def test_dav_004_09_check_fail(self, env): self._write_dav_file(env, 'lock_test.txt', 'lock me') url = self._dav_url(env, 'lock_test.txt') fpath = self._write_temp(env, 'lock_body.xml', self._lock_body()) @@ -202,7 +197,7 @@ def test_dav_002_09(self, env): matches=[r'.*dav:error.*']) # After LOCK then UNLOCK, DELETE succeeds - def test_dav_002_10(self, env): + def test_dav_004_10_after_lock_unlock_delete(self, env): self._write_dav_file(env, 'unlock_test.txt', 'unlock me') url = self._dav_url(env, 'unlock_test.txt') fpath = self._write_temp(env, 'unlock_body.xml', self._lock_body()) diff --git a/test/modules/dav/test_003_if_header.py b/test/modules/dav/test_005_if_header.py similarity index 92% rename from test/modules/dav/test_003_if_header.py rename to test/modules/dav/test_005_if_header.py index 15ac060bc5a..298698b9d39 100644 --- a/test/modules/dav/test_003_if_header.py +++ b/test/modules/dav/test_005_if_header.py @@ -35,9 +35,7 @@ def _class_scope(self, env): conf.install() assert env.apache_restart() == 0 - @pytest.mark.skipif(sys.platform == 'win32', - reason="crashes Apache child process on Windows") - def test_dav_003_01(self, env): + def test_dav_005_01_copy_with_trucated_not(self, env): """COPY with truncated 'Not' in If: header should return 400, not crash or succeed.""" url = env.mkurl("http", "test1", "/dav-if/hello.txt")