From 5285d34cc019c77f7eee2e00a2afd8ff6e84a606 Mon Sep 17 00:00:00 2001
From: AJ Tomko <andrew.j.tomko@gmail.com>
Date: Wed, 19 Oct 2022 08:59:24 -0400
Subject: [PATCH] Dashboard: Alerts user to incorrect tag format for JSON
import (#54657)
* Dashboard: Alerts user to incorrect tag format for JSON import
Fixes #54285: Malformed tags cause hidden title and settings page crash
* Update public/app/features/manage-dashboards/utils/validation.ts
Co-authored-by: Polina Boneva <13227501+polibb@users.noreply.github.com>
* Included Suggestions
- Removed Comments
- Updated Code Block accordingly
- Updated Tests to camelCase over snake_case
* Updates per comments
- Re-wrapped function in try{}, catch{} as I appear to have overlooked including it in the initial refactor
- Re-worded errors to align with initial error
- Added a test case for invalid json
* Update validation.ts
Updated errors to read correctly to the root cause.
Updated dashboard variable as const.
* Update actions.test.ts
Fix tests according to error output rewording
* Update validation.ts
- Included test for an empty string of non-array
* Update actions.test.ts
-- Commented incorrect commit for validation.ts, update:
- Refactored code to better align and separate from generic JSON package tests followed by our manual checks of (1) Is array, and (2) if array, is of strings
- Test cases now include a check for non-array empty string in the tag property
Co-authored-by: Polina Boneva <13227501+polibb@users.noreply.github.com>
---
.../manage-dashboards/state/actions.test.ts | 31 +++++++++++++++++++
.../manage-dashboards/utils/validation.ts | 15 +++++++--
2 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/public/app/features/manage-dashboards/state/actions.test.ts b/public/app/features/manage-dashboards/state/actions.test.ts
index d6c366ed545..c37192e18ee 100644
--- a/public/app/features/manage-dashboards/state/actions.test.ts
+++ b/public/app/features/manage-dashboards/state/actions.test.ts
@@ -2,6 +2,8 @@ import { thunkTester } from 'test/core/thunk/thunkTester';
import { setBackendSrv } from '@grafana/runtime';
+import { validateDashboardJson } from '../utils/validation';
+
import { importDashboard } from './actions';
import { DataSourceInput, ImportDashboardDTO, initialImportDashboardState, InputType } from './reducers';
@@ -75,3 +77,32 @@ describe('importDashboard', () => {
});
});
});
+
+describe('validateDashboardJson', () => {
+ it('Should return true if correct json', async () => {
+ const jsonImportCorrectFormat = '{"title": "Correct Format", "tags": ["tag1", "tag2"], "schemaVersion": 36}';
+ const validateDashboardJsonCorrectFormat = await validateDashboardJson(jsonImportCorrectFormat);
+ expect(validateDashboardJsonCorrectFormat).toBe(true);
+ });
+ it('Should not return true if nested tags', async () => {
+ const jsonImportNestedTags =
+ '{"title": "Nested tags","tags": ["tag1", "tag2", ["nestedTag1", "nestedTag2"]],"schemaVersion": 36}';
+ const validateDashboardJsonNestedTags = await validateDashboardJson(jsonImportNestedTags);
+ expect(validateDashboardJsonNestedTags).toBe('tags expected array of strings');
+ });
+ it('Should not return true if not an array', async () => {
+ const jsonImportNotArray = '{"title": "Not Array","tags": "tag1","schemaVersion":36}';
+ const validateDashboardJsonNotArray = await validateDashboardJson(jsonImportNotArray);
+ expect(validateDashboardJsonNotArray).toBe('tags expected array');
+ });
+ it('Should not return true if not an array and is blank string', async () => {
+ const jsonImportEmptyTags = '{"schemaVersion": 36,"tags": "", "title": "Empty Tags"}';
+ const validateDashboardJsonEmptyTags = await validateDashboardJson(jsonImportEmptyTags);
+ expect(validateDashboardJsonEmptyTags).toBe('tags expected array');
+ });
+ it('Should not return true if not valid JSON', async () => {
+ const jsonImportInvalidJson = '{"schemaVersion": 36,"tags": {"tag", "nested tag"}, "title": "Nested lists"}';
+ const validateDashboardJsonNotValid = await validateDashboardJson(jsonImportInvalidJson);
+ expect(validateDashboardJsonNotValid).toBe('Not valid JSON');
+ });
+});
diff --git a/public/app/features/manage-dashboards/utils/validation.ts b/public/app/features/manage-dashboards/utils/validation.ts
index bee8a2200c8..282b011cb00 100644
--- a/public/app/features/manage-dashboards/utils/validation.ts
+++ b/public/app/features/manage-dashboards/utils/validation.ts
@@ -3,12 +3,23 @@ import { getBackendSrv } from '@grafana/runtime';
import { validationSrv } from '../services/ValidationSrv';
export const validateDashboardJson = (json: string) => {
+ let dashboard;
try {
- JSON.parse(json);
- return true;
+ dashboard = JSON.parse(json);
} catch (error) {
return 'Not valid JSON';
}
+ if (dashboard && dashboard.hasOwnProperty('tags')) {
+ if (Array.isArray(dashboard.tags)) {
+ const hasInvalidTag = dashboard.tags.some((tag: string) => typeof tag !== 'string');
+ if (hasInvalidTag) {
+ return 'tags expected array of strings';
+ }
+ } else {
+ return 'tags expected array';
+ }
+ }
+ return true;
};
export const validateGcomDashboard = (gcomDashboard: string) => {