diff --git a/docs/changelog/1199.bugfix.rst b/docs/changelog/1199.bugfix.rst new file mode 100644 index 00000000..42e9b111 --- /dev/null +++ b/docs/changelog/1199.bugfix.rst @@ -0,0 +1 @@ +Reject RELAX NG include and externalRef patterns without href attributes, including unused definitions. diff --git a/src/turbohtml/_c/validate/relaxng.h b/src/turbohtml/_c/validate/relaxng.h index c6f8ba08..646c2e89 100644 --- a/src/turbohtml/_c/validate/relaxng.h +++ b/src/turbohtml/_c/validate/relaxng.h @@ -561,6 +561,7 @@ static int rng_datatype_id(th_schema *schema, th_node *node, int default_datatyp } static pattern *rng_build(th_schema *schema, th_node *node); +static void rng_check_href(th_schema *schema, th_node *node); static int rng_check_interleave_node(th_schema *schema, th_node *interleave); /* Group the pattern children of a container into a single pattern (Empty when none). */ @@ -712,9 +713,25 @@ static pattern *rng_build(th_schema *schema, th_node *node) { PyErr_SetString(PyExc_ValueError, "RELAX NG has no matching define"); return schema->p_notallowed; } + rng_check_href(schema, node); return schema->p_notallowed; } +static void rng_check_href(th_schema *schema, th_node *node) { + if (node->type != TH_NODE_ELEMENT) { + return; + } + const Py_UCS4 *local, *prefix; + Py_ssize_t local_len = 0, prefix_len = 0; + split_prefix(node->text, node->text_len, &local, &local_len, &prefix, &prefix_len); + if (!u_eq_ascii(local, local_len, "externalRef") && !u_eq_ascii(local, local_len, "include")) { + return; + } + if (attr_exact(schema->tree, node, "href", 4) == NULL) { + PyErr_SetString(PyExc_ValueError, "RELAX NG resource reference is missing the required href attribute"); + } +} + static pattern *rng_resolve(th_schema *schema, int def_index) { def_entry *entry = &schema->defines.items[def_index]; if (entry->built != NULL) { @@ -1135,6 +1152,11 @@ static int rng_compile(th_schema *schema) { PyErr_NoMemory(); /* GCOVR_EXCL_LINE */ return 0; /* GCOVR_EXCL_LINE */ } + } else { + rng_check_href(schema, child); + if (PyErr_Occurred()) { + return 0; + } } } th_node *start = first_schema_child(schema, schema->root, RNG_NS, "start"); @@ -1163,7 +1185,7 @@ static int rng_compile(th_schema *schema) { } } schema->start = rng_build_children(schema, start, NULL); - return 1; + return PyErr_Occurred() ? 0 : 1; } /* Section 4.1 removes annotation subtrees before pattern and name-class construction. */ @@ -1200,7 +1222,8 @@ static int rng_check_unused_refs(th_schema *schema, th_node *container) { return -1; } } - if (rng_check_unused_refs(schema, child) < 0) { + rng_check_href(schema, child); + if (PyErr_Occurred() || rng_check_unused_refs(schema, child) < 0) { return -1; } } diff --git a/tests/test_fuzz_rng_labels.py b/tests/test_fuzz_rng_labels.py index 807eb20d..ece7c69e 100644 --- a/tests/test_fuzz_rng_labels.py +++ b/tests/test_fuzz_rng_labels.py @@ -42,6 +42,17 @@ def engines() -> tuple[ModuleType, ModuleType]: [("turbohtml", "compilation", False, False), ("libxml2", "compilation", False, False)], id="incorrect-schema", ), + pytest.param( + f'', + [("turbohtml", "compilation", False, False), ("libxml2", "compilation", False, False)], + id="missing-external-href", + ), + pytest.param( + f'' + "", + [("turbohtml", "compilation", False, False), ("libxml2", "compilation", False, False)], + id="missing-include-href", + ), pytest.param( f'', [ @@ -90,6 +101,8 @@ def test_rng_labels_instance_names_are_not_resource_metadata(engines: tuple[Modu pytest.param('', id="directory"), pytest.param(f'', id="external-reference"), pytest.param(f'', id="include"), + pytest.param(f'', id="empty-external-href"), + pytest.param(f'', id="empty-include-href"), pytest.param(f'', id="base-uri"), ], ) diff --git a/tests/validate/test_relaxng_missing_href.py b/tests/validate/test_relaxng_missing_href.py new file mode 100644 index 00000000..5670972e --- /dev/null +++ b/tests/validate/test_relaxng_missing_href.py @@ -0,0 +1,77 @@ +from __future__ import annotations + +from typing import Final + +import pytest + +from turbohtml import parse_xml +from turbohtml.validate import RelaxNG + +_RNG: Final = "http://relaxng.org/ns/structure/1.0" + + +@pytest.mark.parametrize("as_node", [False, True], ids=["text", "node"]) +@pytest.mark.parametrize( + "schema", + [ + pytest.param(f'', id="short-external"), + pytest.param(f'', id="short-include"), + pytest.param(f'', id="element-external"), + pytest.param( + f'', + id="grammar-external", + ), + pytest.param( + f'', + id="grammar-include", + ), + pytest.param( + f'' + "", + id="prefixed-include", + ), + pytest.param( + f'', + id="prefixed-external", + ), + pytest.param( + f'', + id="foreign-href-attribute", + ), + pytest.param( + f'' + '', + id="unused-definition-external", + ), + ], +) +def test_relaxng_missing_href_rejects_compilation(schema: str, *, as_node: bool) -> None: + source: Final = parse_xml(schema) if as_node else schema + for _ in range(2): + with pytest.raises(ValueError, match="required href attribute"): + RelaxNG(source) + + +@pytest.mark.parametrize( + "annotation", + [ + pytest.param("", id="foreign-include"), + pytest.param("", id="foreign-external"), + pytest.param("", id="foreign-subtree"), + ], +) +def test_relaxng_missing_href_ignores_foreign_annotations(annotation: str) -> None: + schema: Final = ( + f'\n' + f'{annotation}' + ) + validator: Final = RelaxNG(schema) + assert [validator.validate(parse_xml(document)).valid for document in ("", "")] == [True, False] + + +def test_relaxng_missing_href_preserves_ordinary_patterns() -> None: + validator: Final = RelaxNG(f'') + assert [validator.validate(parse_xml(document)).valid for document in ("text", "")] == [ + True, + False, + ] diff --git a/tools/fuzz/rng_labels.py b/tools/fuzz/rng_labels.py index 21befef0..7b40dee8 100644 --- a/tools/fuzz/rng_labels.py +++ b/tools/fuzz/rng_labels.py @@ -114,7 +114,8 @@ def _cases(suite: lxml.etree._Element) -> Iterator[lxml.etree._Element]: def _requires_resources(case: lxml.etree._Element) -> bool: return any(child.tag in {"resource", "dir"} for child in case) or any( - element.tag in {f"{{{_RNG}}}externalRef", f"{{{_RNG}}}include"} or _XML_BASE in element.attrib + (element.tag in {f"{{{_RNG}}}externalRef", f"{{{_RNG}}}include"} and "href" in element.attrib) + or _XML_BASE in element.attrib for label in case if label.tag in {"correct", "incorrect"} for element in label.iter() diff --git a/tox.toml b/tox.toml index 2f27e5de..ce737d00 100644 --- a/tox.toml +++ b/tox.toml @@ -280,6 +280,7 @@ deps = [ "meson-python>=0.22.1", "ninja>=1.13.2", "pytest>=9.1.1", + "tinycss2>=1.5.1", "typing-extensions>=4.16", ] dependency_groups = []