fix(rng): reject resource references without href

docs/changelog/1199.bugfix.rst

@@ -0,0 +1 @@
+Reject RELAX NG include and externalRef patterns without href attributes, including unused definitions.

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 <ref> 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;
         }
     }

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'<incorrect><element xmlns="{_RNG}" name="foo"><externalRef/></element></incorrect>',
+            [("turbohtml", "compilation", False, False), ("libxml2", "compilation", False, False)],
+            id="missing-external-href",
+        ),
+        pytest.param(
+            f'<incorrect><grammar xmlns="{_RNG}"><include/><start><element name="foo">'
+            "<empty/></element></start></grammar></incorrect>",
+            [("turbohtml", "compilation", False, False), ("libxml2", "compilation", False, False)],
+            id="missing-include-href",
+        ),
         pytest.param(
             f'<correct><grammar xmlns="{_RNG}"/></correct><valid><root/></valid>',
             [
@@ -90,6 +101,8 @@ def test_rng_labels_instance_names_are_not_resource_metadata(engines: tuple[Modu
         pytest.param('<dir name="other"/>', id="directory"),
         pytest.param(f'<correct><externalRef xmlns="{_RNG}" href="other.rng"/></correct>', id="external-reference"),
         pytest.param(f'<correct><include xmlns="{_RNG}" href="other.rng"/></correct>', id="include"),
+        pytest.param(f'<incorrect><externalRef xmlns="{_RNG}" href=""/></incorrect>', id="empty-external-href"),
+        pytest.param(f'<incorrect><include xmlns="{_RNG}" href=""/></incorrect>', id="empty-include-href"),
         pytest.param(f'<correct xml:base="other/"><empty xmlns="{_RNG}"/></correct>', id="base-uri"),
     ],
 )

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'<externalRef xmlns="{_RNG}"/>', id="short-external"),
+        pytest.param(f'<include xmlns="{_RNG}"/>', id="short-include"),
+        pytest.param(f'<element xmlns="{_RNG}" name="foo"><externalRef/></element>', id="element-external"),
+        pytest.param(
+            f'<grammar xmlns="{_RNG}"><start><element name="foo"><externalRef/></element></start></grammar>',
+            id="grammar-external",
+        ),
+        pytest.param(
+            f'<grammar xmlns="{_RNG}"><include/><start><element name="foo"><empty/></element></start></grammar>',
+            id="grammar-include",
+        ),
+        pytest.param(
+            f'<rng:grammar xmlns:rng="{_RNG}"><rng:include/><rng:start><rng:element name="foo">'
+            "<rng:empty/></rng:element></rng:start></rng:grammar>",
+            id="prefixed-include",
+        ),
+        pytest.param(
+            f'<rng:element xmlns:rng="{_RNG}" name="foo"><rng:externalRef/></rng:element>',
+            id="prefixed-external",
+        ),
+        pytest.param(
+            f'<element xmlns="{_RNG}" xmlns:doc="urn:annotation" name="foo"><externalRef doc:href="x"/></element>',
+            id="foreign-href-attribute",
+        ),
+        pytest.param(
+            f'<grammar xmlns="{_RNG}"><start><element name="foo"><empty/></element></start>'
+            '<define name="unused"><externalRef/></define></grammar>',
+            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("<doc:include/>", id="foreign-include"),
+        pytest.param("<doc:externalRef/>", id="foreign-external"),
+        pytest.param("<doc:annotation><externalRef/></doc:annotation>", id="foreign-subtree"),
+    ],
+)
+def test_relaxng_missing_href_ignores_foreign_annotations(annotation: str) -> None:
+    schema: Final = (
+        f'<grammar xmlns="{_RNG}" xmlns:doc="urn:annotation">\n<!-- annotation -->'
+        f'{annotation}<start><element name="foo"><empty/></element></start></grammar>'
+    )
+    validator: Final = RelaxNG(schema)
+    assert [validator.validate(parse_xml(document)).valid for document in ("<foo/>", "<wrong/>")] == [True, False]
+
+
+def test_relaxng_missing_href_preserves_ordinary_patterns() -> None:
+    validator: Final = RelaxNG(f'<element xmlns="{_RNG}" name="foo"><text/></element>')
+    assert [validator.validate(parse_xml(document)).valid for document in ("<foo>text</foo>", "<wrong/>")] == [
+        True,
+        False,
+    ]

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()

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 = []