Skip to content

Commit

Permalink
Make JSON.parse usage safer (#373)
Browse files Browse the repository at this point in the history
* Make JSON.parse usage safer

* Fix any type

* Add readResponseJson

* Use readResponseJson

* Additional updates

* Rename files

* Add types
  • Loading branch information
toasted-nutbread authored Dec 19, 2023
1 parent 5f96276 commit 1ced9aa
Show file tree
Hide file tree
Showing 35 changed files with 257 additions and 77 deletions.
5 changes: 3 additions & 2 deletions dev/bin/schema-validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import fs from 'fs';
import {performance} from 'perf_hooks';
import {parseJson} from '../../ext/js/core/json.js';
import {createJsonSchema} from '../schema-validate.js';

/** */
Expand All @@ -39,14 +40,14 @@ function main() {
}

const schemaSource = fs.readFileSync(args[0], {encoding: 'utf8'});
const schema = JSON.parse(schemaSource);
const schema = parseJson(schemaSource);

for (const dataFileName of args.slice(1)) {
const start = performance.now();
try {
console.log(`Validating ${dataFileName}...`);
const dataSource = fs.readFileSync(dataFileName, {encoding: 'utf8'});
const data = JSON.parse(dataSource);
const data = parseJson(dataSource);
createJsonSchema(mode, schema).validate(data);
const end = performance.now();
console.log(`No issues detected (${((end - start) / 1000).toFixed(2)}s)`);
Expand Down
7 changes: 6 additions & 1 deletion dev/build-libs.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import esbuild from 'esbuild';
import fs from 'fs';
import path from 'path';
import {fileURLToPath} from 'url';
import {parseJson} from './json.js';

const dirname = path.dirname(fileURLToPath(import.meta.url));
const extDir = path.join(dirname, '..', 'ext');
Expand Down Expand Up @@ -61,7 +62,11 @@ export async function buildLibs() {

const schemaDir = path.join(extDir, 'data/schemas/');
const schemaFileNames = fs.readdirSync(schemaDir);
const schemas = schemaFileNames.map((schemaFileName) => JSON.parse(fs.readFileSync(path.join(schemaDir, schemaFileName), {encoding: 'utf8'})));
const schemas = schemaFileNames.map((schemaFileName) => {
/** @type {import('ajv').AnySchema} */
const result = parseJson(fs.readFileSync(path.join(schemaDir, schemaFileName), {encoding: 'utf8'}));
return result;
});
const ajv = new Ajv({
schemas,
code: {source: true, esm: true},
Expand Down
8 changes: 5 additions & 3 deletions dev/dictionary-validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import JSZip from 'jszip';
import path from 'path';
import {performance} from 'perf_hooks';
import {fileURLToPath} from 'url';
import {parseJson} from './json.js';
import {createJsonSchema} from './schema-validate.js';

const dirname = path.dirname(fileURLToPath(import.meta.url));
Expand All @@ -32,7 +33,7 @@ const dirname = path.dirname(fileURLToPath(import.meta.url));
function readSchema(relativeFileName) {
const fileName = path.join(dirname, relativeFileName);
const source = fs.readFileSync(fileName, {encoding: 'utf8'});
return JSON.parse(source);
return parseJson(source);
}

/**
Expand All @@ -57,7 +58,7 @@ async function validateDictionaryBanks(mode, zip, fileNameFormat, schema) {
const file = zip.files[fileName];
if (!file) { break; }

const data = JSON.parse(await file.async('string'));
const data = parseJson(await file.async('string'));
try {
jsonSchema.validate(data);
} catch (e) {
Expand All @@ -83,7 +84,8 @@ export async function validateDictionary(mode, archive, schemas) {
throw new Error('No dictionary index found in archive');
}

const index = JSON.parse(await indexFile.async('string'));
/** @type {import('dictionary-data').Index} */
const index = parseJson(await indexFile.async('string'));
const version = index.format || index.version;

try {
Expand Down
18 changes: 18 additions & 0 deletions dev/json.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright (C) 2023 Yomitan Authors
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/

export {parseJson} from '../ext/js/core/json.js';
5 changes: 3 additions & 2 deletions dev/manifest-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import childProcess from 'child_process';
import fs from 'fs';
import {fileURLToPath} from 'node:url';
import path from 'path';
import {parseJson} from './json.js';

const dirname = path.dirname(fileURLToPath(import.meta.url));

Expand All @@ -29,14 +30,14 @@ const dirname = path.dirname(fileURLToPath(import.meta.url));
* @returns {T}
*/
function clone(value) {
return JSON.parse(JSON.stringify(value));
return parseJson(JSON.stringify(value));
}


export class ManifestUtil {
constructor() {
const fileName = path.join(dirname, 'data', 'manifest-variants.json');
const {manifest, variants, defaultVariant} = /** @type {import('dev/manifest').ManifestConfig} */ (JSON.parse(fs.readFileSync(fileName, {encoding: 'utf8'})));
const {manifest, variants, defaultVariant} = /** @type {import('dev/manifest').ManifestConfig} */ (parseJson(fs.readFileSync(fileName, {encoding: 'utf8'})));
/** @type {import('dev/manifest').Manifest} */
this._manifest = manifest;
/** @type {import('dev/manifest').ManifestVariant[]} */
Expand Down
6 changes: 4 additions & 2 deletions dev/schema-validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import Ajv from 'ajv';
import {readFileSync} from 'fs';
import {JsonSchema} from '../ext/js/data/json-schema.js';
import {DataError} from './data-error.js';
import {parseJson} from './json.js';

class JsonSchemaAjv {
/**
Expand All @@ -32,7 +33,8 @@ class JsonSchemaAjv {
allowUnionTypes: true
});
const metaSchemaPath = require.resolve('ajv/dist/refs/json-schema-draft-07.json');
const metaSchema = JSON.parse(readFileSync(metaSchemaPath, {encoding: 'utf8'}));
/** @type {import('ajv').AnySchemaObject} */
const metaSchema = parseJson(readFileSync(metaSchemaPath, {encoding: 'utf8'}));
ajv.addMetaSchema(metaSchema);
/** @type {import('ajv').ValidateFunction} */
this._validate = ajv.compile(/** @type {import('ajv').Schema} */ (schema));
Expand All @@ -46,7 +48,7 @@ class JsonSchemaAjv {
if (this._validate(data)) { return; }
const {errors} = this._validate;
const error = new DataError('Schema validation failed');
error.data = JSON.parse(JSON.stringify(errors));
error.data = parseJson(JSON.stringify(errors));
throw error;
}
}
Expand Down
5 changes: 3 additions & 2 deletions dev/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import fs from 'fs';
import JSZip from 'jszip';
import path from 'path';
import {parseJson} from './json.js';

/**
* @param {string[]} args
Expand Down Expand Up @@ -112,9 +113,9 @@ export function createDictionaryArchive(dictionaryDirectory, dictionaryName) {
for (const fileName of fileNames) {
if (/\.json$/.test(fileName)) {
const content = fs.readFileSync(path.join(dictionaryDirectory, fileName), {encoding: 'utf8'});
const json = JSON.parse(content);
const json = parseJson(content);
if (fileName === 'index.json' && typeof dictionaryName === 'string') {
json.title = dictionaryName;
/** @type {import('dictionary-data').Index} */ (json).title = dictionaryName;
}
archive.file(fileName, JSON.stringify(json, null, 0));

Expand Down
15 changes: 11 additions & 4 deletions ext/js/background/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {ClipboardReader} from '../comm/clipboard-reader.js';
import {Mecab} from '../comm/mecab.js';
import {clone, deferPromise, generateId, invokeMessageHandler, isObject, log, promiseTimeout} from '../core.js';
import {ExtensionError} from '../core/extension-error.js';
import {parseJson, readResponseJson} from '../core/json.js';
import {AnkiUtil} from '../data/anki-util.js';
import {OptionsUtil} from '../data/options-util.js';
import {PermissionsUtil} from '../data/permissions-util.js';
Expand Down Expand Up @@ -291,7 +292,8 @@ export class Backend {
log.error(e);
}

const deinflectionReasons = /** @type {import('deinflector').ReasonsRaw} */ (await this._fetchJson('/data/deinflect.json'));
/** @type {import('deinflector').ReasonsRaw} */
const deinflectionReasons = await this._fetchJson('/data/deinflect.json');
this._translator.prepare(deinflectionReasons);

await this._optionsUtil.prepare();
Expand Down Expand Up @@ -764,6 +766,7 @@ export class Backend {

const frameId = sender.frameId;
const id = generateId(16);
/** @type {import('cross-frame-api').ActionPortDetails} */
const details = {
name: 'action-port',
id
Expand Down Expand Up @@ -908,11 +911,13 @@ export class Backend {
throw new Error('Port does not have an associated frame ID');
}

/** @type {import('cross-frame-api').CrossFrameCommunicationPortDetails} */
const sourceDetails = {
name: 'cross-frame-communication-port',
otherTabId: targetTabId,
otherFrameId: targetFrameId
};
/** @type {import('cross-frame-api').CrossFrameCommunicationPortDetails} */
const targetDetails = {
name: 'cross-frame-communication-port',
otherTabId: sourceTabId,
Expand Down Expand Up @@ -1530,7 +1535,8 @@ export class Backend {
hasStarted = true;
port.onMessage.removeListener(onMessage);

const messageData = JSON.parse(messageString);
/** @type {{action: string, params?: import('core').SerializableObject}} */
const messageData = parseJson(messageString);
messageString = null;
onMessageComplete(messageData);
}
Expand Down Expand Up @@ -2062,12 +2068,13 @@ export class Backend {
}

/**
* @template [T=unknown]
* @param {string} url
* @returns {Promise<unknown>}
* @returns {Promise<T>}
*/
async _fetchJson(url) {
const response = await this._fetchAsset(url);
return await response.json();
return await readResponseJson(response);
}

/**
Expand Down
3 changes: 2 additions & 1 deletion ext/js/comm/anki-connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/

import {ExtensionError} from '../core/extension-error.js';
import {parseJson} from '../core/json.js';
import {AnkiUtil} from '../data/anki-util.js';

/**
Expand Down Expand Up @@ -419,7 +420,7 @@ export class AnkiConnect {
let result;
try {
responseText = await response.text();
result = JSON.parse(responseText);
result = parseJson(responseText);
} catch (e) {
const error = new ExtensionError('Invalid Anki response');
error.data = {action, params, status: response.status, responseText, originalError: e};
Expand Down
11 changes: 8 additions & 3 deletions ext/js/comm/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import {deferPromise} from '../core.js';
import {ExtensionError} from '../core/extension-error.js';
import {parseJson} from '../core/json.js';

export class API {
/**
Expand Down Expand Up @@ -433,6 +434,7 @@ export class API {
return new Promise((resolve, reject) => {
/** @type {?import('core').Timeout} */
let timer = null;
/** @type {import('core').DeferredPromiseDetails<import('api').CreateActionPortResult>} */
const portDetails = deferPromise();

/**
Expand All @@ -441,8 +443,9 @@ export class API {
const onConnect = async (port) => {
try {
const {name: expectedName, id: expectedId} = await portDetails.promise;
const {name, id} = JSON.parse(port.name);
if (name !== expectedName || id !== expectedId || timer === null) { return; }
/** @type {import('cross-frame-api').PortDetails} */
const portDetails2 = parseJson(port.name);
if (portDetails2.name !== expectedName || portDetails2.id !== expectedId || timer === null) { return; }
} catch (e) {
return;
}
Expand Down Expand Up @@ -470,7 +473,9 @@ export class API {
timer = setTimeout(() => onError(new Error('Timeout')), timeout);

chrome.runtime.onConnect.addListener(onConnect);
this._invoke('createActionPort').then(portDetails.resolve, onError);
/** @type {Promise<import('api').CreateActionPortResult>} */
const createActionPortResult = this._invoke('createActionPort');
createActionPortResult.then(portDetails.resolve, onError);
});
}

Expand Down
4 changes: 3 additions & 1 deletion ext/js/comm/cross-frame-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import {EventDispatcher, EventListenerCollection, invokeMessageHandler, log} from '../core.js';
import {ExtensionError} from '../core/extension-error.js';
import {parseJson} from '../core/json.js';
import {yomitan} from '../yomitan.js';

/**
Expand Down Expand Up @@ -377,9 +378,10 @@ export class CrossFrameAPI {
*/
_onConnect(port) {
try {
/** @type {import('cross-frame-api').PortDetails} */
let details;
try {
details = JSON.parse(port.name);
details = parseJson(port.name);
} catch (e) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion ext/js/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export function stringReverse(string) {
}

/**
* Creates a deep clone of an object or value. This is similar to `JSON.parse(JSON.stringify(value))`.
* Creates a deep clone of an object or value. This is similar to `parseJson(JSON.stringify(value))`.
* @template T
* @param {T} value The value to clone.
* @returns {T} A new clone of the value.
Expand Down
40 changes: 40 additions & 0 deletions ext/js/core/json.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright (C) 2023 Yomitan Authors
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/

/**
* This function is used to ensure more safe usage of `JSON.parse`.
* By default, `JSON.parse` returns a value with type `any`, which is easy to misuse.
* By changing the default to `unknown` and allowing it to be templatized,
* this improves how the return type is used.
* @template [T=unknown]
* @param {string} value
* @returns {T}
*/
export function parseJson(value) {
return JSON.parse(value);
}

/**
* This function is used to ensure more safe usage of `Response.json`,
* which returns the `any` type.
* @template [T=unknown]
* @param {Response} response
* @returns {Promise<T>}
*/
export async function readResponseJson(response) {
return await response.json();
}
Loading

0 comments on commit 1ced9aa

Please sign in to comment.