From 753fa225ed090a357e26058c72a580460c6ad05f Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Mon, 25 Apr 2022 14:10:39 -0700 Subject: [PATCH] Fix crash in CRD display (#5256) --- package.json | 3 +- .../crd-resource-details.tsx | 8 +- .../+custom-resources/crd-resources.tsx | 26 +-- .../utils/__tests__/jsonPath.test.tsx | 162 +++++++++++++++--- src/renderer/utils/jsonPath.ts | 87 ++++++++-- yarn.lock | 46 +++-- 6 files changed, 262 insertions(+), 70 deletions(-) diff --git a/package.json b/package.json index eb72608be7..a78199c1b7 100644 --- a/package.json +++ b/package.json @@ -199,6 +199,7 @@ } }, "dependencies": { + "@astronautlabs/jsonpath": "^1.1.0", "@hapi/call": "^8.0.1", "@hapi/subtext": "^7.0.3", "@kubernetes/client-node": "^0.16.3", @@ -229,7 +230,6 @@ "joi": "^17.6.0", "js-yaml": "^4.1.0", "jsdom": "^16.7.0", - "jsonpath": "^1.1.1", "lodash": "^4.17.15", "mac-ca": "^1.0.6", "marked": "^4.0.12", @@ -299,7 +299,6 @@ "@types/jest": "^26.0.24", "@types/js-yaml": "^4.0.5", "@types/jsdom": "^16.2.14", - "@types/jsonpath": "^0.2.0", "@types/lodash": "^4.14.181", "@types/marked": "^4.0.3", "@types/md5-file": "^4.0.2", diff --git a/src/renderer/components/+custom-resources/crd-resource-details.tsx b/src/renderer/components/+custom-resources/crd-resource-details.tsx index 6ac5d61ed2..1c43769e2e 100644 --- a/src/renderer/components/+custom-resources/crd-resource-details.tsx +++ b/src/renderer/components/+custom-resources/crd-resource-details.tsx @@ -6,7 +6,6 @@ import "./crd-resource-details.scss"; import React from "react"; -import jsonPath from "jsonpath"; import { observer } from "mobx-react"; import { cssNames } from "../../utils"; import { Badge } from "../badge"; @@ -16,10 +15,11 @@ import { KubeObjectMeta } from "../kube-object-meta"; import { Input } from "../input"; import type { AdditionalPrinterColumnsV1 } from "../../../common/k8s-api/endpoints/crd.api"; import { CustomResourceDefinition } from "../../../common/k8s-api/endpoints/crd.api"; -import { parseJsonPath } from "../../utils/jsonPath"; +import { convertKubectlJsonPathToNodeJsonPath } from "../../utils/jsonPath"; import type { KubeObjectMetadata, KubeObjectStatus } from "../../../common/k8s-api/kube-object"; import { KubeObject } from "../../../common/k8s-api/kube-object"; import logger from "../../../common/logger"; +import { JSONPath } from "@astronautlabs/jsonpath"; export interface CustomResourceDetailsProps extends KubeObjectDetailsProps { crd: CustomResourceDefinition; @@ -48,9 +48,9 @@ function convertSpecValue(value: any): any { @observer export class CustomResourceDetails extends React.Component { renderAdditionalColumns(resource: KubeObject, columns: AdditionalPrinterColumnsV1[]) { - return columns.map(({ name, jsonPath: jp }) => ( + return columns.map(({ name, jsonPath }) => ( - {convertSpecValue(jsonPath.value(resource, parseJsonPath(jp.slice(1))))} + {convertSpecValue(JSONPath.query(resource, convertKubectlJsonPathToNodeJsonPath(jsonPath)))} )); } diff --git a/src/renderer/components/+custom-resources/crd-resources.tsx b/src/renderer/components/+custom-resources/crd-resources.tsx index 528f4ea0d9..08d56488bd 100644 --- a/src/renderer/components/+custom-resources/crd-resources.tsx +++ b/src/renderer/components/+custom-resources/crd-resources.tsx @@ -6,14 +6,13 @@ import "./crd-resources.scss"; import React from "react"; -import { value } from "jsonpath"; import { observer } from "mobx-react"; import { KubeObjectListLayout } from "../kube-object-list-layout"; import type { IComputedValue } from "mobx"; import { computed, makeObservable } from "mobx"; import { crdStore } from "./crd.store"; import { apiManager } from "../../../common/k8s-api/api-manager"; -import { parseJsonPath } from "../../utils/jsonPath"; +import { safeJSONPathValue } from "../../utils/jsonPath"; import { TabLayout } from "../layout/tab-layout-2"; import { withInjectables } from "@ogre-tools/injectable-react"; import customResourcesRouteParametersInjectable from "./custom-resources-route-parameters.injectable"; @@ -70,7 +69,7 @@ class NonInjectedCrdResources extends React.Component { [columnId.age]: customResource => -customResource.getCreationTimestamp(), ...Object.fromEntries(extraColumns.map(({ name, jsonPath }) => [ name, - customResource => value(customResource, parseJsonPath(jsonPath.slice(1))), + customResource => safeJSONPathValue(customResource, jsonPath), ])), }} searchFilters={[ @@ -95,22 +94,11 @@ class NonInjectedCrdResources extends React.Component { })), { title: "Age", className: "age", sortBy: columnId.age, id: columnId.age }, ]} - renderTableContents={crdInstance => [ - crdInstance.getName(), - isNamespaced && crdInstance.getNs(), - ...extraColumns.map((column) => { - let rawValue = value(crdInstance, parseJsonPath(column.jsonPath.slice(1))); - - if (Array.isArray(rawValue) || typeof rawValue === "object") { - rawValue = JSON.stringify(rawValue); - } - - return { - renderBoolean: true, - children: rawValue, - }; - }), - , + renderTableContents={customResource => [ + customResource.getName(), + isNamespaced && customResource.getNs(), + ...extraColumns.map((column) => safeJSONPathValue(customResource, column.jsonPath)), + , ]} failedToLoadMessage={( <> diff --git a/src/renderer/utils/__tests__/jsonPath.test.tsx b/src/renderer/utils/__tests__/jsonPath.test.tsx index dd8cfa86b8..db5e64d208 100644 --- a/src/renderer/utils/__tests__/jsonPath.test.tsx +++ b/src/renderer/utils/__tests__/jsonPath.test.tsx @@ -3,44 +3,166 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ -import { parseJsonPath } from "../jsonPath"; +import { convertKubectlJsonPathToNodeJsonPath, safeJSONPathValue } from "../jsonPath"; describe("parseJsonPath", () => { - test("should convert \\. to use indexed notation", () => { - const res = parseJsonPath(".metadata.labels.kubesphere\\.io/alias-name"); + it("should convert \\. to use indexed notation", () => { + const res = convertKubectlJsonPathToNodeJsonPath(".metadata.labels.kubesphere\\.io/alias-name"); - expect(res).toBe(".metadata.labels['kubesphere.io/alias-name']"); + expect(res).toBe("$.metadata.labels['kubesphere.io/alias-name']"); }); - test("should convert keys with escaped characters to use indexed notation", () => { - const res = parseJsonPath(".metadata.labels.kubesphere\\\"io/alias-name"); + it("should convert keys with escaped characters to use indexed notation", () => { + const res = convertKubectlJsonPathToNodeJsonPath(".metadata.labels.kubesphere\\\"io/alias-name"); - expect(res).toBe(".metadata.labels['kubesphere\"io/alias-name']"); + expect(res).toBe("$.metadata.labels['kubesphere\"io/alias-name']"); }); - test("should convert '-' to use indexed notation", () => { - const res = parseJsonPath(".metadata.labels.alias-name"); + it("should convert '-' to use indexed notation", () => { + const res = convertKubectlJsonPathToNodeJsonPath(".metadata.labels.alias-name"); - expect(res).toBe(".metadata.labels['alias-name']"); + expect(res).toBe("$.metadata.labels['alias-name']"); }); - test("should handle scenario when both \\. and indexed notation are present", () => { - const rest = parseJsonPath(".metadata.labels\\.serving['some.other.item']"); + it("should drop leading dot if first group is converted to index notation", () => { + const res = convertKubectlJsonPathToNodeJsonPath(".metadata\\.labels.alias-name"); - expect(rest).toBe(".metadata['labels.serving']['some.other.item']"); + expect(res).toBe("$['metadata.labels']['alias-name']"); }); + it("should handle scenario when both \\. and indexed notation are present", () => { + const rest = convertKubectlJsonPathToNodeJsonPath(".metadata.labels\\.serving['some.other.item']"); - test("should not touch given jsonPath if no invalid characters present", () => { - const res = parseJsonPath(".status.conditions[?(@.type=='Ready')].status"); - - expect(res).toBe(".status.conditions[?(@.type=='Ready')].status"); + expect(rest).toBe("$.metadata['labels.serving']['some.other.item']"); }); - test("strips '\\' away from the result", () => { - const res = parseJsonPath(".metadata.labels['serving\\.knative\\.dev/configuration']"); + it("should not touch given jsonPath if no invalid characters present", () => { + const res = convertKubectlJsonPathToNodeJsonPath(".status.conditions[?(@.type=='Ready')].status"); - expect(res).toBe(".metadata.labels['serving.knative.dev/configuration']"); + expect(res).toBe("$.status.conditions[?(@.type=='Ready')].status"); }); + it("strips '\\' away from the result", () => { + const res = convertKubectlJsonPathToNodeJsonPath(".metadata.labels['serving\\.knative\\.dev/configuration']"); + + expect(res).toBe("$.metadata.labels['serving.knative.dev/configuration']"); + }); + + it("converts all [] to [0]", () => { + const res = convertKubectlJsonPathToNodeJsonPath(".metadata.labels[].foo[]"); + + expect(res).toBe("$.metadata.labels[0].foo[0]"); + }); + + it("converts ending .. to ..*", () => { + const res = convertKubectlJsonPathToNodeJsonPath(".metadata.labels[].."); + + expect(res).toBe("$.metadata.labels[0]"); + }); + + it("converts ending ...name to ..name", () => { + const res = convertKubectlJsonPathToNodeJsonPath(".metadata.labels[]...name"); + + expect(res).toBe("$.metadata.labels[0]..name"); + }); +}); + +describe("safeJSONPathValue", () => { + let oldWarn: typeof console["warn"]; + + beforeEach(() => { + oldWarn = console.warn; + console.warn = jest.fn(); + }); + + afterEach(() => { + console.warn = oldWarn; + }); + + it("should convert boolean values to strings", () => { + const res = safeJSONPathValue({ bar: false }, ".bar"); + + expect(res).toBe("false"); + }); + + it("should convert number values to strings", () => { + const res = safeJSONPathValue({ bar: 0 }, ".bar"); + + expect(res).toBe("0"); + }); + + it("should join sliced entries with commas only", () => { + const res = safeJSONPathValue({ + bar: [ + { + foo: 1, + }, + { + foo: "hello", + }, + ], + }, ".bar[].foo"); + + expect(res).toBe("1"); + }); + + it("should join an array of values using JSON.stringify", () => { + const res = safeJSONPathValue({ + bar: [ + "world", + "hello", + ], + }, ".bar"); + + expect(res).toBe(`["world","hello"]`); + }); + + it("should stringify an object value", () => { + const res = safeJSONPathValue({ + foo: { bar: "bat" }, + }, ".foo"); + + expect(res).toBe(`{"bar":"bat"}`); + }); + + it("should use convertKubectlJsonPathToNodeJsonPath", () => { + const res = safeJSONPathValue({ + foo: { "hello.world": "bat" }, + }, ".foo.hello\\.world"); + + expect(res).toBe("bat"); + }); + + it("should not throw when given '.spec.metrics[*].external.highWatermark..'", () => { + const obj = { + spec: { + metrics: [ + { + external: { + metricName: "cpu", + highWatermark: "100", + }, + }, + { + external: { + metricName: "memory", + highWatermark: "100", + }, + }, + ], + }, + }; + + const res = safeJSONPathValue(obj, ".spec.metrics[*].external.highWatermark.."); + + expect(res).toBe("100, 100"); + }); + + it("should not throw if path is invalid jsonpath", () => { + const res = safeJSONPathValue({ + foo: { "hello.world": "bat" }, + }, "asd["); + + expect(res).toBe(""); + }); }); diff --git a/src/renderer/utils/jsonPath.ts b/src/renderer/utils/jsonPath.ts index 6bbf0c7a24..3aedbd0919 100644 --- a/src/renderer/utils/jsonPath.ts +++ b/src/renderer/utils/jsonPath.ts @@ -3,28 +3,63 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ -// Helper to convert strings used for jsonPath where \. or - is present to use indexed notation, -// for example: .metadata.labels.kubesphere\.io/alias-name -> .metadata.labels['kubesphere\.io/alias-name'] +import { JSONPath } from "@astronautlabs/jsonpath"; -export function parseJsonPath(jsonPath: string) { - let pathExpression = jsonPath; +const slashDashSearch = /[\\-]/g; +const pathByBareDots = /(?<=\w)\./; +const textBeforeFirstSquare = /^.*(?=\[)/g; +const backSlash = /\\/g; +const kubectlOptionPrefix = /^\$?\.?(?.*)/; +const sliceVersion = /\[]/g; +const tripleDotName = /\.\.\.(?.)/g; +const trailingDotDot = /\.\.$/; - if (jsonPath.match(/[\\-]/g)) { // search for '\' and '-' - const [first, ...rest] = jsonPath.split(/(?<=\w)\./); // split jsonPath by '.' (\. cases are ignored) +/** + * The GO package that kubectl and kubernetes uses for its JSONpath implementation has some + * shorthand conveniences that are not part of the official spec. This function tries to convert + * those shorthands to the official spec. + * + * Known shorthands: + * - Leading `$` is optional (but implied) + * - The string `\.` is used to denote the "value of '.'" and not "next key" + * - The string `-` can be used while not in quotes + * - `[]` as shorthand for `[0]` + * - Remove `..` at the end of a path, we will just format it slightly differently + * - Allow `...foo` as well as `..foo` + */ +export function convertKubectlJsonPathToNodeJsonPath(jsonPath: string) { + const startMatch = jsonPath.match(kubectlOptionPrefix); + let start = "$"; + + if (!startMatch) { + return start; + } + + let { pathExpression } = startMatch.groups; + + if (pathExpression.match(slashDashSearch)) { + const [first, ...rest] = pathExpression.split(pathByBareDots); pathExpression = `${convertToIndexNotation(first, true)}${rest.map(value => convertToIndexNotation(value)).join("")}`; } + pathExpression = pathExpression.replace(trailingDotDot, ""); + pathExpression = pathExpression.replace(tripleDotName, "..$"); + + if (!pathExpression.startsWith("[")) { + start += "."; + } + // strip '\' characters from the result - return pathExpression.replace(/\\/g, ""); + return `${start}${pathExpression.replace(backSlash, "").replace(sliceVersion, "[0]")}`; } function convertToIndexNotation(key: string, firstItem = false) { - if (key.match(/[\\-]/g)) { // check if found '\' and '-' in key + if (key.match(slashDashSearch)) { if (key.includes("[")) { // handle cases where key contains [...] - const keyToConvert = key.match(/^.*(?=\[)/g); // get the text from the key before '[' + const keyToConvert = key.match(textBeforeFirstSquare); // get the text from the key before '[' - if (keyToConvert && keyToConvert[0].match(/[\\-]/g)) { // check if that part contains illegal characters + if (keyToConvert && keyToConvert[0].match(slashDashSearch)) { // check if that part contains illegal characters return key.replace(keyToConvert[0], `['${keyToConvert[0]}']`); // surround key with '[' and ']' } else { return `.${key}`; // otherwise return as is with leading '.' @@ -38,3 +73,35 @@ function convertToIndexNotation(key: string, firstItem = false) { return `${prefix}${key}`; } } + +function formatJSONValue(value: unknown) { + if (typeof value === "object") { + return JSON.stringify(value); + } + + return String(value); +} + +/** + * This function is a safer version of `JSONPath.value(obj, path)` with untrusted jsonpath strings + * + * This function will also stringify the value retreived from the object + */ +export function safeJSONPathValue(obj: object, path: string): string { + try { + const parsedPath = JSONPath.parse(convertKubectlJsonPathToNodeJsonPath(path)); + const isSlice = parsedPath.some((exp: any) => exp.expression.type === "slice" || "wildcard"); + const value = JSONPath.query(obj, JSONPath.stringify(parsedPath), isSlice ? Infinity : 1); + + if (isSlice) { + return value.map(formatJSONValue).join(", "); + } + + return formatJSONValue(value[0]); + } catch (error) { + // something failed + console.warn("[JSON-PATH]: failed to parse jsonpath", error); + + return ""; + } +} diff --git a/yarn.lock b/yarn.lock index 6f3264ae45..612dc8281b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7,6 +7,17 @@ resolved "https://registry.yarnpkg.com/7zip-bin/-/7zip-bin-5.1.1.tgz#9274ec7460652f9c632c59addf24efb1684ef876" integrity sha512-sAP4LldeWNz0lNzmTird3uWfFDWWTeg6V/MsmyyLR9X1idwKBWIgt/ZvinqQldJm3LecKEs1emkbquO6PCiLVQ== +"@astronautlabs/jsonpath@^1.1.0": + version "1.1.0" + resolved "https://registry.yarnpkg.com/@astronautlabs/jsonpath/-/jsonpath-1.1.0.tgz#9b4b04603be2e858d0763619ab3f9d1050b52ef3" + integrity sha512-I4sckUQNYEeF4w6AkujiRblVzC0jo3ja+bxKHB1M2aFiaBjevSGlBdgZ80PgCX5sj65/AhKSFN9+cT908avBlQ== + dependencies: + "@types/esprima" "^4.0.2" + "@types/mkdirp" "^1.0.0" + esprima "1.2.2" + static-eval "2.0.2" + underscore "1.7.0" + "@async-fn/jest@1.5.3": version "1.5.3" resolved "https://registry.yarnpkg.com/@async-fn/jest/-/jest-1.5.3.tgz#42be6c0e8ba5ccd737e006ca600e7e319fe2a591" @@ -1449,6 +1460,13 @@ "@types/estree" "*" "@types/json-schema" "*" +"@types/esprima@^4.0.2": + version "4.0.3" + resolved "https://registry.yarnpkg.com/@types/esprima/-/esprima-4.0.3.tgz#e9068297cc3dd75231fa5cdaa6d75c50d5fb632f" + integrity sha512-jo14dIWVVtF0iMsKkYek6++4cWJjwpvog+rchLulwgFJGTXqIeTdCOvY0B3yMLTaIwMcKCdJ6mQbSR6wYHy98A== + dependencies: + "@types/estree" "*" + "@types/estree@*", "@types/estree@^0.0.51": version "0.0.51" resolved "https://registry.yarnpkg.com/@types/estree/-/estree-0.0.51.tgz#cfd70924a25a3fd32b218e5e420e6897e1ac4f40" @@ -1611,11 +1629,6 @@ resolved "https://registry.yarnpkg.com/@types/json5/-/json5-0.0.29.tgz#ee28707ae94e11d2b827bcbe5270bcea7f3e71ee" integrity sha1-7ihweulOEdK4J7y+UnC86n8+ce4= -"@types/jsonpath@^0.2.0": - version "0.2.0" - resolved "https://registry.yarnpkg.com/@types/jsonpath/-/jsonpath-0.2.0.tgz#13c62db22a34d9c411364fac79fd374d63445aa1" - integrity sha512-v7qlPA0VpKUlEdhghbDqRoKMxFB3h3Ch688TApBJ6v+XLDdvWCGLJIYiPKGZnS6MAOie+IorCfNYVHOPIHSWwQ== - "@types/keyv@*": version "3.1.1" resolved "https://registry.yarnpkg.com/@types/keyv/-/keyv-3.1.1.tgz#e45a45324fca9dab716ab1230ee249c9fb52cfa7" @@ -1662,6 +1675,13 @@ dependencies: "@types/node" "*" +"@types/mkdirp@^1.0.0": + version "1.0.2" + resolved "https://registry.yarnpkg.com/@types/mkdirp/-/mkdirp-1.0.2.tgz#8d0bad7aa793abe551860be1f7ae7f3198c16666" + integrity sha512-o0K1tSO0Dx5X6xlU5F1D6625FawhC3dU3iqr25lluNv/+/QIVH8RLNEiVokgIZo+mz+87w/3Mkg/VvQS+J51fQ== + dependencies: + "@types/node" "*" + "@types/mock-fs@^4.13.1": version "4.13.1" resolved "https://registry.yarnpkg.com/@types/mock-fs/-/mock-fs-4.13.1.tgz#9201554ceb23671badbfa8ac3f1fa9e0706305be" @@ -8219,15 +8239,6 @@ jsonpath-plus@^0.19.0: resolved "https://registry.yarnpkg.com/jsonpath-plus/-/jsonpath-plus-0.19.0.tgz#b901e57607055933dc9a8bef0cc25160ee9dd64c" integrity sha512-GSVwsrzW9LsA5lzsqe4CkuZ9wp+kxBb2GwNniaWzI2YFn5Ig42rSW8ZxVpWXaAfakXNrx5pgY5AbQq7kzX29kg== -jsonpath@^1.1.1: - version "1.1.1" - resolved "https://registry.yarnpkg.com/jsonpath/-/jsonpath-1.1.1.tgz#0ca1ed8fb65bb3309248cc9d5466d12d5b0b9901" - integrity sha512-l6Cg7jRpixfbgoWgkrl77dgEj8RPvND0wMH6TwQmi9Qs4TFfS9u5cUFnbeKTwj5ga5Y3BTGGNI28k117LJ009w== - dependencies: - esprima "1.2.2" - static-eval "2.0.2" - underscore "1.12.1" - jsprim@^1.2.2: version "1.4.1" resolved "https://registry.yarnpkg.com/jsprim/-/jsprim-1.4.1.tgz#313e66bc1e5cc06e438bc1b7499c2e5c56acb6a2" @@ -13202,7 +13213,12 @@ undefsafe@^2.0.5: resolved "https://registry.yarnpkg.com/undefsafe/-/undefsafe-2.0.5.tgz#38733b9327bdcd226db889fb723a6efd162e6e2c" integrity sha512-WxONCrssBM8TSPRqN5EmsjVrsv4A8X12J4ArBiiayv3DyyG3ZlIg6yysuuSYdZsVz3TKcTg2fd//Ujd4CHV1iA== -underscore@1.12.1, underscore@^1.9.1: +underscore@1.7.0: + version "1.7.0" + resolved "https://registry.yarnpkg.com/underscore/-/underscore-1.7.0.tgz#6bbaf0877500d36be34ecaa584e0db9fef035209" + integrity sha1-a7rwh3UA02vjTsqlhODbn+8DUgk= + +underscore@^1.9.1: version "1.12.1" resolved "https://registry.yarnpkg.com/underscore/-/underscore-1.12.1.tgz#7bb8cc9b3d397e201cf8553336d262544ead829e" integrity sha512-hEQt0+ZLDVUMhebKxL4x1BTtDY7bavVofhZ9KZ4aI26X9SRaE+Y3m83XUL1UP2jn8ynjndwCCpEHdUG+9pP1Tw==