Skip to content

Commit

Permalink
feat: stronger check on bad cdata closure
Browse files Browse the repository at this point in the history
It is also faster.
  • Loading branch information
lddubeau committed Aug 14, 2018
1 parent c41d3c2 commit d416760
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 17 deletions.
25 changes: 12 additions & 13 deletions lib/saxes.js
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,6 @@ class SaxesParser {
* @returns this
*/
fail(er) {
this.closeText();
const message = (this.trackPosition) ?
`${this.fileName}:${this.line}:${this.column}: ${er}` : er;

Expand Down Expand Up @@ -621,7 +620,7 @@ class SaxesParser {
}

/**
* Skip dwhitespace characters.
* Skip whitespace characters.
*
* @private
*
Expand Down Expand Up @@ -696,20 +695,11 @@ class SaxesParser {
cx => cx !== LESS && cx !== AMP,
"textNode",
(fragment) => {
// Text fragments is a buffer we use to check for the precence of a
// textFragments is a buffer we use to check for the precence of a
// literal "]]>" in text nodes. We cannot do the check against textNode
// itself because textNode will contain resolve entities so "]]>"
// itself because textNode will contain resolved entities so "]]>"
// would turn to "]]>" in textNode and raise a false error.
this.textFragments += fragment;
// We also have to check the end of textFragments because some cases may
// slip through otherwise. For instance, if client code write
// char-by-char. Then fragment will never contain ]]> but instead we'll
// have 3 fragments one with "]", a second with "]" and a third with
// ">".
if (fragment.includes("]]>") || this.textFragments.endsWith("]]>")) {
this.fail("the string \"]]>\" is disallowed in char data.");
}

if (!this.inRoot && /\S/.test(fragment)) {
// We use the reportedTextBeforeRoot and reportedTextAfterRoot flags
// to avoid reporting errors for every single character that is out of
Expand All @@ -726,6 +716,15 @@ class SaxesParser {
}
});

// We also have to check the end of textFragments because some cases may
// slip through otherwise. For instance, if client code writes
// char-by-char. Then fragment will never contain ]]> but instead we'll have
// 3 fragments one with "]", a second with "]" and a third with ">".
if (this.textFragments.includes("]]>")) {
this.textFragments = "";
this.fail("the string \"]]>\" is disallowed in char data.");
}

switch (c) {
case LESS:
this.state = S_OPEN_WAKA;
Expand Down
8 changes: 4 additions & 4 deletions test/issue-86.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ require(".").test({
"error",
"undefined:1:19: text data outside of root node.",
],
[
"text",
"de",
],
[
"error",
"undefined:1:20: unexpected end.",
],
[
"text",
"de",
],
],
opt: {},
});
98 changes: 98 additions & 0 deletions test/wrong-cdata-closure.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
"use strict";

const { test } = require(".");

const xml = "<span>somethingx]]>moo</span>";

describe("wrong cdata closure", () => {
test({
name: "one shot",
xml,
expect: [
["opentagstart", {
name: "span",
attributes: {},
}],
["opentag", {
name: "span",
attributes: {},
isSelfClosing: false,
}],
["error",
"undefined:1:23: the string \"]]>\" is disallowed in char data."],
["text", "somethingx]]>moo"],
["closetag", {
name: "span",
attributes: {},
isSelfClosing: false,
}],
["end", undefined],
["ready", undefined],
],
opt: {},
});

test({
name: "one-by-one",
fn(parser) {
for (const x of xml) {
parser.write(x);
}
parser.close();
},
expect: [
["opentagstart", {
name: "span",
attributes: {},
}],
["opentag", {
name: "span",
attributes: {},
isSelfClosing: false,
}],
["error",
"undefined:1:19: the string \"]]>\" is disallowed in char data."],
["text", "somethingx]]>moo"],
["closetag", {
name: "span",
attributes: {},
isSelfClosing: false,
}],
["end", undefined],
["ready", undefined],
],
opt: {},
});

test({
name: "two-by-two",
fn(parser) {
for (let ix = 0; ix < xml.length; ix += 2) {
parser.write(xml.slice(ix, ix + 2));
}
parser.close();
},
expect: [
["opentagstart", {
name: "span",
attributes: {},
}],
["opentag", {
name: "span",
attributes: {},
isSelfClosing: false,
}],
["error",
"undefined:1:20: the string \"]]>\" is disallowed in char data."],
["text", "somethingx]]>moo"],
["closetag", {
name: "span",
attributes: {},
isSelfClosing: false,
}],
["end", undefined],
["ready", undefined],
],
opt: {},
});
});

0 comments on commit d416760

Please sign in to comment.