From 25aa46a6cac23e41bb28cd11d70647bbfd7916f1 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Tue, 18 Aug 2026 16:38:21 +0800 Subject: [PATCH] webapi: xml parsing reject documetns that are not well formed Hooks into the xml5ever parse_error callback to capture parse errors and reject malformed XML. Because of this stricter error handling, we need to do some input pre-processing for edge cases (thank you WPT). We need to strip out ]> which comes form Illustrator SVG export since that now causes errors (but should be ignored). Finally, by default xml5ever closes any opened tags at the end of the stream. Per WPT, these should be invalid. The rust work was all Claude-driven. --- src/browser/frame/parse.zig | 2 +- src/browser/parser/Parser.zig | 31 +++++++- src/browser/tests/domparser.html | 93 ++++++++++++++++++++++++ src/browser/webapi/DOMParser.zig | 44 ++++++++---- src/html5ever/lib.rs | 120 ++++++++++++++++++++++++++++++- 5 files changed, 270 insertions(+), 20 deletions(-) diff --git a/src/browser/frame/parse.zig b/src/browser/frame/parse.zig index ea00bdece..109114216 100644 --- a/src/browser/frame/parse.zig +++ b/src/browser/frame/parse.zig @@ -117,7 +117,7 @@ pub fn xmlDocument(frame: *Frame, xml: []const u8) !?*Document.XMLDocument { return error.ExecutionTerminated; } - if (parser.err != null or doc_node.firstChild() == null) { + if (parser.err != null or parser.xml_error or doc_node.firstChild() == null) { return null; } diff --git a/src/browser/parser/Parser.zig b/src/browser/parser/Parser.zig index f735916ad..903636adb 100644 --- a/src/browser/parser/Parser.zig +++ b/src/browser/parser/Parser.zig @@ -77,6 +77,7 @@ buf: std.ArrayList(u8), // innerHTML and DOMParser (per spec). Set from Options at init. allow_declarative_shadow: bool = false, +xml_error: bool = false, terminated: bool = false, appends_until_terminate_check: u16 = TERMINATE_CHECK_INTERVAL, @@ -253,7 +254,7 @@ pub fn parseXML(self: *Parser, xml: []const u8) void { createXMLElementCallback, getDataCallback, appendCallback, - parseErrorCallback, + xmlParseErrorCallback, popCallback, createCommentCallback, createProcessingInstruction, @@ -430,6 +431,21 @@ fn parseErrorCallback(ctx: *anyopaque, err: h5e.StringSlice) callconv(.c) void { // std.debug.print("PEC: {s}\n", .{err.slice()}); } +// xml5ever xml error callback indicating a well-formedness violating. +fn xmlParseErrorCallback(ctx: *anyopaque, err: h5e.StringSlice) callconv(.c) void { + const self: *Parser = @ptrCast(@alignCast(ctx)); + const msg = err.slice(); + if (std.mem.startsWith(u8, msg, "Bad character") and msg.len > "Bad character".len) { + // discouraged by legal code point + return; + } + if (std.mem.startsWith(u8, msg, "Invalid character reference")) { + // xml5ever doesn't know about this entity, but it could be valid + return; + } + self.xml_error = true; +} + fn popCallback(ctx: *anyopaque, node_ref: *anyopaque) callconv(.c) void { const self: *Parser = @ptrCast(@alignCast(ctx)); if (self.terminated) { @@ -488,10 +504,21 @@ fn _createElementCallbackWithDefaultnamespace(ctx: *anyopaque, data: *anyopaque, } fn _createElementCallback(self: *Parser, data: *anyopaque, qname: h5e.QualName, attributes: h5e.AttributeIterator, default_namespace: Element.Namespace) !*anyopaque { const frame = self.frame; - const name = qname.local.slice(); + const local = qname.local.slice(); + // Elements are keyed by qualified name (prefix/localName split on ':'), + // like createElementNS. html5ever never sets a prefix; xml5ever does. + const name = if (qname.prefix.unwrap()) |prefix| blk: { + if (prefix.len == 0) break :blk local; + break :blk try std.fmt.allocPrint(frame.local_arena, "{s}:{s}", .{ prefix.slice(), local }); + } else local; const namespace_string = qname.ns.slice(); const namespace = if (namespace_string.len == 0) default_namespace else Element.Namespace.parse(namespace_string); const node = try Frame.node_factory.createElementNS(frame, namespace, name, attributes); + if (namespace == .unknown and namespace_string.len > 0) { + // Same as Document.createElementNS: keep the URI so namespaceURI and + // lookupNamespaceURI can return it. + try frame._element_namespace_uris.put(frame.arena, node.as(Element), try frame.dupeString(namespace_string)); + } const pn = try self.arena.create(ParsedNode); pn.* = .{ diff --git a/src/browser/tests/domparser.html b/src/browser/tests/domparser.html index 9bb5ccde0..40e05d3ac 100644 --- a/src/browser/tests/domparser.html +++ b/src/browser/tests/domparser.html @@ -442,3 +442,96 @@ testing.expectEqual(true, r.hasAttributeNS('http://www.w3.org/XML/1998/namespace', 'lang')); } + + + + + + diff --git a/src/browser/webapi/DOMParser.zig b/src/browser/webapi/DOMParser.zig index bd1babfb1..00729d6b5 100644 --- a/src/browser/webapi/DOMParser.zig +++ b/src/browser/webapi/DOMParser.zig @@ -23,6 +23,7 @@ const js = @import("../js/js.zig"); const Frame = @import("../Frame.zig"); const Parser = @import("../parser/Parser.zig"); +const Node = @import("Node.zig"); const HTMLDocument = @import("HTMLDocument.zig"); const Document = @import("Document.zig"); @@ -41,15 +42,9 @@ pub fn parseFromString( mime_type: []const u8, frame: *Frame, ) !*Document { - const target_mime = std.meta.stringToEnum(enum { - @"text/html", - @"text/xml", - @"application/xml", - @"application/xhtml+xml", - @"image/svg+xml", - }, mime_type) orelse return error.NotSupported; + const target_mime = std.meta.stringToEnum(SupportedType, mime_type) orelse return error.NotSupported; - return switch (target_mime) { + switch (target_mime) { .@"text/html" => { const arena = try frame.getArena(.medium, "DOMParser.parseFromString"); defer arena.release(); @@ -89,13 +84,34 @@ pub fn parseFromString( return doc.asDocument(); }, else => { - const doc = (try Frame.parse.xmlDocument(frame, html)) orelse blk: { - // Return a document with a element per spec. - break :blk (try Frame.parse.xmlDocument(frame, "error")).?; - }; - return doc.asDocument(); + const xml_doc = (try Frame.parse.xmlDocument(frame, html)) orelse try parserErrorDocument(frame); + const doc = xml_doc.asDocument(); + doc._content_type = @tagName(target_mime); + return doc; }, - }; + } +} + +const SupportedType = enum { + @"text/html", + @"text/xml", + @"application/xml", + @"application/xhtml+xml", + @"image/svg+xml", +}; + +const parsererror_ns = "http://www.mozilla.org/newlayout/xml/parsererror.xml"; + +// Per spec, a well-formedness error yields a document whose only child is +// in the Mozilla error namespace. +fn parserErrorDocument(frame: *Frame) !*Document.XMLDocument { + const doc = try frame._factory.document(Document.XMLDocument{ ._proto = undefined }); + const root = try Frame.node_factory.createElementNS(frame, .unknown, "parsererror", null); + try frame._element_namespace_uris.put(frame.arena, root.as(Node.Element), parsererror_ns); + const text = try Frame.node_factory.createTextNode(frame, "error"); + _ = try root.appendChild(text, frame); + _ = try doc.asNode().appendChild(root, frame); + return doc; } pub const JsApi = struct { diff --git a/src/html5ever/lib.rs b/src/html5ever/lib.rs index 4b1e23cec..d69ffe7c3 100644 --- a/src/html5ever/lib.rs +++ b/src/html5ever/lib.rs @@ -807,7 +807,121 @@ pub extern "C" fn xml5ever_parse_document( }; let bytes = unsafe { std::slice::from_raw_parts(xml, len) }; - xml5ever::driver::parse_document(sink, xml5ever::driver::XmlParseOpts::default()) - .from_utf8() - .one(bytes); + let bytes = strip_doctype_internal_subset(bytes); + let tb = xml5ever::tree_builder::XmlTreeBuilder::new(sink, Default::default()); + let tokenizer = xml5ever::tokenizer::XmlTokenizer::new( + UnclosedTagSink { tb, depth: Cell::new(0) }, + Default::default(), + ); + html5ever::tendril::stream::Utf8LossyDecoder::new(XmlDocumentParser { + tokenizer, + input_buffer: Default::default(), + }) + .one(&*bytes); +} + +// xml5ever's tokenizer has no notion of a DOCTYPE internal subset +// (` ]>`, as old Illustrator exports +// emit): the `[` ends the doctype as bogus and the declarations tokenize as +// junk before the root, each step a parse error. Nothing in the subset is used +// anyway (no DTD support), so cut it out before tokenizing. Quotes are +// respected; nested `[`/`]` are not (they don't occur in the subset syntax). +fn strip_doctype_internal_subset(bytes: &[u8]) -> std::borrow::Cow<'_, [u8]> { + let Some(doctype) = bytes.windows(9).position(|w| w == b" = None; + let mut open = None; + while i < bytes.len() { + let c = bytes[i]; + match quote { + Some(q) => { + if c == q { + quote = None + } + }, + None => match c { + b'"' | b'\'' => quote = Some(c), + b'>' if open.is_none() => return std::borrow::Cow::Borrowed(bytes), + b'[' if open.is_none() => open = Some(i), + b']' if open.is_some() => { + let mut out = Vec::with_capacity(bytes.len()); + out.extend_from_slice(&bytes[..open.unwrap()]); + out.extend_from_slice(&bytes[i + 1..]); + return std::borrow::Cow::Owned(out); + }, + _ => {}, + }, + } + i += 1; + } + std::borrow::Cow::Borrowed(bytes) +} + +// xml5ever's tree builder silently closes elements still open at EOF, and +// its tokenizer reports nothing either, so a truncated document +// (`text`) parses "cleanly". Browsers reject it. This sits between +// the tokenizer and the tree builder, tracks tag nesting from the raw token +// stream, and reports an error when EOF arrives with tags still open. +struct UnclosedTagSink<'arena> { + tb: xml5ever::tree_builder::XmlTreeBuilder>, + depth: Cell, +} + +impl<'arena> xml5ever::tokenizer::TokenSink for UnclosedTagSink<'arena> { + type Handle = Ref; + + fn process_token( + &self, + token: xml5ever::tokenizer::Token, + ) -> xml5ever::tokenizer::ProcessResult { + use xml5ever::tokenizer::{TagKind, Token}; + match &token { + Token::Tag(tag) => match tag.kind { + TagKind::StartTag => self.depth.set(self.depth.get() + 1), + TagKind::EndTag | TagKind::ShortTag => { + self.depth.set(self.depth.get().saturating_sub(1)) + }, + TagKind::EmptyTag => {}, + }, + Token::EndOfFile => { + if self.depth.get() > 0 { + use xml5ever::tree_builder::TreeSink; + self.tb.sink.parse_error(std::borrow::Cow::Borrowed("Unclosed element at EOF")); + } + }, + _ => {}, + } + self.tb.process_token(token) + } + + fn end(&self) { + self.tb.end() + } +} + +// xml5ever::driver::XmlParser, minus the tree-builder-typed tokenizer so the +// UnclosedTagSink can sit in between. +struct XmlDocumentParser<'arena> { + tokenizer: xml5ever::tokenizer::XmlTokenizer>, + input_buffer: xml5ever::buffer_queue::BufferQueue, +} + +impl<'arena> TendrilSink for XmlDocumentParser<'arena> { + type Output = (); + + fn process(&mut self, t: StrTendril) { + self.input_buffer.push_back(t); + while let xml5ever::TokenizerResult::Script(_) = self.tokenizer.feed(&self.input_buffer) {} + } + + fn error(&mut self, desc: std::borrow::Cow<'static, str>) { + use xml5ever::tree_builder::TreeSink; + self.tokenizer.sink.tb.sink.parse_error(desc) + } + + fn finish(self) -> () { + self.tokenizer.end(); + } }