Skip to content

Commit

Permalink
fix(parse): reject numeric strings with non-numbers (#2729)
Browse files Browse the repository at this point in the history 
* fix(parse): reject numeric strings with non-numbers

This updates the number parsing utilities to reject strings like
"1A", which `parseFloat` would ordinarily happily accept as the
value `1`.

* chore: use more compact regex
  • Loading branch information
@JordonPhillips
JordonPhillips committed Sep 2, 2021
1 parent 05a7701 commit afeccd7
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 30 deletions.
108 changes: 98 additions & 10 deletions packages/smithy-client/src/parse-utils.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,13 +320,44 @@ describe("strictParseDouble", () => {
expect(strictParseDouble("NaN")).toEqual(NaN);
});

it("rejects implicit NaN", () => {
expect(() => strictParseDouble("foo")).toThrowError();
describe("rejects implicit NaN", () => {
it.each([
"foo",
"123ABC",
"ABC123",
"12AB3C",
"1.A",
"1.1A",
"1.1A1",
"0xFF",
"0XFF",
"0b1111",
"0B1111",
"0777",
"0o777",
"0O777",
"1n",
"1N",
"1_000",
"e",
"e1",
".1",
])("rejects %s", (value) => {
expect(() => strictParseDouble(value)).toThrowError();
});
});

it("accepts numeric strings", () => {
expect(strictParseDouble("1")).toEqual(1);
expect(strictParseDouble("-1")).toEqual(-1);
expect(strictParseDouble("1.1")).toEqual(1.1);
expect(strictParseDouble("1e1")).toEqual(10);
expect(strictParseDouble("-1e1")).toEqual(-10);
expect(strictParseDouble("1e+1")).toEqual(10);
expect(strictParseDouble("1e-1")).toEqual(0.1);
expect(strictParseDouble("1E1")).toEqual(10);
expect(strictParseDouble("1E+1")).toEqual(10);
expect(strictParseDouble("1E-1")).toEqual(0.1);
});

describe("accepts numbers", () => {
Expand All @@ -347,8 +378,31 @@ describe("strictParseFloat32", () => {
expect(strictParseFloat32("NaN")).toEqual(NaN);
});

it("rejects implicit NaN", () => {
expect(() => strictParseFloat32("foo")).toThrowError();
describe("rejects implicit NaN", () => {
it.each([
"foo",
"123ABC",
"ABC123",
"12AB3C",
"1.A",
"1.1A",
"1.1A1",
"0xFF",
"0XFF",
"0b1111",
"0B1111",
"0777",
"0o777",
"0O777",
"1n",
"1N",
"1_000",
"e",
"e1",
".1",
])("rejects %s", (value) => {
expect(() => strictParseFloat32(value)).toThrowError();
});
});

describe("rejects doubles", () => {
Expand All @@ -359,7 +413,15 @@ describe("strictParseFloat32", () => {

it("accepts numeric strings", () => {
expect(strictParseFloat32("1")).toEqual(1);
expect(strictParseFloat32("-1")).toEqual(-1);
expect(strictParseFloat32("1.1")).toEqual(1.1);
expect(strictParseFloat32("1e1")).toEqual(10);
expect(strictParseFloat32("-1e1")).toEqual(-10);
expect(strictParseFloat32("1e+1")).toEqual(10);
expect(strictParseFloat32("1e-1")).toEqual(0.1);
expect(strictParseFloat32("1E1")).toEqual(10);
expect(strictParseFloat32("1E+1")).toEqual(10);
expect(strictParseFloat32("1E-1")).toEqual(0.1);
});

describe("accepts numbers", () => {
Expand Down Expand Up @@ -489,12 +551,26 @@ describe("strictParseLong", () => {
});

describe("rejects non-integers", () => {
it.each([1.1, "1.1", "NaN", "Infinity", "-Infinity", NaN, Infinity, -Infinity, true, false, [], {}])(
"rejects %s",
(value) => {
expect(() => strictParseLong(value as any)).toThrowError();
}
);
it.each([
1.1,
"1.1",
"NaN",
"Infinity",
"-Infinity",
NaN,
Infinity,
-Infinity,
true,
false,
[],
{},
"foo",
"123ABC",
"ABC123",
"12AB3C",
])("rejects %s", (value) => {
expect(() => strictParseLong(value as any)).toThrowError();
});
});
});

Expand Down Expand Up @@ -530,6 +606,10 @@ describe("strictParseInt32", () => {
-(2 ** 63 + 1),
2 ** 31,
-(2 ** 31 + 1),
"foo",
"123ABC",
"ABC123",
"12AB3C",
])("rejects %s", (value) => {
expect(() => strictParseInt32(value as any)).toThrowError();
});
Expand Down Expand Up @@ -570,6 +650,10 @@ describe("strictParseShort", () => {
-(2 ** 31 + 1),
2 ** 15,
-(2 ** 15 + 1),
"foo",
"123ABC",
"ABC123",
"12AB3C",
])("rejects %s", (value) => {
expect(() => strictParseShort(value as any)).toThrowError();
});
Expand Down Expand Up @@ -612,6 +696,10 @@ describe("strictParseByte", () => {
-(2 ** 15 + 1),
128,
-129,
"foo",
"123ABC",
"ABC123",
"12AB3C",
])("rejects %s", (value) => {
expect(() => strictParseByte(value as any)).toThrowError();
});
Expand Down
42 changes: 22 additions & 20 deletions packages/smithy-client/src/parse-utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,8 @@ export const expectString = (value: any): string | undefined => {
* @returns The value as a number, or undefined if it's null/undefined.
*/
export const strictParseDouble = (value: string | number): number | undefined => {
if (value === "NaN") {
return NaN;
}
if (typeof value == "string") {
const parsed: number = parseFloat(value);
if (Number.isNaN(parsed)) {
throw new TypeError(`Expected real number, got implicit NaN`);
}
return expectNumber(parsed);
return expectNumber(parseNumber(value));
}
return expectNumber(value);
};
Expand All @@ -262,19 +255,28 @@ export const strictParseFloat = strictParseDouble;
* @returns The value as a number, or undefined if it's null/undefined.
*/
export const strictParseFloat32 = (value: string | number): number | undefined => {
if (value === "NaN") {
return NaN;
}
if (typeof value == "string") {
const parsed: number = parseFloat(value);
if (Number.isNaN(parsed)) {
throw new TypeError(`Expected real number, got implicit NaN`);
}
return expectFloat32(parsed);
return expectFloat32(parseNumber(value));
}
return expectFloat32(value);
};

// This regex matches JSON-style numbers. In short:
// * The integral may start with a negative sign, but not a positive one
// * No leading 0 on the integral unless it's immediately followed by a '.'
// * Exponent indicated by a case-insensitive 'E' optionally followed by a
// positive/negative sign and some number of digits.
// It also matches both positive and negative infinity as well and explicit NaN.
const NUMBER_REGEX = /(-?(?:0|[1-9]\d*)(?:\.\d+)?(?:[eE][+-]?\d+)?)|(-?Infinity)|(NaN)/g;

const parseNumber = (value: string): number => {
const matches = value.match(NUMBER_REGEX);
if (matches === null || matches[0].length !== value.length) {
throw new TypeError(`Expected real number, got implicit NaN`);
}
return parseFloat(value);
};

/**
* Asserts a value is a number and returns it. If the value is a string
* representation of a non-numeric number type (NaN, Infinity, -Infinity),
Expand Down Expand Up @@ -346,7 +348,7 @@ export const strictParseLong = (value: string | number): number | undefined => {
if (typeof value === "string") {
// parseInt can't be used here, because it will silently discard any
// existing decimals. We want to instead throw an error if there are any.
return expectLong(parseFloat(value));
return expectLong(parseNumber(value));
}
return expectLong(value);
};
Expand All @@ -370,7 +372,7 @@ export const strictParseInt32 = (value: string | number): number | undefined =>
if (typeof value === "string") {
// parseInt can't be used here, because it will silently discard any
// existing decimals. We want to instead throw an error if there are any.
return expectInt32(parseFloat(value));
return expectInt32(parseNumber(value));
}
return expectInt32(value);
};
Expand All @@ -389,7 +391,7 @@ export const strictParseShort = (value: string | number): number | undefined =>
if (typeof value === "string") {
// parseInt can't be used here, because it will silently discard any
// existing decimals. We want to instead throw an error if there are any.
return expectShort(parseFloat(value));
return expectShort(parseNumber(value));
}
return expectShort(value);
};
Expand All @@ -408,7 +410,7 @@ export const strictParseByte = (value: string | number): number | undefined => {
if (typeof value === "string") {
// parseInt can't be used here, because it will silently discard any
// existing decimals. We want to instead throw an error if there are any.
return expectByte(parseFloat(value));
return expectByte(parseNumber(value));
}
return expectByte(value);
};

0 comments on commit afeccd7

Please sign in to comment.