diff --git a/scripts/lib/svg-active-content.mjs b/scripts/lib/svg-active-content.mjs
index 8a6512d3..41c55204 100644
--- a/scripts/lib/svg-active-content.mjs
+++ b/scripts/lib/svg-active-content.mjs
@@ -77,13 +77,29 @@ const ATTRIBUTE_PATTERN =
* `]*>` cannot express this: the first `>` inside an internal
* subset closes an `` declaration, not the DOCTYPE, so that
* pattern deletes the declaration and leaves a bare `]>` behind -- in a file
- * the caller then publishes as well-formed XML. The subset alternative carries
- * its own closing `>` so that a `]` inside a quoted entity value cannot end
- * the subset early. A DOCTYPE that opens a subset and never closes it matches
- * neither alternative and is left in place, where hasDoctype() and the entity
- * check below still see it.
+ * the caller then publishes as well-formed XML.
+ *
+ * Neither can a pattern that treats `[` as the unambiguous start of the
+ * subset. Per the XML grammar a DOCTYPE is
+ * `''`, and an
+ * ExternalID carries quoted literals that may themselves contain `[`, `]` or
+ * `>`. `` is an external
+ * identifier with no subset at all, but an unquoted reading sees its `[` as a
+ * subset that never closes: the declaration is then left in place (so the
+ * strip silently does nothing), or -- where the document later holds any
+ * `]`-then-`>`, which every `` section provides -- everything
+ * between is deleted, destroying live markup that was never part of the
+ * declaration. Both outcomes survive the callers' post-strip rescans, because
+ * an unstripped external identifier and a truncated document are each inert.
+ *
+ * So each part is matched with the quote-aware alternation below, in which a
+ * `"` or `'` always consumes through its closing mate. A `]` inside a quoted
+ * entity value therefore cannot end the subset early either. A DOCTYPE that
+ * opens a subset and never closes it still matches nothing and is left in
+ * place, where hasDoctype() and the entity check below see it.
*/
-const DOCTYPE_PATTERN = /]*(?:\[[\s\S]*?\]\s*>|>)\s*/gi;
+const DOCTYPE_PATTERN =
+ /"']|"[^"]*"|'[^']*')*(?:\[(?:[^\]"']|"[^"]*"|'[^']*')*\]\s*)?>\s*/gi;
/** An entity declaration, which only appears inside an internal DTD subset. */
const ENTITY_DECLARATION = / {
assert.equal(stripDoctype(INERT), INERT);
});
+// An ExternalID carries quoted literals, and a quoted literal may hold any of
+// the characters that otherwise delimit the declaration. Reading `[` as the
+// unambiguous start of an internal subset therefore misreads a subset-less
+// declaration as one that never closes, and the strip silently does nothing.
+test('stripDoctype removes an external identifier containing a bracket', () => {
+ const svg = '\n' + INERT;
+ assert.equal(stripDoctype(svg), INERT);
+ assert.equal(hasDoctype(stripDoctype(svg)), false);
+});
+
+test('stripDoctype removes an external identifier containing "]>" or ">"', () => {
+ assert.equal(
+ stripDoctype(
+ 'y.dtd">\n' + INERT,
+ ),
+ INERT,
+ );
+ assert.equal(
+ stripDoctype(
+ 'y.dtd">\n' + INERT,
+ ),
+ INERT,
+ );
+});
+
+// The deletion that an unterminated subset reading produces is unbounded: it
+// runs to the next `]`-then-`>`, and every `` section supplies
+// one. The wreckage is inert, so it survives the callers' post-strip rescans
+// and is published as a well-formed-looking asset.
+test('stripDoctype does not swallow markup up to a later CDATA terminator', () => {
+ const body =
+ '';
+ const svg = '\n' + body;
+ assert.equal(stripDoctype(svg), body);
+});
+
+test('stripDoctype leaves an unclosed subset in place for the entity check', () => {
+ const svg = '\n' + INERT;
+ assert.equal(stripDoctype(svg), svg);
+ assert.equal(hasDoctype(svg), true);
+ assert.deepEqual(findActiveContent(svg), [
+ 'contains an entity declaration in an internal DTD subset ' +
+ '(the XML parser expands it, so its payload is not visible here)',
+ ]);
+});
+
+test('stripDoctype matches an unterminated literal in linear time', () => {
+ // The alternation branches are disjoint on their first character, so no
+ // input can make the engine explore them combinatorially.
+ const started = Date.now();
+ stripDoctype('` makes the browser fetch the target
// when the SVG is opened directly, and `type="text/xsl"` applies an XSLT
diff --git a/tests/validate-architecture-assets.test.mjs b/tests/validate-architecture-assets.test.mjs
index 27cafc7b..01ec4d8e 100644
--- a/tests/validate-architecture-assets.test.mjs
+++ b/tests/validate-architecture-assets.test.mjs
@@ -203,6 +203,30 @@ test('--fix removes a DOCTYPE declaration instead of reporting it', () => {
assert.match(result.stdout, /removed DOCTYPE/);
});
+test('--fix removes a DOCTYPE whose external identifier contains a bracket', () => {
+ const svg = `\n${VALID_SVG}`;
+ const result = runFix(svg);
+ assert.equal(result.status, 0, result.stderr);
+ assert.doesNotMatch(result.files[ASSET], /DOCTYPE/);
+ assert.equal(result.files[ASSET], VALID_SVG);
+ assert.match(result.stdout, /removed DOCTYPE/);
+});
+
+test('--fix reports a DOCTYPE it could not remove instead of claiming success', () => {
+ // An internal subset that never closes matches no DOCTYPE pattern, so the
+ // strip returns the source unchanged. Recording "removed DOCTYPE" for it
+ // would hand back a declaration the run claims to have deleted.
+ const svg = `\n${VALID_SVG}`;
+ const result = runFix(svg);
+ assert.notEqual(result.status, 0);
+ assert.equal(result.files[ASSET], svg);
+ assert.doesNotMatch(result.stdout, /removed DOCTYPE/);
+ assert.match(
+ result.stdout + result.stderr,
+ /DOCTYPE declaration that could not be removed automatically/,
+ );
+});
+
test('--fix adds a viewBox derived from width and height', () => {
const svg =
'';