mod_allowmethods: Populate Allow for denied methods

changes-entries/allowmethods-allow-header.txt

@@ -0,0 +1,2 @@
+  *) mod_allowmethods: Send the configured methods in the Allow header of the
+     405 response instead of only TRACE.

modules/aaa/mod_allowmethods.c

@@ -53,6 +53,24 @@ typedef struct am_conf_t {
 
 module AP_MODULE_DECLARE_DATA allowmethods_module;
 
+/* Publish the configured methods for the Allow field of the 405 response.
+ * TRACE is left out, it is added by the core according to TraceEnable.
+ */
+static void am_publish_allowed(request_rec *r, ap_method_mask_t allowed)
+{
+    int m;
+
+    ap_allow_methods(r, 1, NULL);
+    for (m = 0; m < METHODS; m++) {
+        const char *name;
+
+        if (m != M_TRACE && (allowed & (AP_METHOD_BIT << m))
+            && (name = ap_method_name_of(r->pool, m)) != NULL) {
+            ap_allow_methods(r, 0, name, NULL);
+        }
+    }
+}
+
 static int am_check_access(request_rec *r)
 {
     int method = r->method_number;
@@ -77,6 +95,7 @@ static int am_check_access(request_rec *r)
                   r->filename ? "" : "uri ",
                   r->filename ? r->filename : r->uri);
 
+    am_publish_allowed(r, conf->allowed);
     return HTTP_METHOD_NOT_ALLOWED;
 }
 

test/modules/core/env.py

@@ -12,7 +12,7 @@ class CoreTestSetup(HttpdTestSetup):
     def __init__(self, env: 'HttpdTestEnv'):
         super().__init__(env=env)
         self.add_source_dir(os.path.dirname(inspect.getfile(CoreTestSetup)))
-        self.add_modules(["cgid","include","userdir","suexec","headers"])
+        self.add_modules(["cgid","include","userdir","suexec","headers","allowmethods"])
 
 
 class CoreTestEnv(HttpdTestEnv):

test/modules/core/htdocs/allowmethods/get-head-options/f.txt

@@ -0,0 +1 @@
+ok

test/modules/core/htdocs/allowmethods/get-post-options/f.txt

@@ -0,0 +1 @@
+ok

test/modules/core/htdocs/allowmethods/get-post/f.txt

@@ -0,0 +1 @@
+ok

test/modules/core/htdocs/allowmethods/get/f.txt

@@ -0,0 +1 @@
+ok

test/modules/core/htdocs/allowmethods/merge/child/f.txt

@@ -0,0 +1 @@
+ok

test/modules/core/htdocs/allowmethods/merge/f.txt

@@ -0,0 +1 @@
+ok

test/modules/core/htdocs/allowmethods/plain/f.txt

@@ -0,0 +1 @@
+ok

test/modules/core/test_010_allowmethods_allow.py

@@ -0,0 +1,121 @@
+import pytest
+
+from pyhttpd.conf import HttpdConf
+
+BASE = "/allowmethods"
+
+
+def _conf(env, trace):
+    conf = HttpdConf(env)
+    conf.add(f"TraceEnable {trace}")
+    conf.start_vhost(domains=[f"test1.{env.http_tld}"], port=env.http_port,
+                     doc_root="htdocs/allowmethods", with_ssl=False)
+    conf.add([
+        '<Location "/get-head-options/">',
+        '    AllowMethods GET HEAD OPTIONS',
+        '</Location>',
+        '<Location "/get-post-options/">',
+        '    AllowMethods GET POST OPTIONS',
+        '</Location>',
+        '<Location "/get/">',
+        '    AllowMethods GET',
+        '</Location>',
+        '<Location "/get-post/">',
+        '    AllowMethods GET POST',
+        '</Location>',
+        '<Location "/merge/">',
+        '    AllowMethods GET POST OPTIONS',
+        '</Location>',
+        '<Location "/merge/child/">',
+        '    AllowMethods -POST',
+        '</Location>',
+    ])
+    conf.end_vhost()
+    conf.install()
+    assert env.apache_restart() == 0
+
+
+def _request(env, method, path):
+    options = ['-I'] if method == 'HEAD' else ['-X', method]
+    r = env.curl_get(env.mkurl("http", "test1", f"{path}f.txt"),
+                     options=options)
+    assert r.response, f"no response: {r.stderr}"
+    # denied on purpose
+    env.httpd_error_log.ignore_recent(lognos=["AH01623", "AH00135"])
+    allow = r.response["header"].get("allow")
+    methods = None if allow is None else \
+        {m.strip() for m in allow.split(',') if m.strip()}
+    return r.response["status"], methods
+
+
+class TestAllowMethodsAllow:
+
+    @pytest.fixture(autouse=True, scope='class')
+    def _class_scope(self, env):
+        _conf(env, "on")
+
+    # a 405 lists the methods the resource does support
+    @pytest.mark.parametrize("path, supported", [
+        ("/get-head-options/", {"GET", "HEAD", "OPTIONS"}),
+        ("/get-post-options/", {"GET", "HEAD", "POST", "OPTIONS"}),
+        ("/get/", {"GET", "HEAD"}),
+        ("/get-post/", {"GET", "HEAD", "POST"}),
+        ("/merge/", {"GET", "HEAD", "POST", "OPTIONS"}),
+        ("/merge/child/", {"GET", "HEAD", "OPTIONS"}),
+    ])
+    @pytest.mark.parametrize("method", ["PUT", "DELETE", "PATCH"])
+    def test_core_010_001(self, env, path, supported, method):
+        status, allow = _request(env, method, path)
+        assert status == 405
+        # TRACE is not up to AllowMethods
+        assert allow == supported | {"TRACE"}, f"{method} {path}: {allow}"
+
+    def test_core_010_002(self, env):
+        status, allow = _request(env, "POST", "/get-head-options/")
+        assert status == 405
+        assert allow == {"GET", "HEAD", "OPTIONS", "TRACE"}
+        status, allow = _request(env, "POST", "/get/")
+        assert status == 405
+        assert allow == {"GET", "HEAD", "TRACE"}
+
+    # the methods that are listed do work
+    @pytest.mark.parametrize("method, path, status", [
+        ("GET", "/get-head-options/", 200),
+        ("HEAD", "/get-head-options/", 200),
+        ("OPTIONS", "/get-head-options/", 200),
+        ("GET", "/get/", 200),
+        ("HEAD", "/get/", 200),
+        ("POST", "/get-post/", 200),
+        ("POST", "/merge/", 200),
+        ("POST", "/merge/child/", 405),
+        ("TRACE", "/get-head-options/", 200),
+    ])
+    def test_core_010_003(self, env, method, path, status):
+        assert _request(env, method, path)[0] == status
+
+    # a method the resource handler refuses is answered as before
+    def test_core_010_004(self, env):
+        status, allow = _request(env, "PUT", "/get-post-options/")
+        assert status == 405
+        assert allow == {"GET", "HEAD", "POST", "OPTIONS", "TRACE"}
+
+    # without AllowMethods nothing changes
+    def test_core_010_005(self, env):
+        status, allow = _request(env, "PUT", "/plain/")
+        assert status == 405
+        assert allow == {"GET", "HEAD", "POST", "OPTIONS", "TRACE"}
+        assert _request(env, "FROBNICATE", "/plain/")[0] == 501
+
+
+class TestAllowMethodsAllowNoTrace:
+
+    @pytest.fixture(autouse=True, scope='class')
+    def _class_scope(self, env):
+        _conf(env, "off")
+
+    # TraceEnable decides about TRACE
+    def test_core_010_101(self, env):
+        status, allow = _request(env, "PUT", "/get-head-options/")
+        assert status == 405
+        assert allow == {"GET", "HEAD", "OPTIONS"}
+        assert _request(env, "TRACE", "/get-head-options/")[0] == 405