Skip to content

Commit f6b66b0

Browse files
benchristelbenchristel
authored
Move isRealJSONParse to its own file (#1829)
This enables the logic to be tested. Issue: none ## Test plan: `yarn test` Author: benchristel Reviewers: mark-fitzgerald, handeyeco, Myranae Required Reviewers: Approved By: mark-fitzgerald Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1829
1 parent 8aaf296 commit f6b66b0

File tree

4 files changed

+114
-86
lines changed

4 files changed

+114
-86
lines changed

‎.changeset/sharp-pianos-flow.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@khanacademy/perseus": patch
3+
---
4+
5+
Internal: Refactor parsePerseusItem and add tests

‎packages/perseus/src/util/is-real-json-parse.test.ts

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import {isRealJSONParse} from "./is-real-json-parse";
2+
3+
describe("isRealJSONParse", () => {
4+
it("returns false given a function that messes with itemData", () => {
5+
function fakeJSONParse(json: string) {
6+
const parsed = JSON.parse(json);
7+
parsed.data.assessmentItem.item.itemData = "";
8+
return parsed;
9+
}
10+
11+
expect(isRealJSONParse(fakeJSONParse)).toBe(false);
12+
});
13+
14+
it("returns true given the native JSON.parse function", () => {
15+
expect(isRealJSONParse(JSON.parse)).toBe(true);
16+
});
17+
});

‎packages/perseus/src/util/is-real-json-parse.ts

+88
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import Util from "../util";
2+
3+
const deepEq = Util.deepEq;
4+
5+
export function isRealJSONParse(jsonParse: typeof JSON.parse): boolean {
6+
const randomPhrase = buildRandomPhrase();
7+
const randomHintPhrase = buildRandomPhrase();
8+
const randomString = buildRandomString();
9+
const testingObject = JSON.stringify({
10+
answerArea: {
11+
calculator: false,
12+
chi2Table: false,
13+
financialCalculatorMonthlyPayment: false,
14+
financialCalculatorTimeToPayOff: false,
15+
financialCalculatorTotalAmount: false,
16+
periodicTable: false,
17+
periodicTableWithKey: false,
18+
tTable: false,
19+
zTable: false,
20+
},
21+
hints: [randomHintPhrase, `=${Math.floor(Math.random() * 50) + 1}`],
22+
itemDataVersion: {major: 0, minor: 1},
23+
question: {
24+
content: `${randomPhrase}`,
25+
images: {},
26+
widgets: {
27+
expression1: {
28+
alignment: "default",
29+
graded: false,
30+
options: {
31+
answerForms: [
32+
{
33+
considered: "wrong",
34+
form: false,
35+
key: 0,
36+
simplify: false,
37+
value: `${randomString}`,
38+
},
39+
],
40+
ariaLabel: "Answer",
41+
buttonSets: ["basic"],
42+
functions: ["f", "g", "h"],
43+
static: true,
44+
times: false,
45+
visibleLabel: "Answer",
46+
},
47+
static: true,
48+
type: "expression",
49+
version: {major: 1, minor: 0},
50+
},
51+
},
52+
},
53+
});
54+
const testJSON = buildTestData(testingObject.replace(/"/g, '\\"'));
55+
const parsedTestJSON = jsonParse(testJSON);
56+
const parsedTestItemData: string =
57+
parsedTestJSON.data.assessmentItem.item.itemData;
58+
return deepEq(parsedTestItemData, testingObject);
59+
}
60+
61+
function buildRandomString(capitalize: boolean = false) {
62+
let randomString: string = "";
63+
const randomLength = Math.floor(Math.random() * 8) + 3;
64+
for (let i = 0; i < randomLength; i++) {
65+
const randomLetter = String.fromCharCode(
66+
97 + Math.floor(Math.random() * 26),
67+
);
68+
randomString +=
69+
capitalize && i === 0 ? randomLetter.toUpperCase() : randomLetter;
70+
}
71+
return randomString;
72+
}
73+
74+
function buildRandomPhrase() {
75+
const phrases: string[] = [];
76+
const randomLength = Math.floor(Math.random() * 10) + 5;
77+
for (let i = 0; i < randomLength; i++) {
78+
phrases.push(buildRandomString(i === 0));
79+
}
80+
const modifierStart = ["**", "$"];
81+
const modifierEnd = ["**", "$"];
82+
const modifierIndex = Math.floor(Math.random() * modifierStart.length);
83+
return `${modifierStart[modifierIndex]}${phrases.join(" ")}${modifierEnd[modifierIndex]}`;
84+
}
85+
86+
function buildTestData(testObject: string) {
87+
return `{"data":{"assessmentItem":{"__typename":"AssessmentItemOrError","error":null,"item":{"__typename":"AssessmentItem","id":"x890b3c70f3e8f4a6","itemData":"${testObject}","problemType":"Type 1","sha":"c7284a3ad65214b4e62bccce236d92f7f5d35941"}}}}`;
88+
}

‎packages/perseus/src/util/parse-perseus-json.ts

+4-86
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
1-
import Util from "../util";
1+
import {isRealJSONParse} from "./is-real-json-parse";
22

33
import type {PerseusItem} from "../perseus-types";
44

5-
const deepEq = Util.deepEq;
6-
75
/**
86
* Helper to parse PerseusItem JSON
97
* Why not just use JSON.parse? We want:
@@ -13,90 +11,10 @@ const deepEq = Util.deepEq;
1311
* @returns {PerseusItem} the parsed PerseusItem object
1412
*/
1513
export function parsePerseusItem(json: string): PerseusItem {
16-
const randomPhrase = buildRandomPhrase();
17-
const randomHintPhrase = buildRandomPhrase();
18-
const randomString = buildRandomString();
19-
const testingObject = JSON.stringify({
20-
answerArea: {
21-
calculator: false,
22-
chi2Table: false,
23-
financialCalculatorMonthlyPayment: false,
24-
financialCalculatorTimeToPayOff: false,
25-
financialCalculatorTotalAmount: false,
26-
periodicTable: false,
27-
periodicTableWithKey: false,
28-
tTable: false,
29-
zTable: false,
30-
},
31-
hints: [randomHintPhrase, `=${Math.floor(Math.random() * 50) + 1}`],
32-
itemDataVersion: {major: 0, minor: 1},
33-
question: {
34-
content: `${randomPhrase}`,
35-
images: {},
36-
widgets: {
37-
expression1: {
38-
alignment: "default",
39-
graded: false,
40-
options: {
41-
answerForms: [
42-
{
43-
considered: "wrong",
44-
form: false,
45-
key: 0,
46-
simplify: false,
47-
value: `${randomString}`,
48-
},
49-
],
50-
ariaLabel: "Answer",
51-
buttonSets: ["basic"],
52-
functions: ["f", "g", "h"],
53-
static: true,
54-
times: false,
55-
visibleLabel: "Answer",
56-
},
57-
static: true,
58-
type: "expression",
59-
version: {major: 1, minor: 0},
60-
},
61-
},
62-
},
63-
});
64-
// @ts-expect-error TS2550: Property 'replaceAll' does not exist on type 'string'.
65-
const testJSON = buildTestData(testingObject.replaceAll('"', '\\"'));
66-
const parsedJSON = JSON.parse(testJSON);
67-
const parsedItemData: string = parsedJSON.data.assessmentItem.item.itemData;
68-
const isNotCheating = deepEq(parsedItemData, testingObject);
69-
if (isNotCheating) {
14+
// Try to block a cheating vector which relies on monkey-patching
15+
// JSON.parse
16+
if (isRealJSONParse(JSON.parse)) {
7017
return JSON.parse(json);
7118
}
7219
throw new Error("Something went wrong.");
7320
}
74-
75-
function buildRandomString(capitalize: boolean = false) {
76-
let randomString: string = "";
77-
const randomLength = Math.floor(Math.random() * 8) + 3;
78-
for (let i = 0; i < randomLength; i++) {
79-
const randomLetter = String.fromCharCode(
80-
97 + Math.floor(Math.random() * 26),
81-
);
82-
randomString +=
83-
capitalize && i === 0 ? randomLetter.toUpperCase() : randomLetter;
84-
}
85-
return randomString;
86-
}
87-
88-
function buildRandomPhrase() {
89-
const phrases: string[] = [];
90-
const randomLength = Math.floor(Math.random() * 10) + 5;
91-
for (let i = 0; i < randomLength; i++) {
92-
phrases.push(buildRandomString(i === 0));
93-
}
94-
const modifierStart = ["**", "$"];
95-
const modifierEnd = ["**", "$"];
96-
const modifierIndex = Math.floor(Math.random() * modifierStart.length);
97-
return `${modifierStart[modifierIndex]}${phrases.join(" ")}${modifierEnd[modifierIndex]}`;
98-
}
99-
100-
function buildTestData(testObject: string) {
101-
return `{"data":{"assessmentItem":{"__typename":"AssessmentItemOrError","error":null,"item":{"__typename":"AssessmentItem","id":"x890b3c70f3e8f4a6","itemData":"${testObject}","problemType":"Type 1","sha":"c7284a3ad65214b4e62bccce236d92f7f5d35941"}}}}`;
102-
}

0 commit comments

Comments
 (0)