From 5791647cdc91eabe0c5b9e2bd49e9ad72cc0a29d Mon Sep 17 00:00:00 2001 From: Marius Vollmer Date: Wed, 18 Dec 2024 18:24:07 +0200 Subject: [PATCH] WIP - validation --- files.js | 1 + pkg/base1/test-import-json.ts | 264 +++++++++++++++++++++++++++++++++ pkg/lib/import-json.ts | 262 ++++++++++++++++++++++++++++++++ pkg/shell/machines/machines.js | 12 +- pkg/shell/manifests.ts | 71 ++++++++- pkg/shell/state.tsx | 3 +- 6 files changed, 605 insertions(+), 8 deletions(-) create mode 100644 pkg/base1/test-import-json.ts create mode 100644 pkg/lib/import-json.ts diff --git a/files.js b/files.js index 0c8f6e0f5fb3..be95e27a24f9 100644 --- a/files.js +++ b/files.js @@ -78,6 +78,7 @@ const info = { "base1/test-types.ts", "base1/test-user.js", "base1/test-websocket.js", + "base1/test-import-json.ts", "kdump/test-config-client.js", diff --git a/pkg/base1/test-import-json.ts b/pkg/base1/test-import-json.ts new file mode 100644 index 000000000000..c14c50d94d05 --- /dev/null +++ b/pkg/base1/test-import-json.ts @@ -0,0 +1,264 @@ +/* + * This file is part of Cockpit. + * + * Copyright (C) 2024 Red Hat, Inc. + * + * Cockpit is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; either version 2.1 of the License, or + * (at your option) any later version. + * + * Cockpit 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Cockpit; If not, see . + */ + +import { JsonValue } from 'cockpit'; +import { + import_json_object, import_optional, import_mandatory, + import_string, import_number, import_boolean, + import_array, import_record, + validate +} from "import-json"; + +import QUnit from 'qunit-tests'; + +// Hook into console.error + +let console_errors: string[] = []; +const console_error = console.error; +console.error = function (msg) { + console_errors.push(msg); + console_error(msg); +}; + +QUnit.hooks.beforeEach(() => { + console_errors = []; +}); + +QUnit.test("import_string", function(assert) { + assert.equal(import_string("foo"), "foo", "string"); + assert.throws(() => import_string(12), "not a string"); + assert.deepEqual( + console_errors, + [ + 'JSON validation error for : Not a string: 12' + ], + "console errors" + ); +}); + +QUnit.test("import_number", function(assert) { + assert.equal(import_number(12), 12, "number"); + assert.throws(() => import_number("foo"), "not a number"); + assert.deepEqual( + console_errors, + [ + 'JSON validation error for : Not a number: "foo"' + ], + "console errors" + ); +}); + +QUnit.test("import_boolean", function(assert) { + assert.equal(import_boolean(true), true, "boolean"); + assert.throws(() => import_boolean("foo"), "not a boolean"); + assert.deepEqual( + console_errors, + [ + 'JSON validation error for : Not a boolean: "foo"' + ], + "console errors" + ); +}); + +QUnit.test("import_array", function(assert) { + assert.deepEqual(import_array(["a", "b", "c"], import_string), ["a", "b", "c"], "array of strings"); + assert.deepEqual(import_array([1, 2, 3], import_number), [1, 2, 3], "array of numbers"); + assert.deepEqual(import_array([1, 2, "c"], import_number), [1, 2], "array of numbers with a string"); + assert.throws(() => import_array("foo", import_string), "not an array"); + assert.deepEqual( + console_errors, + [ + 'JSON validation error for [2]: Not a number: "c"', + 'JSON validation error for : Not an array: "foo"' + ], + "console errors" + ); +}); + +QUnit.test("import_record", function(assert) { + assert.deepEqual( + import_record({ a: "a", b: "b", c: "c" }, import_string), + { a: "a", b: "b", c: "c" }, + "record of strings"); + assert.deepEqual( + import_record({ a: 1, b: 2, c: 3 }, import_number), + { a: 1, b: 2, c: 3 }, + "record of numbers"); + assert.deepEqual( + import_record({ a: 1, b: 2, c: "c" }, import_number), + { a: 1, b: 2 }, + "record of numbers with a string"); + assert.throws(() => import_record("foo", import_string), "not a record"); + assert.deepEqual( + console_errors, + [ + 'JSON validation error for .c: Not a number: "c"', + 'JSON validation error for : Not an object: "foo"' + ], + "console errors" + ); +}); + +QUnit.test("validate", function(assert) { + assert.equal(validate("test input", "foo", import_string, ""), "foo", "string"); + assert.equal(validate("test input", 12, import_string, ""), "", "not a string"); + assert.deepEqual( + console_errors, + [ + 'JSON validation error for test input: Not a string: 12' + ], + "console errors" + ); +}); + +interface Player { + name: string; + age?: number; +} + +function import_Player(val: JsonValue): Player { + const obj = import_json_object(val); + const res: Player = { + name: import_mandatory(obj, "name", import_string), + }; + import_optional(res, obj, "age", import_number); + return res; +} + +interface Team { + name: string; + players: Player[]; +} + +function import_Team(val: JsonValue): Team { + const obj = import_json_object(val); + const res: Team = { + name: import_mandatory(obj, "name", import_string), + players: import_mandatory(obj, "players", v => import_array(v, import_Player)) + }; + return res; +} + +interface Teams { + [name: string]: Team; +} + +function import_Teams(val: JsonValue): Teams { + return import_record(val, import_Team); +} + +QUnit.test("objects", function(assert) { + const checks: [JsonValue, Teams][] = [ + [ + { + MAC: { + name: "ManCity", + players: [ + { name: "Haaland", age: 24 }, + { name: "De Bruyne", age: 33 } + ], + stadium: "City of Manchester Stadium" + }, + }, + { + MAC: { + name: "ManCity", + players: [ + { name: "Haaland", age: 24 }, + { name: "De Bruyne", age: 33 } + ], + }, + } + + ], + [ + { + MAC: { + name: "ManCity", + players: [ + { name: "Haaland", age: "unknown" }, + { name: "De Bruyne", age: 33 } + ], + stadium: "City of Manchester Stadium" + }, + }, + { + MAC: { + name: "ManCity", + players: [ + { name: "Haaland" }, + { name: "De Bruyne", age: 33 } + ], + }, + } + + ], + [ + { + MAC: { + name: "ManCity", + players: [ + { name: ["Erling", "Braut", "Haaland"] }, + { name: "De Bruyne", age: 33 } + ], + stadium: "City of Manchester Stadium" + }, + }, + { + MAC: { + name: "ManCity", + players: [ + { name: "De Bruyne", age: 33 } + ], + }, + } + ], + [ + { + MAC: { + name: "ManCity", + players: "TBD", + stadium: "City of Manchester Stadium" + }, + }, + {} + ], + [ + "...", + {} + ] + ]; + + for (let i = 0; i < checks.length; i++) { + assert.deepEqual(validate("test input", checks[i][0], import_Teams, {}), checks[i][1]); + } + + assert.deepEqual( + console_errors, + [ + 'JSON validation error for test input.MAC.players[0].age: Not a number: "unknown"', + 'JSON validation error for test input.MAC.players[0].name: Not a string: ["Erling","Braut","Haaland"]', + 'JSON validation error for test input.MAC.players: Not an array: "TBD"', + 'JSON validation error for test input: Not an object: "..."' + ], + "console errors" + ); +}); + +QUnit.start(); diff --git a/pkg/lib/import-json.ts b/pkg/lib/import-json.ts new file mode 100644 index 000000000000..c48a99cc52cc --- /dev/null +++ b/pkg/lib/import-json.ts @@ -0,0 +1,262 @@ +/* + * This file is part of Cockpit. + * + * Copyright (C) 2024 Red Hat, Inc. + * + * Cockpit is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; either version 2.1 of the License, or + * (at your option) any later version. + * + * Cockpit 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 + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Cockpit; If not, see . + */ + +import { JsonValue, JsonObject } from "cockpit"; + +/* GENERIC VALIDATION MACHINERY + + This module helps with turning arbitrary user provided JSON blobs + into well-typed objects. + + The basic idea is that for a TypeScript interface "Foo" you will + write a importer function with this signature: + + function import_Foo(val: JsonValue): Foo; + + This function will either return a valid Foo, or throw a + ValidationError. + + When needing to convert a JSON blob into a Foo, you can call this + function directly. You might need to catch the potential + ValidationError. + + Alternatively, you can also use the "validate" wrapper like so + + const foo = validate("config.foo", config.foo, import_Foo, {}); + + This will include "config.foo" in the error messages to give a + better clue where the invalid data is actually coming from, and + will catch the ValidationError and return a fallback value. + + Validation is generally lenient: If a validation error occurs deep + inside a nested structure, only the affected part of the structure + is omitted. More conretely, if an element of an array is invalid, + this element is omitted from the array. If a optional field of an + object is invalid, it will be omitted. This doesn't happen + silently, of course. In all cases, errors are written to the + browser console. + + For example, given these declarations + + interface Player { + name: string; + age?: number; + } + + interface Team { + name: string; + players: Player[]; + } + + function import_Team(val: JsonObject): Team; + + the following inputs behave as shown: + + { + "name": "ManCity", + "players": [ + { "name": "Haaland", "age": 24 }, + { "name": "De Bruyne", "age", 33 } + ], + "stadium": "City of Manchester Stadium" + } + + This is a fully valid Team and import_Team will return it without + any errors or exceptions. However, the result will not contain the + "stadium" field since Team objects don't have that. + + { + "name": "ManCity", + "players": [ + { "name": "Haaland", "age": "unknown" }, + { "name": "De Bruyne", "age", 33 } + ] + } + + The "age" field of Haaland is not a number, but it is + optional. The import_Team function will log an error and will omit + the "age" field from the Player object for Håland. + + { + "name": "ManCity", + "players": [ + { "name": [ "Erling", "Braut", "Haaland" ], "age": 24 }, + { "name": "De Bruyne", "age", 33 } + ] + } + + The "name" field for Håland is not a string, and it is + mandatory. An error will be logged and the whole Håland entry is + omitted from "players". + + { + "name": "ManCity", + "players": "TBD" + } + + The "players" field is not an array, and it is mandatory. The + import_Team function will raise a ValidationError exception. + */ + +/* WRITING IMPORTER FUNCTIONS + + The process of writing a importer function for a given TypeScript + interface is pretty mechanic, and could well be automated. + + For example, these are the functions for the Player and Team + interfaces from above. + + interface Player { + name: string; + age?: number; + } + + function import_Player(val: JsonValue): Player { + const obj = import_json_object(val); + const res: Player = { + name: import_mandatory(obj, "name", import_string), + }; + import_optional(res, obj, "age", import_number); + return res; + } + + interface Team { + name: string; + players: Player[]; + } + + function import_Team(val: JsonValue): Team { + const obj = import_json_object(val); + const res: Team = { + name: import_mandatory(obj, "name", import_string), + players: import_mandatory(obj, "players", v => import_array(v, import_Player)) + }; + return res; + } + + More examples can be found in "pkg/shell/manifests.ts". + */ + +class ValidationError extends Error { } + +const validation_path: string[] = []; + +function with_validation_path(p: string, func: () => T): T { + validation_path.push(p); + try { + return func(); + } finally { + validation_path.pop(); + } +} + +function validation_error(msg: string): never { + console.error(`JSON validation error for ${validation_path.join("")}: ${msg}`); + throw new ValidationError(); +} + +export function import_string(val: JsonValue): string { + if (typeof val == "string") + return val; + validation_error(`Not a string: ${JSON.stringify(val)}`); +} + +export function import_number(val: JsonValue): number { + if (typeof val == "number") + return val; + validation_error(`Not a number: ${JSON.stringify(val)}`); +} + +export function import_boolean(val: JsonValue): boolean { + if (typeof val == "boolean") + return val; + validation_error(`Not a boolean: ${JSON.stringify(val)}`); +} + +export function import_json_object(val: JsonValue): JsonObject { + if (!!val && typeof val == "object" && val.length === undefined) + return val as JsonObject; + validation_error(`Not an object: ${JSON.stringify(val)}`); +} + +export function import_json_array(val: JsonValue): JsonValue[] { + if (!!val && typeof val == "object" && val.length !== undefined) + return val as JsonValue[]; + validation_error(`Not an array: ${JSON.stringify(val)}`); +} + +export function import_record(val: JsonValue, importer: (val: JsonValue) => T): Record { + const obj = import_json_object(val); + const res: Record = {}; + for (const key of Object.keys(obj)) { + try { + with_validation_path(`.${key}`, () => { res[key] = importer(obj[key]) }); + } catch (e) { + if (!(e instanceof ValidationError)) + throw e; + } + } + return res; +} + +export function import_array(val: JsonValue, importer: (val: JsonValue) => T): Array { + const arr = import_json_array(val); + const res: Array = []; + for (let i = 0; i < arr.length; i++) { + try { + with_validation_path(`[${i}]`, () => { res.push(importer(arr[i])) }); + } catch (e) { + if (!(e instanceof ValidationError)) + throw e; + } + } + return res; +} + +export function import_optional(res: T, obj: JsonObject, field: F, importer: (val: JsonValue) => T[F]): void { + if (obj[field as string] === undefined) + return; + + try { + with_validation_path(`.${String(field)}`, () => { res[field] = importer(obj[field]) }); + } catch (e) { + if (!(e instanceof ValidationError)) + throw e; + } +} + +export function import_mandatory(obj: JsonObject, field: string, importer: (val: JsonValue) => T): T { + if (obj[field as string] === undefined) { + validation_error(`Field "${String(field)}" is missing`); + } + return with_validation_path(`.${String(field)}`, () => importer(obj[field])); +} + +export function validate(path: string, val: JsonValue | undefined, importer: (val: JsonValue) => T, fallback: T): T { + if (val === undefined) + return fallback; + + try { + return with_validation_path(path, () => importer(val)); + } catch (e) { + if (!(e instanceof ValidationError)) + throw e; + return fallback; + } +} diff --git a/pkg/shell/machines/machines.js b/pkg/shell/machines/machines.js index ac6275f64c61..b540f19930c1 100644 --- a/pkg/shell/machines/machines.js +++ b/pkg/shell/machines/machines.js @@ -1,5 +1,6 @@ import cockpit from "cockpit"; import { import_Manifests } from "../manifests"; +import { validate } from "import-json"; import ssh_add_key_sh from "../../lib/ssh-add-key.sh"; @@ -110,6 +111,11 @@ export function split_connection_string (conn_to) { return parts; } +function import_manifests(val) { + return validate("manifests", val, import_Manifests, {}); +} + + function Machines() { const self = this; @@ -127,7 +133,7 @@ function Machines() { overlay: { localhost: { visible: true, - manifests: cockpit.manifests + manifests: import_manifests(cockpit.manifests) } } }; @@ -573,7 +579,7 @@ function Loader(machines, session_only) { request.responseType = "json"; request.open("GET", url, true); request.addEventListener("load", () => { - const overlay = { manifests: import_Manifests(request.response) }; + const overlay = { manifests: import_manifests(request.response) }; const etag = request.getResponseHeader("ETag"); if (etag) /* and remove quotes */ overlay.checksum = etag.replace(/^"(.+)"$/, '$1'); @@ -611,7 +617,7 @@ function Loader(machines, session_only) { if (args[0] == "cockpit.Packages") { if (args[1].Manifests) { const manifests = JSON.parse(args[1].Manifests.v); - machines.overlay(host, { manifests: import_Manifests(manifests) }); + machines.overlay(host, { manifests: import_manifests(manifests) }); } } }); diff --git a/pkg/shell/manifests.ts b/pkg/shell/manifests.ts index e87cf8491dd2..7422884adf04 100644 --- a/pkg/shell/manifests.ts +++ b/pkg/shell/manifests.ts @@ -19,6 +19,12 @@ import { JsonValue } from "cockpit"; +import { + import_json_object, + import_string, import_number, import_boolean, import_record, import_array, + import_optional, import_mandatory +} from "import-json"; + export interface ManifestKeyword { matches: string[]; goto?: string; @@ -26,11 +32,31 @@ export interface ManifestKeyword { translate?: boolean; } +function import_ManifestKeyword(val: JsonValue): ManifestKeyword { + const obj = import_json_object(val); + const res: ManifestKeyword = { + matches: import_mandatory(obj, "matches", v => import_array(v, import_string)), + }; + import_optional(res, obj, "goto", import_string); + import_optional(res, obj, "weight", import_number); + import_optional(res, obj, "translate", import_boolean); + return res; +} + export interface ManifestDocs { label: string; url: string; } +function import_ManifestDocs(val: JsonValue): ManifestDocs { + const obj = import_json_object(val); + const res: ManifestDocs = { + label: import_mandatory(obj, "label", import_string), + url: import_mandatory(obj, "url", import_string), + }; + return res; +} + export interface ManifestEntry { path?: string; label?: string; @@ -39,15 +65,38 @@ export interface ManifestEntry { keywords?: ManifestKeyword[]; } +function import_ManifestEntry(val: JsonValue): ManifestEntry { + const obj = import_json_object(val); + const res: ManifestEntry = { }; + import_optional(res, obj, "path", import_string); + import_optional(res, obj, "label", import_string); + import_optional(res, obj, "order", import_number); + import_optional(res, obj, "docs", v => import_array(v, import_ManifestDocs)); + import_optional(res, obj, "keywords", v => import_array(v, import_ManifestKeyword)); + return res; +} + export interface ManifestSection { [name: string]: ManifestEntry; } +function import_ManifestSection(val: JsonValue): ManifestSection { + return import_record(val, import_ManifestEntry); +} + export interface ManifestParentSection { component?: string; docs?: ManifestDocs[]; } +function import_ManifestParentSection(val: JsonValue): ManifestParentSection { + const obj = import_json_object(val); + const res: ManifestParentSection = { }; + import_optional(res, obj, "component", import_string); + import_optional(res, obj, "docs", v => import_array(v, import_ManifestDocs)); + return res; +} + export interface Manifest { dashboard?: ManifestSection; menu?: ManifestSection; @@ -58,13 +107,24 @@ export interface Manifest { ".checksum"?: string; } +function import_Manifest(val: JsonValue): Manifest { + const obj = import_json_object(val); + const res: Manifest = { }; + import_optional(res, obj, "dashboard", import_ManifestSection); + import_optional(res, obj, "menu", import_ManifestSection); + import_optional(res, obj, "tools", import_ManifestSection); + import_optional(res, obj, "preload", v => import_array(v, import_string)); + import_optional(res, obj, "parent", import_ManifestParentSection); + import_optional(res, obj, ".checksum", import_string); + return res; +} + export interface Manifests { [pkg: string]: Manifest; } export function import_Manifests(val: JsonValue): Manifests { - // TODO - validate against schema - return val as unknown as Manifests; + return import_record(val, import_Manifest); } export interface ShellManifest { @@ -73,6 +133,9 @@ export interface ShellManifest { } export function import_ShellManifest(val: JsonValue): ShellManifest { - // TODO - validate against schema - return val as unknown as ShellManifest; + const obj = import_json_object(val); + const res: ShellManifest = { }; + import_optional(res, obj, "docs", v => import_array(v, import_ManifestDocs)); + import_optional(res, obj, "locales", v => import_record(v, import_string)); + return res; } diff --git a/pkg/shell/state.tsx b/pkg/shell/state.tsx index 10bfac125604..73544ee960b4 100644 --- a/pkg/shell/state.tsx +++ b/pkg/shell/state.tsx @@ -33,6 +33,7 @@ import { Location, ManifestItem, CompiledComponents, } from "./util.jsx"; import { Manifest, ShellManifest, import_ShellManifest } from "./manifests"; +import { validate } from "import-json"; export interface ShellConfig { language: string; @@ -110,7 +111,7 @@ export class ShellState extends EventEmitter { language, language_direction: cockpit.language_direction, host_switcher_enabled: false, - manifest: import_ShellManifest(cockpit.manifests.shell || {}), + manifest: validate("manifests.shell", cockpit.manifests.shell, import_ShellManifest, {}), }; /* Host switcher enabled? */