From c3fd60759ec4a53699790b402f3691d4613161fd Mon Sep 17 00:00:00 2001 From: Marius Vollmer Date: Mon, 16 Dec 2024 17:01:56 +0200 Subject: [PATCH] WIP - validation --- pkg/lib/import-json.ts | 267 +++++++++++++++++++++++++++++++++ pkg/shell/machines/machines.js | 12 +- pkg/shell/manifests.ts | 72 ++++++++- pkg/shell/state.tsx | 3 +- 4 files changed, 345 insertions(+), 9 deletions(-) create mode 100644 pkg/lib/import-json.ts diff --git a/pkg/lib/import-json.ts b/pkg/lib/import-json.ts new file mode 100644 index 000000000000..703eaaae8f40 --- /dev/null +++ b/pkg/lib/import-json.ts @@ -0,0 +1,267 @@ +/* + * 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 data is actually coming from that is invalid, + 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 (or a + dictionary) is invalid, this element is omitted from the array (or + dictionary). 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 } + ], + "stadiun": "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. import_Team 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 since it is a + mandatory field in a Player object, the whole entry is omitted + from "players". + + { + "name": "ManCity", + "players": "TBD" + } + + The "players" field is not an array. import_Team will raise a + ValidationError exception. + + [...] + + XXX - what about default values like `interface Team { name: string = "-", ... }`? + + */ + +/* 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, this is the function for the Player interface 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; + } + + [...] + */ + +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 704a5295174b..956c04eb5822 100644 --- a/pkg/shell/manifests.ts +++ b/pkg/shell/manifests.ts @@ -17,7 +17,12 @@ * along with Cockpit; If not, see . */ -import { JsonValue, JsonObject } from "cockpit"; +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[]; @@ -26,11 +31,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 +64,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 +106,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 +132,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 afc8066464b1..eb422e8d7f8d 100644 --- a/pkg/shell/state.tsx +++ b/pkg/shell/state.tsx @@ -32,6 +32,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; @@ -117,7 +118,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? */