Skip to content

Commit

Permalink
Backport #20379 (#22162)
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisdavies authored Aug 17, 2018
1 parent 8ab5152 commit c6af290
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 6 deletions.
9 changes: 8 additions & 1 deletion src/core_plugins/kibana/public/dashboard/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import dashboardTemplate from './dashboard_app.html';
import dashboardListingTemplate from './listing/dashboard_listing_ng_wrapper.html';

import { DashboardConstants, createDashboardEditUrl } from './dashboard_constants';
import { SavedObjectNotFound } from 'ui/errors';
import { InvalidJSONProperty, SavedObjectNotFound } from 'ui/errors';
import { FeatureCatalogueRegistryProvider, FeatureCatalogueCategory } from 'ui/registry/feature_catalogue';
import { SavedObjectsClientProvider } from 'ui/saved_objects';
import { recentlyAccessed } from 'ui/persisted_log';
Expand Down Expand Up @@ -113,6 +113,13 @@ uiRoutes
return savedDashboard;
})
.catch((error) => {
// A corrupt dashboard was detected (e.g. with invalid JSON properties)
if (error instanceof InvalidJSONProperty) {
toastNotifications.addDanger(error.message);
kbnUrl.redirect(DashboardConstants.LANDING_PAGE_PATH);
return;
}

// Preserve BWC of v5.3.0 links for new, unsaved dashboards.
// See https://github.com/elastic/kibana/issues/10951 for more context.
if (error instanceof SavedObjectNotFound && id === 'create') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ export async function resolveSavedObjects(
// Keep track of how many we actually import because the user
// can cancel an override
let importedObjectCount = 0;
// Keep a record of any objects which fail to import for unknown reasons.
const failedImports = [];
// Start with the index patterns since everything is dependent on them
await awaitEachItemInParallel(
Expand All @@ -186,8 +187,6 @@ export async function resolveSavedObjects(
// exist. We will provide a way for the user to manually select a new index pattern for those
// saved objects.
const conflictedIndexPatterns = [];
// Keep a record of any objects which fail to import for unknown reasons.

// It's possible to have saved objects that link to saved searches which then link to index patterns
// and those could error out, but the error comes as an index pattern not found error. We can't resolve
// those the same as way as normal index pattern not found errors, but when those are fixed, it's very
Expand Down Expand Up @@ -226,6 +225,7 @@ export async function resolveSavedObjects(
if (error.savedObjectType === 'search') {
failedImports.push({ obj, error });
}

if (error.savedObjectType === 'index-pattern') {
if (obj.savedSearchId) {
conflictedSavedObjectsLinkedToSavedSearches.push(obj);
Expand Down
21 changes: 20 additions & 1 deletion src/ui/public/courier/saved_object/__tests__/saved_object.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import BluebirdPromise from 'bluebird';
import { SavedObjectProvider } from '../saved_object';
import { IndexPatternProvider } from '../../../index_patterns/_index_pattern';
import { SavedObjectsClientProvider } from '../../../saved_objects';

import { StubIndexPatternsApiClientModule } from '../../../index_patterns/__tests__/stub_index_patterns_api_client';
import { InvalidJSONProperty } from '../../../errors';

describe('Saved Object', function () {
require('test_utils/no_digest_promises').activateForSuite();
Expand Down Expand Up @@ -300,6 +300,25 @@ describe('Saved Object', function () {
});
});

it('throws error invalid JSON is detected', async function () {
const savedObject = await createInitializedSavedObject({ type: 'dashboard', searchSource: true });
const response = {
found: true,
_source: {
kibanaSavedObjectMeta: {
searchSourceJSON: '\"{\\n \\\"filter\\\": []\\n}\"'
}
}
};

try {
await savedObject.applyESResp(response);
throw new Error('applyESResp should have failed, but did not.');
} catch (err) {
expect(err instanceof InvalidJSONProperty).to.be(true);
}
});

it('preserves original defaults if not overridden', function () {
const id = 'anid';
const preserveMeValue = 'here to stay!';
Expand Down
12 changes: 10 additions & 2 deletions src/ui/public/courier/saved_object/saved_object.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import angular from 'angular';
import _ from 'lodash';

import { SavedObjectNotFound } from '../../errors';
import { InvalidJSONProperty, SavedObjectNotFound } from '../../errors';
import MappingSetupProvider from '../../utils/mapping_setup';

import { SearchSourceProvider } from '../search_source';
Expand Down Expand Up @@ -117,7 +117,15 @@ export function SavedObjectProvider(Promise, Private, Notifier, confirmModalProm
try {
searchSourceValues = JSON.parse(searchSourceJson);
} catch (e) {
searchSourceValues = {};
throw new InvalidJSONProperty(
`Invalid JSON in ${esType} "${this.id}". ${e.message} JSON: ${searchSourceJson}`
);
}

// This detects a scenario where documents with invalid JSON properties have been imported into the saved object index.
// (This happened in issue #20308)
if (!searchSourceValues || typeof searchSourceValues !== 'object') {
throw new InvalidJSONProperty(`Invalid searchSourceJSON in ${esType} "${this.id}".`);
}

const searchSourceFields = this.searchSource.getFields();
Expand Down
11 changes: 11 additions & 0 deletions src/ui/public/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,17 @@ export class PersistedStateError extends KbnError {
}
}

/**
* This error is for scenarios where a saved object is detected that has invalid JSON properties.
* There was a scenario where we were importing objects with double-encoded JSON, and the system
* was silently failing. This error is now thrown in those scenarios.
*/
export class InvalidJSONProperty extends KbnError {
constructor(message) {
super(message);
}
}

/**
* UI Errors
*/
Expand Down

0 comments on commit c6af290

Please sign in to comment.