diff --git a/changes-entries/mime_magic_rsl.txt b/changes-entries/mime_magic_rsl.txt new file mode 100644 index 00000000000..c7ef892649d --- /dev/null +++ b/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] diff --git a/docs/log-message-tags/next-number b/docs/log-message-tags/next-number index b0267f86665..5c74e1d8650 100644 --- a/docs/log-message-tags/next-number +++ b/docs/log-message-tags/next-number @@ -1 +1 @@ -10622 +10624 diff --git a/modules/metadata/mod_mime_magic.c b/modules/metadata/mod_mime_magic.c index 799d3fc5631..c15d11b7ff7 100644 --- a/modules/metadata/mod_mime_magic.c +++ b/modules/metadata/mod_mime_magic.c @@ -650,7 +650,7 @@ 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) @@ -658,16 +658,9 @@ static char *rsl_strdup(request_rec *r, int start_frag, int start_pos, int len) /* 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; } } diff --git a/test/modules/metadata/__init__.py b/test/modules/metadata/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/modules/metadata/conftest.py b/test/modules/metadata/conftest.py new file mode 100644 index 00000000000..a90f150bf37 --- /dev/null +++ b/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 diff --git a/test/modules/metadata/env.py b/test/modules/metadata/env.py new file mode 100644 index 00000000000..038d478968a --- /dev/null +++ b/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)) diff --git a/test/modules/metadata/test_001_mime_magic.py b/test/modules/metadata/test_001_mime_magic.py new file mode 100644 index 00000000000..29b3acc59f3 --- /dev/null +++ b/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"\nhello\n") + # HTML token followed by an ESC byte. + with open(os.path.join(doc_dir, "html-escape"), "wb") as f: + f.write(b"\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"])