Skip to content

Commit

Permalink
fix: #43 floating point issue in multipleOf validation
Browse files Browse the repository at this point in the history
- fix multipleOf validation logic
- remove floatingPointPrecision setting
  • Loading branch information
Sascha Goldhofer committed Aug 15, 2023
1 parent d6a307c commit 743ff3d
Show file tree
Hide file tree
Showing 12 changed files with 151 additions and 16 deletions.
2 changes: 1 addition & 1 deletion dist/jsonSchemaLibrary.js

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion dist/lib/config/settings.d.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
declare const _default: {
DECLARATOR_ONEOF: string;
GET_TEMPLATE_RECURSION_LIMIT: number;
floatingPointPrecision: number;
propertyBlacklist: string[];
templateDefaultOptions: {
addOptionalProps: boolean;
Expand Down
4 changes: 4 additions & 0 deletions dist/lib/utils/getPrecision.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/**
* returns the floating point precision of a decimal number or 0
*/
export declare function getPrecision(value: number): number;
1 change: 0 additions & 1 deletion dist/module/lib/config/settings.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
export default {
DECLARATOR_ONEOF: "oneOfProperty",
GET_TEMPLATE_RECURSION_LIMIT: 1,
floatingPointPrecision: 10000,
propertyBlacklist: ["_id"],
templateDefaultOptions: {
addOptionalProps: false,
Expand Down
8 changes: 8 additions & 0 deletions dist/module/lib/utils/getPrecision.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/**
* returns the floating point precision of a decimal number or 0
*/
export function getPrecision(value) {
const string = `${value}`;
const index = string.indexOf(".");
return index === -1 ? 0 : string.length - (index + 1);
}
22 changes: 17 additions & 5 deletions dist/module/lib/validation/keyword.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import { validateAllOf } from "../features/allOf";
import { validateAnyOf } from "../features/anyOf";
import { validateDependencies } from "../features/dependencies";
import { validateOneOf } from "../features/oneOf";
import { getPrecision } from "../utils/getPrecision";
import deepEqual from "fast-deep-equal";
const FPP = settings.floatingPointPrecision;
const hasOwnProperty = Object.prototype.hasOwnProperty;
const hasProperty = (value, property) => !(value[property] === undefined || !hasOwnProperty.call(value, property));
// list of validation keywords: http://json-schema.org/latest/json-schema-validation.html#rfc.section.5
Expand Down Expand Up @@ -283,12 +283,24 @@ const KeywordValidation = {
return undefined;
},
multipleOf: (draft, schema, value, pointer) => {
if (isNaN(schema.multipleOf)) {
if (isNaN(schema.multipleOf) || typeof value !== "number") {
return undefined;
}
// https://github.com/cfworker/cfworker/blob/master/packages/json-schema/src/validate.ts#L1061
// https://github.com/ExodusMovement/schemasafe/blob/master/src/compile.js#L441
if (((value * FPP) % (schema.multipleOf * FPP)) / FPP !== 0) {
const valuePrecision = getPrecision(value);
const multiplePrecision = getPrecision(schema.multipleOf);
if (valuePrecision > multiplePrecision) {
// higher precision of value can never be multiple of value
return draft.errors.multipleOfError({
multipleOf: schema.multipleOf,
value,
pointer,
schema
});
}
const precision = Math.pow(10, multiplePrecision);
const val = Math.round(value * precision);
const multiple = Math.round(schema.multipleOf * precision);
if ((val % multiple) / precision !== 0) {
return draft.errors.multipleOfError({
multipleOf: schema.multipleOf,
value,
Expand Down
1 change: 0 additions & 1 deletion lib/config/settings.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
export default {
DECLARATOR_ONEOF: "oneOfProperty",
GET_TEMPLATE_RECURSION_LIMIT: 1,
floatingPointPrecision: 10000,
propertyBlacklist: ["_id"],
templateDefaultOptions: {
addOptionalProps: false,
Expand Down
1 change: 0 additions & 1 deletion lib/getChildSchemaSelection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ export default function getChildSchemaSelection(
}

const result = draft.step(property, schema, {}, "#");

if (isJsonError(result)) {
return result;
}
Expand Down
8 changes: 8 additions & 0 deletions lib/utils/getPrecision.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/**
* returns the floating point precision of a decimal number or 0
*/
export function getPrecision(value: number): number {
const string = `${value}`;
const index = string.indexOf(".");
return index === -1 ? 0 : string.length - (index + 1);
}
28 changes: 22 additions & 6 deletions lib/validation/keyword.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import { validateAllOf } from "../features/allOf";
import { validateAnyOf } from "../features/anyOf";
import { validateDependencies } from "../features/dependencies";
import { validateOneOf } from "../features/oneOf";
import { getPrecision } from "../utils/getPrecision";
import deepEqual from "fast-deep-equal";
const FPP = settings.floatingPointPrecision;

const hasOwnProperty = Object.prototype.hasOwnProperty;
const hasProperty = (value: Record<string, unknown>, property: string) =>
Expand Down Expand Up @@ -316,20 +316,36 @@ const KeywordValidation: Record<string, JsonValidator> = {
return undefined;
},
multipleOf: (draft, schema, value: number, pointer) => {
if (isNaN(schema.multipleOf)) {
if (isNaN(schema.multipleOf) || typeof value !== "number") {
return undefined;
}
// https://github.com/cfworker/cfworker/blob/master/packages/json-schema/src/validate.ts#L1061
// https://github.com/ExodusMovement/schemasafe/blob/master/src/compile.js#L441
if (((value * FPP) % (schema.multipleOf * FPP)) / FPP !== 0) {

const valuePrecision = getPrecision(value);
const multiplePrecision = getPrecision(schema.multipleOf);
if (valuePrecision > multiplePrecision) {
// value with higher precision then multipleOf-precision can never be multiple
return draft.errors.multipleOfError({
multipleOf: schema.multipleOf,
value,
pointer,
schema
});
}

const precision = Math.pow(10, multiplePrecision);
const val = Math.round(value * precision);
const multiple = Math.round(schema.multipleOf * precision);
if ((val % multiple) / precision !== 0) {
return draft.errors.multipleOfError({
multipleOf: schema.multipleOf,
value,
pointer,
schema
});
}
// also check https://stackoverflow.com/questions/1815367/catch-and-compute-overflow-during-multiplication-of-two-large-integers

// maybe also check overflow
// https://stackoverflow.com/questions/1815367/catch-and-compute-overflow-during-multiplication-of-two-large-integers
return undefined;
},
not: (draft, schema, value, pointer) => {
Expand Down
80 changes: 80 additions & 0 deletions test/unit/issues/issue43.multipleOf.float.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { expect } from "chai";
import { Draft07 as Draft } from "../../../lib/draft07";

describe("issue#43 - multipleOf .01", () => {
let draft: Draft;
beforeEach(() => {
draft = new Draft({
type: "number",
multipleOf: 0.01
});
});

it("should validate 1", () => {
const result = draft.validate(1);
expect(result).to.have.length(0);
});

it("should validate .2", () => {
const result = draft.validate(0.2);
expect(result).to.have.length(0);
});

it("should validate .02", () => {
const result = draft.validate(0.02);
expect(result).to.have.length(0);
});

it("should not validate .025", () => {
const result = draft.validate(0.025);
expect(result).to.have.length(1);
});

it("should validate 1.36", () => {
const result = draft.validate(1.36);
expect(result).to.have.length(0);
});

it("should validate 2.74", () => {
const result = draft.validate(2.74);
expect(result).to.have.length(0);
});

it("should validate 123456789", () => {
const result = draft.validate(123456789);
expect(result).to.have.length(0);
});

it("should not validate Infinity", () => {
const result = draft.validate(1e308);
expect(result).to.have.length(1);
});

it("should validate all floats with two decimals", () => {
for (let i = 0; i <= 100; i++) {
let num = `2.${i}`;
expect(draft.validate(parseFloat(num))).to.have.length(
0,
`should have validated '${num}'`
);
}
});

it("should still validate multiple of integers", () => {
draft = new Draft({
type: "number",
multipleOf: 3
});
const result = draft.validate(9);
expect(result).to.have.length(0);
});

it("should still invalidate non-multiples of integers", () => {
draft = new Draft({
type: "number",
multipleOf: 3
});
const result = draft.validate(7);
expect(result).to.have.length(1);
});
});
11 changes: 11 additions & 0 deletions test/unit/utils/getPrecision.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { expect } from "chai";
import { getPrecision } from "../../../lib/utils/getPrecision";

describe("getPrecision", () => {
it("should return decimal precision", () => {
expect(getPrecision(1.1)).to.equal(1);
expect(getPrecision(0.12)).to.equal(2);
expect(getPrecision(0.123)).to.equal(3);
expect(getPrecision(123.4567)).to.equal(4);
});
});

0 comments on commit 743ff3d

Please sign in to comment.