Skip to content

Commit

Permalink
fix: Fix bug stringifying number-like keys and values (#645)
Browse files Browse the repository at this point in the history 
A string like "1e 6" will currently be stringified as 1e+6, which is
ambiguous because it looks like a number literal and it will be parsed
as one. This patch fixes the stringify code so that these strings are
quoted. AQF strings have a similar issue and this patch addresses them
as well.
  • Loading branch information
@dmaccormack
dmaccormack authored Nov 22, 2023
1 parent d502347 commit 9f66986
Show file tree
Hide file tree
Showing 4 changed files with 240 additions and 40 deletions.
76 changes: 53 additions & 23 deletions src/JsonURL.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { setupToJsonURLText, toJsonURLText } from "./proto.js";

const RX_DECODE_SPACE = /\+/g;
const RX_ENCODE_SPACE = / /g;
const RX_AQF_DECODE = /(![\s\S]?)/g;
const RX_AQF_DECODE_ESCAPE = /(![\s\S]?)/g;

//
// patterns for use with RegEx.test().
Expand All @@ -42,7 +42,9 @@ const RX_AQF_DECODE = /(![\s\S]?)/g;
const RX_ENCODE_STRING_SAFE =
/^[-A-Za-z0-9._~!$*;@?/ ][-A-Za-z0-9._~!$*;@?/' ]*$/;
const RX_ENCODE_STRING_QSAFE = /^[-A-Za-z0-9._~!$*,;@?/(): ]+$/;
const RX_ENCODE_NUMBER = /^-?\d+(?:\.\d+)?(?:[eE][-+]?\d+)?$/;
const RX_ENCODE_NUMBER = /^-?\d+(?:\.\d+)?(?:[eE][-]?\d+)?$/;
const RX_ENCODE_NUMBER_PLUS = /^-?\d+(?:\.\d+)?[eE]\+\d+$/;
const RX_ENCODE_NUMBER_SPACE = /^-?\d+(?:\.\d+)?[eE] \d+$/;

const RX_ENCODE_BASE = /[(),:]|%2[04]|%3B/gi;
const RX_ENCODE_BASE_MAP = {
Expand All @@ -55,7 +57,7 @@ const RX_ENCODE_BASE_MAP = {
"%3B": ";",
};

const RX_ENCODE_AQF = /[!(),:]|%2[01489C]|%3[AB]/gi;
const RX_ENCODE_AQF = /[!(),:]|%2[01489BC]|%3[AB]/gi;
const RX_ENCODE_AQF_MAP = {
"%20": "+",
"%21": "!!",
Expand All @@ -66,6 +68,7 @@ const RX_ENCODE_AQF_MAP = {
"%29": "!)",
")": "!)",
"+": "!+",
"%2B": "!+",
"%2C": "!,",
",": "!,",
"%3A": "!:",
Expand Down Expand Up @@ -131,6 +134,7 @@ UNESCAPE[CHAR_n] = "n";
const EMPTY_STRING = "";
const EMPTY_STRING_AQF = "!e";
const SPACE = " ";
const PLUS = "+";

function newEmptyString(pos, emptyOK) {
if (emptyOK) {
Expand Down Expand Up @@ -205,7 +209,17 @@ function hexDecode(pos, c) {
}
}

function isBoolNullNumber(s) {
function isBang(s, offset) {
return (
s.charCodeAt(offset - 1) === CHAR_BANG ||
(offset > 2 &&
s.charCodeAt(offset - 3) === CHAR_PERCENT &&
s.charCodeAt(offset - 2) === CHAR_0 + 2 &&
s.charCodeAt(offset - 1) === CHAR_0 + 1)
);
}

function isBoolNullNoPlusNumber(s) {
if (s === "true" || s === "false" || s === "null") {
return true;
}
Expand Down Expand Up @@ -269,30 +283,41 @@ function toJsonURLText_String(options, depth, isKey) {
return encodeStringLiteral(this, options.AQF);
}

if (isBoolNullNumber(this)) {
if (isBoolNullNoPlusNumber(this)) {
//
// if this string looks like a Boolean, Number, or ``null'' literal
// then it must be quoted
// this string looks like a boolean, `null`, or number literal without
// a plus char
//
if (isKey === true) {
// keys are assumed to be strings
return this;
}
if (options.AQF) {
if (this.indexOf("+") == -1) {
return "!" + this;
}
return this.replace("+", "!+");
}
if (this.indexOf("+") == -1) {
return "'" + this + "'";
return "!" + this;
}
return "'" + this + "'";
}

if (RX_ENCODE_NUMBER_PLUS.test(this)) {
//
// this string looks like a number with an exponent that includes a `+`
//
if (options.AQF) {
return this.replace(PLUS, "!+");
}
return this.replace(PLUS, "%2B");
}
if (RX_ENCODE_NUMBER_SPACE.test(this)) {
//
// if the string needs to be encoded then it no longer looks like a
// literal and does not need to be quoted.
// this string would look like a number if it were allowed to have a
// space represented as a plus
//
return encodeURIComponent(this);
if (options.AQF) {
return "!" + this.replace(SPACE, "+");
}
return "'" + this.replace(SPACE, "+") + "'";
}

if (options.AQF) {
return encodeStringLiteral(this, true);
}
Expand Down Expand Up @@ -957,10 +982,6 @@ class ParserAQF extends Parser {
return ret;
}

acceptPlus() {
return this.accept(CHAR_PLUS);
}

findLiteralEnd() {
const end = this.end;
const pos = this.pos;
Expand Down Expand Up @@ -1025,7 +1046,16 @@ class ParserAQF extends Parser {
const text = this.text;
const pos = this.pos;
const ret = decodeURIComponent(
text.substring(pos, litend).replace(RX_DECODE_SPACE, SPACE)
text
.substring(pos, litend)
.replace(RX_DECODE_SPACE, function (match, offset) {
if (offset === 0 || !isBang(text, pos + offset)) {
return SPACE;
}
return PLUS;
// const c = text.charCodeAt(pos + offset - 1);
// return c === CHAR_BANG ? PLUS : SPACE;
})
);

this.pos = litend;
Expand All @@ -1034,7 +1064,7 @@ class ParserAQF extends Parser {
return EMPTY_STRING;
}

return ret.replace(RX_AQF_DECODE, function name(match, _p, offset) {
return ret.replace(RX_AQF_DECODE_ESCAPE, function (match, _p, offset) {
if (match.length === 2) {
const c = match.charCodeAt(1);
const uc = UNESCAPE[c];
Expand Down
92 changes: 90 additions & 2 deletions test/parse.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ test.each([
["('Hello,+(World)!')", ["Hello, (World)!"]],
["('','')", ["", ""]],
["('qkey':g)", { qkey: "g" }],
["1e%2B1", "1e+1"],
])("JsonURL.parse(%s)", (text, expected) => {
expect(u.parse(text)).toEqual(expected);
expect(JsonURL.parse(text)).toEqual(expected);
Expand All @@ -116,7 +115,6 @@ test.each([
["('Hello!,+!(World!)!!')", ["'Hello, (World)!'"]],
["(!e,!e)", ["", ""]],
["(!e:g)", { "": "g" }],
["1e%2B1", 10],
["%48%45%4C%4C%4F%21,+%57%4F%52%4C%44!!", "HELLO, WORLD!"],
["%28%61%3A%62%2C%63%3A%64%29", { a: "b", c: "d" }],
["%28%61%2C%62%2C%63%2c%64%29", ["a", "b", "c", "d"]],
Expand Down Expand Up @@ -150,6 +148,96 @@ test.each([
expect(parseAQF(text, options)).toEqual(expected);
});

//
// Test edge cases for number-like strings combined with various options
//
test.each([
[
"1e+1",
"1e%2B1",
"1e!+1",
"1e%2B1",
"1e!+1",
"1e%2B1",
"1e%2B1",
"1e!+1",
"1e!+1",
],
[
"1e+3",
"1e+3",
"1e+3",
"1e%2B3",
"1e!+3",
"1e%2B3",
"1e%2B3",
"1e!+3",
"1e!+3",
],
[
"1e 3",
"'1e+3'",
"!1e+3",
"1e+3",
"1e+3",
"'1e+3'",
"1e+3",
"!1e+3",
"1e+3",
],
])(
"JsonURL.parseNumberLikeString(%p)",
(
expected,
inputKey,
inputKeyAqf,
inputKeyIsl,
inputKeyIslAqf,
inputBase,
inputImpliedStringLiteral,
inputAqf,
inputImpliedStringAqf
) => {
function makeObject(s) {
const ret = {};
ret[s] = "a";
return ret;
}
function makeText(s) {
return "(" + s + ":a)";
}

expect(JsonURL.parse(makeText(inputKey))).toEqual(makeObject(expected));
expect(JsonURL.parse(makeText(inputKeyAqf), { AQF: true })).toEqual(
makeObject(expected)
);
expect(
JsonURL.parse(makeText(inputKeyIsl), { impliedStringLiterals: true })
).toEqual(makeObject(expected));

expect(
JsonURL.parse(makeText(inputKeyIslAqf), {
AQF: true,
impliedStringLiterals: true,
})
).toEqual(makeObject(expected));

expect(u.parse(inputBase)).toBe(expected);
expect(
u.parse(inputImpliedStringLiteral, { impliedStringLiterals: true })
).toBe(expected);
expect(u.parse(inputAqf, { AQF: true })).toBe(expected);
expect(
u.parse(inputImpliedStringAqf, { AQF: true, impliedStringLiterals: true })
).toBe(expected);
}
);

/*
(text, value, aqfValue, impliedStrValue) => {
*/

test.each([undefined])("JsonURL.parse(%p)", (text) => {
expect(u.parse(text)).toBeUndefined();
expect(JsonURL.parse(text)).toBeUndefined();
Expand Down
35 changes: 21 additions & 14 deletions test/parseLiteral.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,12 @@ function escapeStringAQF(s) {
.replace(/:/, "!:");
}

function runTest(text, value, keyValue, strLitValue) {
function runTest(text, value, keyValue, impliedStrValue) {
expect(u.parseLiteral(text)).toBe(value);
expect(JsonURL.parse(text)).toBe(value);
expect(u.parseLiteral(text, 0, text.length, true)).toBe(keyValue);
expect(
u.parseLiteral(text, 0, text.length, true, { impliedStringLiterals: true })
).toBe(strLitValue);
).toBe(impliedStrValue);

//
// verify that parseLiteral() and parse() return the same thing (as
Expand All @@ -61,10 +60,10 @@ function runTest(text, value, keyValue, strLitValue) {
expect(JsonURL.parse(text)).toBe(value);
}

function runTestAQF(text, value, strLitValue) {
function runTestAQF(text, value, impliedStrValue) {
expect(u.parse(text, { AQF: true })).toBe(value);
expect(u.parse(text, { AQF: true, impliedStringLiterals: true })).toBe(
strLitValue
impliedStrValue
);
}

Expand Down Expand Up @@ -147,14 +146,15 @@ test.each([
runTestAQF(textAQF, value, keyValue);
});

// eslint-disable-next-line jest/expect-expect
test.each([
//
// fixed point
//
["-3e0", -3, undefined, "-3e0"],
["1e+2", 1e2, undefined, "1e 2"],
["-2e+1", -2e1, undefined, "-2e 1"],
["1e-2", 1e-2, undefined, "1e-2"],
["1e+2", 1e2, undefined, "1e 2"],

//
// floating point
Expand All @@ -164,26 +164,33 @@ test.each([
//
// string
//
["'hello'", "hello", "'hello'", undefined],
["'hello'", "hello", "'hello'", "'hello'"],
["hello%2Bworld", "hello+world", undefined, undefined],
["y+%3D+mx+%2B+b", "y = mx + b", undefined, undefined],
["a%3Db%26c%3Dd", "a=b&c=d", undefined, undefined],
["hello%F0%9F%8D%95world", "hello\uD83C\uDF55world", undefined, undefined],
["-e+", "-e ", undefined, undefined],
["-e+1", "-e 1", undefined, undefined],
["1e%2B1", "1e+1", 10, "1e+1"],
["1e%2B1", "1e+1", "1e+1", "1e+1"],
["%26true", "&true", undefined, undefined],
["%3Dtrue", "=true", undefined, undefined],
])("JsonURL.parseLiteral(%p)", (text, value, aqfValue, strLitValue) => {
])("JsonURL.parseLiteral(%p)", (text, value, aqfValue, impliedStrValue) => {
let keyValue = typeof value === "string" ? value : text;
if (aqfValue === undefined) {
aqfValue = value;
}
if (strLitValue === undefined) {
strLitValue = aqfValue;
}
runTest(text, value, keyValue, strLitValue);
runTestAQF(text, aqfValue, strLitValue);

runTest(
text,
value,
keyValue,
impliedStrValue === undefined ? keyValue : impliedStrValue
);

expect(u.parse(text, { AQF: true })).toBe(aqfValue);
expect(u.parse(text, { AQF: true, impliedStringLiterals: true })).toBe(
impliedStrValue === undefined ? aqfValue : impliedStrValue
);
});

test("JsonURL.parseLiteral('null')", () => {
Expand Down
Loading

0 comments on commit 9f66986

Please sign in to comment.