mod_mime_magic whitespace/"with escape" fixes

changes-entries/mime_magic_rsl.txt

@@ -0,0 +1,3 @@
+  *) mod_mime_magic: Fix the content type and encoding derived from a
+     magic file entry being terminated with trailing whitespace.  Ignore
+     a content encoding which is not a valid token. [Joe Orton]

docs/log-message-tags/next-number

@@ -1 +1 @@
-10622
+10624

modules/metadata/mod_mime_magic.c

@@ -650,24 +650,17 @@ static char *rsl_strdup(request_rec *r, int start_frag, int start_pos, int len)
     /* loop through and collect the string */
     res_pos = 0;
     for (frag = req_dat->head, cur_frag = 0;
-         frag->next;
+         frag->next && res_pos < len;
          frag = frag->next, cur_frag++) {
         /* loop to the first fragment */
         if (cur_frag < start_frag)
             continue;
 
         /* loop through and collect chars */
         for (cur_pos = (cur_frag == start_frag) ? start_pos : 0;
-             frag->str[cur_pos];
+             frag->str[cur_pos] && res_pos < len;
              cur_pos++) {
-            if (cur_frag >= start_frag
-                && cur_pos >= start_pos
-                && res_pos <= len) {
-                result[res_pos++] = frag->str[cur_pos];
-                if (res_pos > len) {
-                    break;
-                }
-            }
+            result[res_pos++] = frag->str[cur_pos];
         }
     }
 
@@ -698,6 +691,7 @@ static int magic_rsl_to_request(request_rec *r)
         encoding_len;     /* content encoding length */
 
     char *tmp;
+    const char *p, *q;
     magic_rsl *frag;      /* list-traversal pointer */
     rsl_states state;
 
@@ -810,6 +804,14 @@ static int magic_rsl_to_request(request_rec *r)
 
     /* save the info in the request record */
     tmp = rsl_strdup(r, type_frag, type_pos, type_len);
+    /* the type must be of the form token "/" token */
+    p = ap_scan_http_token(tmp);
+    if (p == tmp || *p != '/'
+        || (q = ap_scan_http_token(p + 1)) == p + 1 || *q != '\0') {
+        ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(10622)
+                      "ignoring invalid content type '%s'", tmp);
+        return DECLINED;
+    }
     /* XXX: this could be done at config time I'm sure... but I'm
      * confused by all this magic_rsl stuff. -djg */
     ap_content_type_tolower(tmp);
@@ -818,10 +820,20 @@ static int magic_rsl_to_request(request_rec *r)
     if (state == rsl_encoding) {
         tmp = rsl_strdup(r, encoding_frag,
                                          encoding_pos, encoding_len);
-        /* XXX: this could be done at config time I'm sure... but I'm
-         * confused by all this magic_rsl stuff. -djg */
-        ap_str_tolower(tmp);
-        r->content_encoding = tmp;
+        /* the encoding must be a token; anything else following the
+         * type is a descriptive note and is ignored */
+        if (*ap_scan_http_token(tmp) != '\0') {
+            ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(10623)
+                          "ignoring invalid content encoding '%s'",
+                          tmp);
+            state = rsl_separator;
+        }
+        else {
+            /* XXX: this could be done at config time I'm sure... but I'm
+             * confused by all this magic_rsl stuff. -djg */
+            ap_str_tolower(tmp);
+            r->content_encoding = tmp;
+        }
     }
 
     /* detect memory allocation or other errors */
@@ -1998,7 +2010,6 @@ static int mcheck(request_rec *r, union VALUETYPE *p, struct magic *m)
 
 static int ascmagic(request_rec *r, unsigned char *buf, apr_size_t nbytes)
 {
-    int has_escapes = 0;
     unsigned char *s;
     char nbuf[SMALL_HOWMANY + 1];  /* one extra for terminating '\0' */
     char *token;
@@ -2036,14 +2047,11 @@ static int ascmagic(request_rec *r, unsigned char *buf, apr_size_t nbytes)
     /* make a copy of the buffer here because apr_strtok() will destroy it */
     s = (unsigned char *) memcpy(nbuf, buf, small_nbytes);
     s[small_nbytes] = '\0';
-    has_escapes = (memchr(s, '\033', small_nbytes) != NULL);
     while ((token = apr_strtok((char *) s, " \t\n\r\f", &strtok_state)) != NULL) {
         s = NULL;  /* make apr_strtok() keep on tokin' */
         for (p = names; p < names + NNAMES; p++) {
             if (STREQ(p->name, token)) {
                 magic_rsl_puts(r, types[p->type]);
-                if (has_escapes)
-                    magic_rsl_puts(r, " (with escape sequences)");
                 return 1;
             }
         }

test/modules/metadata/conftest.py

@@ -0,0 +1,35 @@
+import logging
+import os
+
+import pytest
+import sys
+
+from .env import MetadataTestEnv
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '../..'))
+
+
+def pytest_report_header(config, start_path):
+    env = MetadataTestEnv()
+    return f"metadata [apache: {env.get_httpd_version()}, mpm: {env.mpm_module}, {env.prefix}]"
+
+
+@pytest.fixture(scope="package")
+def env(pytestconfig) -> MetadataTestEnv:
+    level = logging.INFO
+    console = logging.StreamHandler()
+    console.setLevel(level)
+    console.setFormatter(logging.Formatter('%(levelname)s: %(message)s'))
+    logging.getLogger('').addHandler(console)
+    logging.getLogger('').setLevel(level=level)
+    env = MetadataTestEnv(pytestconfig=pytestconfig)
+    env.setup_httpd()
+    env.apache_access_log_clear()
+    env.httpd_error_log.clear_log()
+    return env
+
+
+@pytest.fixture(autouse=True, scope="package")
+def _stop_package_scope(env):
+    yield
+    assert env.apache_stop() == 0

test/modules/metadata/env.py

@@ -0,0 +1,25 @@
+import inspect
+import logging
+import os
+
+from pyhttpd.env import HttpdTestEnv, HttpdTestSetup
+
+log = logging.getLogger(__name__)
+
+
+class MetadataTestSetup(HttpdTestSetup):
+
+    def __init__(self, env: 'HttpdTestEnv'):
+        super().__init__(env=env)
+        self.add_source_dir(os.path.dirname(inspect.getfile(MetadataTestSetup)))
+        self.add_modules(["mime", "mime_magic"])
+
+
+class MetadataTestEnv(HttpdTestEnv):
+
+    def __init__(self, pytestconfig=None):
+        super().__init__(pytestconfig=pytestconfig)
+        self.add_httpd_log_modules(["mime_magic", "core"])
+
+    def setup_httpd(self, setup: HttpdTestSetup = None):
+        super().setup_httpd(setup=MetadataTestSetup(env=self))

test/modules/metadata/test_001_mime_magic.py

@@ -0,0 +1,83 @@
+import os
+import re
+import pytest
+
+from pyhttpd.conf import HttpdConf
+
+
+class TestMimeMagic:
+
+    @pytest.fixture(autouse=True, scope='class')
+    def _class_scope(self, env):
+        # A small magic file: one entry giving both a type and an
+        # encoding, and one with a file(1)-style descriptive note in
+        # place of the encoding.  The token-based text detection is left
+        # to handle everything else.
+        magic_file = os.path.join(env.gen_dir, "magic")
+        with open(magic_file, "w") as f:
+            f.write("0\tstring\tTESTMAGIC\tapplication/x-test-magic\tx-test-encoding\n")
+            f.write("0\tstring\tTESTNOTE\tapplication/x-test-note (some note)\n")
+
+        # Files without an extension, so mod_mime sets no type and
+        # mod_mime_magic has to derive one from the content.
+        doc_dir = os.path.join(env.server_dir, "htdocs", "test1", "magic")
+        os.makedirs(doc_dir, exist_ok=True)
+        # (a rule is only tested on files of at least 64 bytes)
+        with open(os.path.join(doc_dir, "softmagic"), "wb") as f:
+            f.write(b"TESTMAGIC" + b" and some more content" * 4 + b"\n")
+        with open(os.path.join(doc_dir, "softnote"), "wb") as f:
+            f.write(b"TESTNOTE" + b" and some more content" * 4 + b"\n")
+        with open(os.path.join(doc_dir, "html-plain"), "wb") as f:
+            f.write(b"<html>\n<body>hello</body>\n")
+        # HTML token followed by an ESC byte.
+        with open(os.path.join(doc_dir, "html-escape"), "wb") as f:
+            f.write(b"<html>\n\x1b[1mhello\x1b[0m\n")
+
+        conf = HttpdConf(env, extras={
+            'base': f"""
+            MimeMagicFile "{magic_file}"
+            """,
+        })
+        conf.add_vhost_test1()
+        conf.install()
+        assert env.apache_restart() == 0
+
+    # type and encoding from a magic file entry
+    def test_metadata_001_01_softmagic(self, env):
+        url = env.mkurl("http", "test1", "/magic/softmagic")
+        r = env.curl_get(url)
+        assert r.response, "no response: server may have crashed"
+        assert r.response["status"] == 200
+        assert r.response["header"]["content-type"] == "application/x-test-magic"
+        assert r.response["header"]["content-encoding"] == "x-test-encoding"
+
+    # type from the HTML token, no encoding
+    def test_metadata_001_02_html(self, env):
+        url = env.mkurl("http", "test1", "/magic/html-plain")
+        r = env.curl_get(url)
+        assert r.response, "no response: server may have crashed"
+        assert r.response["status"] == 200
+        assert r.response["header"]["content-type"] == "text/html"
+        assert "content-encoding" not in r.response["header"]
+
+    # type from the HTML token, escape sequences in the content
+    def test_metadata_001_03_html_escapes(self, env):
+        url = env.mkurl("http", "test1", "/magic/html-escape")
+        r = env.curl_get(url)
+        assert r.response, "no response: server may have crashed"
+        assert r.response["status"] == 200
+        assert r.response["header"]["content-type"] == "text/html"
+        assert "content-encoding" not in r.response["header"]
+
+    # type from a magic file entry followed by a descriptive note: the
+    # type must end at the whitespace, and the note is not an encoding
+    def test_metadata_001_04_softmagic_note(self, env):
+        url = env.mkurl("http", "test1", "/magic/softnote")
+        r = env.curl_get(url)
+        assert r.response, "no response: server may have crashed"
+        assert r.response["status"] == 200
+        assert r.response["header"]["content-type"] == "application/x-test-note"
+        assert "content-encoding" not in r.response["header"]
+        assert env.httpd_error_log.scan_recent(
+            re.compile(r'.*AH10623: .*ignoring invalid content encoding.*'))
+        env.httpd_error_log.ignore_recent(lognos=["AH10623"])