Skip to content

Commit 680de2d

Browse files
committed
Implement custom isEqualNode
Netsurf's dom_node_is_equal appears to be both unsafe and incorrect. It's unsafe because various node types don't have the dom_node_get_attributes implementation so they default to setting the node attributes to null: https://github.com/lightpanda-io/libdom/blob/master/src/core/node.c#L658 However, it doesn't do a NULL check when comparing them, so it crashes: https://github.com/lightpanda-io/libdom/blob/da8b967905a38455e4f6b75cc9ad2767bae3ccde/src/core/namednodemap.c#L312 Furthermore, specific nodes need to be compared using specific attributes/values. This PR fixes a WPT crash.
1 parent 4a696b4 commit 680de2d

File tree

4 files changed

+116
-0
lines changed

4 files changed

+116
-0
lines changed

src/browser/dom/character_data.zig

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,21 @@ pub const CharacterData = struct {
9797
pub fn _substringData(self: *parser.CharacterData, offset: u32, count: u32) ![]const u8 {
9898
return try parser.characterDataSubstringData(self, offset, count);
9999
}
100+
101+
// netsurf's CharacterData (text, comment) doesn't implement the
102+
// dom_node_get_attributes and thus will crash if we try to call nodeIsEqualNode.
103+
pub fn _isEqualNode(self: *parser.CharacterData, other_node: *parser.Node) !bool {
104+
if (try parser.nodeType(@ptrCast(self)) != try parser.nodeType(other_node)) {
105+
return false;
106+
}
107+
108+
const other: *parser.CharacterData = @ptrCast(other_node);
109+
if (std.mem.eql(u8, try get_data(self), try get_data(other)) == false) {
110+
return false;
111+
}
112+
113+
return true;
114+
}
100115
};
101116

102117
// Tests
@@ -163,4 +178,9 @@ test "Browser.DOM.CharacterData" {
163178
.{ "cdata.substringData('OK'.length-1, 'replaced'.length) == 'replaced'", "true" },
164179
.{ "cdata.substringData('OK'.length-1, 0) == ''", "true" },
165180
}, .{});
181+
182+
try runner.testCases(&.{
183+
.{ "cdata.substringData('OK'.length-1, 'replaced'.length) == 'replaced'", "true" },
184+
.{ "cdata.substringData('OK'.length-1, 0) == ''", "true" },
185+
}, .{});
166186
}

src/browser/dom/document_fragment.zig

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@ pub const DocumentFragment = struct {
3434
parser.documentHTMLToDocument(state.document.?),
3535
);
3636
}
37+
38+
pub fn _isEqualNode(self: *parser.DocumentFragment, other_node: *parser.Node) !bool {
39+
const other_type = try parser.nodeType(other_node);
40+
if (other_type != .document_fragment) {
41+
return false;
42+
}
43+
_ = self;
44+
return true;
45+
}
3746
};
3847

3948
const testing = @import("../../testing.zig");
@@ -45,4 +54,11 @@ test "Browser.DOM.DocumentFragment" {
4554
.{ "const dc = new DocumentFragment()", "undefined" },
4655
.{ "dc.constructor.name", "DocumentFragment" },
4756
}, .{});
57+
58+
try runner.testCases(&.{
59+
.{ "const dc1 = new DocumentFragment()", "undefined" },
60+
.{ "const dc2 = new DocumentFragment()", "undefined" },
61+
.{ "dc1.isEqualNode(dc1)", "true" },
62+
.{ "dc1.isEqualNode(dc2)", "true" },
63+
}, .{});
4864
}

src/browser/dom/document_type.zig

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,42 @@ pub const DocumentType = struct {
3939
pub fn get_systemId(self: *parser.DocumentType) ![]const u8 {
4040
return try parser.documentTypeGetSystemId(self);
4141
}
42+
43+
// netsurf's DocumentType doesn't implement the dom_node_get_attributes
44+
// and thus will crash if we try to call nodeIsEqualNode.
45+
pub fn _isEqualNode(self: *parser.DocumentType, other_node: *parser.Node) !bool {
46+
if (try parser.nodeType(other_node) != .document_type) {
47+
return false;
48+
}
49+
50+
const other: *parser.DocumentType = @ptrCast(other_node);
51+
if (std.mem.eql(u8, try get_name(self), try get_name(other)) == false) {
52+
return false;
53+
}
54+
if (std.mem.eql(u8, try get_publicId(self), try get_publicId(other)) == false) {
55+
return false;
56+
}
57+
if (std.mem.eql(u8, try get_systemId(self), try get_systemId(other)) == false) {
58+
return false;
59+
}
60+
return true;
61+
}
4262
};
63+
64+
const testing = @import("../../testing.zig");
65+
test "Browser.DOM.DocumentType" {
66+
var runner = try testing.jsRunner(testing.tracking_allocator, .{});
67+
defer runner.deinit();
68+
69+
try runner.testCases(&.{
70+
.{ "let dt1 = document.implementation.createDocumentType('qname1', 'pid1', 'sys1');", "undefined" },
71+
.{ "let dt2 = document.implementation.createDocumentType('qname2', 'pid2', 'sys2');", "undefined" },
72+
.{ "let dt3 = document.implementation.createDocumentType('qname1', 'pid1', 'sys1');", "undefined" },
73+
.{ "dt1.isEqualNode(dt1)", "true" },
74+
.{ "dt1.isEqualNode(dt3)", "true" },
75+
.{ "dt1.isEqualNode(dt2)", "false" },
76+
.{ "dt2.isEqualNode(dt3)", "false" },
77+
.{ "dt1.isEqualNode(document)", "false" },
78+
.{ "document.isEqualNode(dt1)", "false" },
79+
}, .{});
80+
}

src/browser/dom/processing_instruction.zig

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,36 @@ pub const ProcessingInstruction = struct {
4646
pub fn set_data(self: *parser.ProcessingInstruction, data: []u8) !void {
4747
try parser.nodeSetValue(parser.processingInstructionToNode(self), data);
4848
}
49+
50+
// netsurf's ProcessInstruction doesn't implement the dom_node_get_attributes
51+
// and thus will crash if we try to call nodeIsEqualNode.
52+
pub fn _isEqualNode(self: *parser.ProcessingInstruction, other_node: *parser.Node) !bool {
53+
if (try parser.nodeType(other_node) != .processing_instruction) {
54+
return false;
55+
}
56+
57+
const other: *parser.ProcessingInstruction = @ptrCast(other_node);
58+
59+
if (std.mem.eql(u8, try get_target(self), try get_target(other)) == false) {
60+
return false;
61+
}
62+
63+
{
64+
const self_data = try get_data(self);
65+
const other_data = try get_data(other);
66+
if (self_data == null and other_data != null) {
67+
return false;
68+
}
69+
if (self_data != null and other_data == null) {
70+
return false;
71+
}
72+
if (std.mem.eql(u8, self_data.?, other_data.?) == false) {
73+
return false;
74+
}
75+
}
76+
77+
return true;
78+
}
4979
};
5080

5181
const testing = @import("../../testing.zig");
@@ -62,4 +92,16 @@ test "Browser.DOM.ProcessingInstruction" {
6292

6393
.{ "let pi2 = pi.cloneNode()", "undefined" },
6494
}, .{});
95+
96+
try runner.testCases(&.{
97+
.{ "let pi11 = document.createProcessingInstruction('target1', 'data1');", "undefined" },
98+
.{ "let pi12 = document.createProcessingInstruction('target2', 'data2');", "undefined" },
99+
.{ "let pi13 = document.createProcessingInstruction('target1', 'data1');", "undefined" },
100+
.{ "pi11.isEqualNode(pi11)", "true" },
101+
.{ "pi11.isEqualNode(pi13)", "true" },
102+
.{ "pi11.isEqualNode(pi12)", "false" },
103+
.{ "pi12.isEqualNode(pi13)", "false" },
104+
.{ "pi11.isEqualNode(document)", "false" },
105+
.{ "document.isEqualNode(pi11)", "false" },
106+
}, .{});
65107
}

0 commit comments

Comments
 (0)