SavedObjects management: display explicit import error in case of failure and avoid invalid file to crash the server (#82406) (#82613)

pgayvallet · web-flow · commit 85fa2caf608a · 2020-11-04T17:57:21.000+01:00
* display error cause in case of import failure

* avoid server crash when importing invalid file

* remove unused translations

* fix unit tests

* change savedObjects.maxImportPayloadBytes default to 25mb

* fix types and logic
# Conflicts:
#	test/functional/apps/management/_import_objects.ts
diff --git a/src/core/server/core_usage_data/core_usage_data_service.mock.ts b/src/core/server/core_usage_data/core_usage_data_service.mock.ts
@@ -108,7 +108,7 @@ const createStartContractMock = () => {
           },
           savedObjects: {
             maxImportExportSizeBytes: 10000,
-            maxImportPayloadBytes: 10485760,
+            maxImportPayloadBytes: 26214400,
           },
         },
         environment: {
diff --git a/src/core/server/core_usage_data/core_usage_data_service.test.ts b/src/core/server/core_usage_data/core_usage_data_service.test.ts
@@ -188,7 +188,7 @@ describe('CoreUsageDataService', () => {
               },
               "savedObjects": Object {
                 "maxImportExportSizeBytes": 10000,
-                "maxImportPayloadBytes": 10485760,
+                "maxImportPayloadBytes": 26214400,
               },
             },
             "environment": Object {
diff --git a/src/core/server/mocks.ts b/src/core/server/mocks.ts
@@ -72,7 +72,7 @@ export function pluginInitializerContextConfigMock<T>(config: T) {
     },
     path: { data: '/tmp' },
     savedObjects: {
-      maxImportPayloadBytes: new ByteSizeValue(10485760),
+      maxImportPayloadBytes: new ByteSizeValue(26214400),
     },
   };
 
diff --git a/src/core/server/plugins/plugin_context.test.ts b/src/core/server/plugins/plugin_context.test.ts
@@ -93,7 +93,7 @@ describe('createPluginInitializerContext', () => {
         pingTimeout: duration(30, 's'),
       },
       path: { data: fromRoot('data') },
-      savedObjects: { maxImportPayloadBytes: new ByteSizeValue(10485760) },
+      savedObjects: { maxImportPayloadBytes: new ByteSizeValue(26214400) },
     });
   });
 
diff --git a/src/core/server/saved_objects/routes/import.ts b/src/core/server/saved_objects/routes/import.ts
@@ -71,10 +71,19 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig)
         return res.badRequest({ body: `Invalid file extension ${fileExtension}` });
       }
 
+      let readStream: Readable;
+      try {
+        readStream = await createSavedObjectsStreamFromNdJson(file);
+      } catch (e) {
+        return res.badRequest({
+          body: e,
+        });
+      }
+
       const result = await importSavedObjectsFromStream({
         savedObjectsClient: context.core.savedObjects.client,
         typeRegistry: context.core.savedObjects.typeRegistry,
-        readStream: createSavedObjectsStreamFromNdJson(file),
+        readStream,
         objectLimit: maxImportExportSize,
         overwrite,
         createNewCopies,
diff --git a/src/core/server/saved_objects/routes/integration_tests/export.test.ts b/src/core/server/saved_objects/routes/integration_tests/export.test.ts
@@ -33,7 +33,7 @@ type SetupServerReturn = UnwrapPromise<ReturnType<typeof setupServer>>;
 const exportSavedObjectsToStream = exportMock.exportSavedObjectsToStream as jest.Mock;
 const allowedTypes = ['index-pattern', 'search'];
 const config = {
-  maxImportPayloadBytes: 10485760,
+  maxImportPayloadBytes: 26214400,
   maxImportExportSize: 10000,
 } as SavedObjectConfig;
 
diff --git a/src/core/server/saved_objects/routes/integration_tests/import.test.ts b/src/core/server/saved_objects/routes/integration_tests/import.test.ts
@@ -30,7 +30,7 @@ type SetupServerReturn = UnwrapPromise<ReturnType<typeof setupServer>>;
 
 const { v4: uuidv4 } = jest.requireActual('uuid');
 const allowedTypes = ['index-pattern', 'visualization', 'dashboard'];
-const config = { maxImportPayloadBytes: 10485760, maxImportExportSize: 10000 } as SavedObjectConfig;
+const config = { maxImportPayloadBytes: 26214400, maxImportExportSize: 10000 } as SavedObjectConfig;
 const URL = '/internal/saved_objects/_import';
 
 describe(`POST ${URL}`, () => {
diff --git a/src/core/server/saved_objects/routes/integration_tests/resolve_import_errors.test.ts b/src/core/server/saved_objects/routes/integration_tests/resolve_import_errors.test.ts
@@ -29,7 +29,7 @@ type SetupServerReturn = UnwrapPromise<ReturnType<typeof setupServer>>;
 
 const { v4: uuidv4 } = jest.requireActual('uuid');
 const allowedTypes = ['index-pattern', 'visualization', 'dashboard'];
-const config = { maxImportPayloadBytes: 10485760, maxImportExportSize: 10000 } as SavedObjectConfig;
+const config = { maxImportPayloadBytes: 26214400, maxImportExportSize: 10000 } as SavedObjectConfig;
 const URL = '/api/saved_objects/_resolve_import_errors';
 
 describe(`POST ${URL}`, () => {
diff --git a/src/core/server/saved_objects/routes/resolve_import_errors.ts b/src/core/server/saved_objects/routes/resolve_import_errors.ts
@@ -78,10 +78,19 @@ export const registerResolveImportErrorsRoute = (router: IRouter, config: SavedO
         return res.badRequest({ body: `Invalid file extension ${fileExtension}` });
       }
 
+      let readStream: Readable;
+      try {
+        readStream = await createSavedObjectsStreamFromNdJson(file);
+      } catch (e) {
+        return res.badRequest({
+          body: e,
+        });
+      }
+
       const result = await resolveSavedObjectsImportErrors({
         typeRegistry: context.core.savedObjects.typeRegistry,
         savedObjectsClient: context.core.savedObjects.client,
-        readStream: createSavedObjectsStreamFromNdJson(file),
+        readStream,
         retries: req.body.retries,
         objectLimit: maxImportExportSize,
         createNewCopies: req.query.createNewCopies,
diff --git a/src/core/server/saved_objects/routes/utils.test.ts b/src/core/server/saved_objects/routes/utils.test.ts
@@ -27,7 +27,7 @@ async function readStreamToCompletion(stream: Readable) {
 
 describe('createSavedObjectsStreamFromNdJson', () => {
   it('transforms an ndjson stream into a stream of saved objects', async () => {
-    const savedObjectsStream = createSavedObjectsStreamFromNdJson(
+    const savedObjectsStream = await createSavedObjectsStreamFromNdJson(
       new Readable({
         read() {
           this.push('{"id": "foo", "type": "foo-type"}\n');
@@ -52,7 +52,7 @@ describe('createSavedObjectsStreamFromNdJson', () => {
   });
 
   it('skips empty lines', async () => {
-    const savedObjectsStream = createSavedObjectsStreamFromNdJson(
+    const savedObjectsStream = await createSavedObjectsStreamFromNdJson(
       new Readable({
         read() {
           this.push('{"id": "foo", "type": "foo-type"}\n');
@@ -79,7 +79,7 @@ describe('createSavedObjectsStreamFromNdJson', () => {
   });
 
   it('filters the export details entry from the stream', async () => {
-    const savedObjectsStream = createSavedObjectsStreamFromNdJson(
+    const savedObjectsStream = await createSavedObjectsStreamFromNdJson(
       new Readable({
         read() {
           this.push('{"id": "foo", "type": "foo-type"}\n');
diff --git a/src/core/server/saved_objects/routes/utils.ts b/src/core/server/saved_objects/routes/utils.ts
@@ -19,23 +19,30 @@
 
 import { Readable } from 'stream';
 import { SavedObject, SavedObjectsExportResultDetails } from 'src/core/server';
-import { createSplitStream, createMapStream, createFilterStream } from '../../utils/streams';
+import {
+  createSplitStream,
+  createMapStream,
+  createFilterStream,
+  createPromiseFromStreams,
+  createListStream,
+  createConcatStream,
+} from '../../utils/streams';
 
-export function createSavedObjectsStreamFromNdJson(ndJsonStream: Readable) {
-  return ndJsonStream
-    .pipe(createSplitStream('\n'))
-    .pipe(
-      createMapStream((str: string) => {
-        if (str && str.trim() !== '') {
-          return JSON.parse(str);
-        }
-      })
-    )
-    .pipe(
-      createFilterStream<SavedObject | SavedObjectsExportResultDetails>(
-        (obj) => !!obj && !(obj as SavedObjectsExportResultDetails).exportedCount
-      )
-    );
+export async function createSavedObjectsStreamFromNdJson(ndJsonStream: Readable) {
+  const savedObjects = await createPromiseFromStreams([
+    ndJsonStream,
+    createSplitStream('\n'),
+    createMapStream((str: string) => {
+      if (str && str.trim() !== '') {
+        return JSON.parse(str);
+      }
+    }),
+    createFilterStream<SavedObject | SavedObjectsExportResultDetails>(
+      (obj) => !!obj && !(obj as SavedObjectsExportResultDetails).exportedCount
+    ),
+    createConcatStream([]),
+  ]);
+  return createListStream(savedObjects);
 }
 
 export function validateTypes(types: string[], supportedTypes: string[]): string | undefined {
diff --git a/src/core/server/saved_objects/saved_objects_config.ts b/src/core/server/saved_objects/saved_objects_config.ts
@@ -36,7 +36,7 @@ export type SavedObjectsConfigType = TypeOf<typeof savedObjectsConfig.schema>;
 export const savedObjectsConfig = {
   path: 'savedObjects',
   schema: schema.object({
-    maxImportPayloadBytes: schema.byteSize({ defaultValue: 10485760 }),
+    maxImportPayloadBytes: schema.byteSize({ defaultValue: 26214400 }),
     maxImportExportSize: schema.byteSize({ defaultValue: 10000 }),
   }),
 };
diff --git a/src/core/server/utils/streams/map_stream.test.ts b/src/core/server/utils/streams/map_stream.test.ts
@@ -58,4 +58,19 @@ describe('createMapStream()', () => {
 
     expect(result).toEqual([0, 2, 6]);
   });
+
+  test('handles errors in async mappers', async () => {
+    await expect(
+      createPromiseFromStreams([
+        createListStream([1, 2, 3]),
+        createMapStream(async (n: number, i: number) => {
+          if (n === 2) {
+            await Promise.reject(new Error('that went bad'));
+          }
+          return n;
+        }),
+        createConcatStream([]),
+      ])
+    ).rejects.toMatchInlineSnapshot(`[Error: that went bad]`);
+  });
 });
diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/components/__snapshots__/flyout.test.tsx.snap b/src/plugins/saved_objects_management/public/management_section/objects_table/components/__snapshots__/flyout.test.tsx.snap
diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/components/flyout.test.tsx b/src/plugins/saved_objects_management/public/management_section/objects_table/components/flyout.test.tsx
@@ -460,7 +460,9 @@ describe('Flyout', () => {
       // Ensure all promises resolve
       await new Promise((resolve) => process.nextTick(resolve));
 
-      expect(component.state('error')).toEqual('foobar');
+      expect(component.state('error')).toMatchInlineSnapshot(
+        `"The file could not be processed due to error: \\"foobar\\""`
+      );
       expect(component.find('EuiFlyoutBody EuiCallOut')).toMatchSnapshot();
     });
   });
diff --git a/src/plugins/saved_objects_management/public/management_section/objects_table/components/flyout.tsx b/src/plugins/saved_objects_management/public/management_section/objects_table/components/flyout.tsx
@@ -106,6 +106,17 @@ interface ConflictingRecord {
   done: (result: [boolean, string | undefined]) => void;
 }
 
+const getErrorMessage = (e: any) => {
+  const errorMessage =
+    e.body?.error && e.body?.message ? `${e.body.error}: ${e.body.message}` : e.message;
+  return i18n.translate('savedObjectsManagement.objectsTable.flyout.importFileErrorMessage', {
+    defaultMessage: 'The file could not be processed due to error: "{error}"',
+    values: {
+      error: errorMessage,
+    },
+  });
+};
+
 export class Flyout extends Component<FlyoutProps, FlyoutState> {
   constructor(props: FlyoutProps) {
     super(props);
@@ -178,9 +189,7 @@ export class Flyout extends Component<FlyoutProps, FlyoutState> {
     } catch (e) {
       this.setState({
         status: 'error',
-        error: i18n.translate('savedObjectsManagement.objectsTable.flyout.importFileErrorMessage', {
-          defaultMessage: 'The file could not be processed.',
-        }),
+        error: getErrorMessage(e),
       });
       return;
     }
@@ -236,10 +245,7 @@ export class Flyout extends Component<FlyoutProps, FlyoutState> {
     } catch (e) {
       this.setState({
         status: 'error',
-        error: i18n.translate(
-          'savedObjectsManagement.objectsTable.flyout.resolveImportErrorsFileErrorMessage',
-          { defaultMessage: 'The file could not be processed.' }
-        ),
+        error: getErrorMessage(e),
       });
     }
   };
@@ -432,8 +438,8 @@ export class Flyout extends Component<FlyoutProps, FlyoutState> {
         );
       } catch (e) {
         this.setState({
-          error: e.message,
           status: 'error',
+          error: getErrorMessage(e),
           loadingMessage: undefined,
         });
         return;
@@ -583,7 +589,7 @@ export class Flyout extends Component<FlyoutProps, FlyoutState> {
           }
           color="danger"
         >
-          <p>{error}</p>
+          <p data-test-subj="importSavedObjectsErrorText">{error}</p>
         </EuiCallOut>
         <EuiSpacer size="s" />
       </Fragment>
@@ -737,6 +743,7 @@ export class Flyout extends Component<FlyoutProps, FlyoutState> {
           }
         >
           <EuiFilePicker
+            accept=".ndjson, .json"
             fullWidth
             initialPromptText={
               <FormattedMessage
diff --git a/test/functional/apps/management/_import_objects.js b/test/functional/apps/management/_import_objects.js
@@ -415,6 +415,44 @@ export default function ({ getService, getPageObjects }) {
         const isSavedObjectImported = objects.includes('saved object imported with index pattern');
         expect(isSavedObjectImported).to.be(true);
       });
+
+      it('should display an explicit error message when importing object from a higher Kibana version', async () => {
+        await PageObjects.savedObjects.importFile(
+          path.join(__dirname, 'exports', '_import_higher_version.ndjson')
+        );
+
+        await PageObjects.savedObjects.checkImportError();
+
+        const errorText = await PageObjects.savedObjects.getImportErrorText();
+
+        expect(errorText).to.contain(
+          `has property "visualization" which belongs to a more recent version of Kibana [9.15.82]`
+        );
+      });
+
+      it('should display an explicit error message when importing a file bigger than allowed', async () => {
+        await PageObjects.savedObjects.importFile(
+          path.join(__dirname, 'exports', '_import_too_big.ndjson')
+        );
+
+        await PageObjects.savedObjects.checkImportError();
+
+        const errorText = await PageObjects.savedObjects.getImportErrorText();
+
+        expect(errorText).to.contain(`Payload content length greater than maximum allowed`);
+      });
+
+      it('should display an explicit error message when importing an invalid file', async () => {
+        await PageObjects.savedObjects.importFile(
+          path.join(__dirname, 'exports', '_import_invalid_format.ndjson')
+        );
+
+        await PageObjects.savedObjects.checkImportError();
+
+        const errorText = await PageObjects.savedObjects.getImportErrorText();
+
+        expect(errorText).to.contain(`Unexpected token T in JSON at position 0`);
+      });
     });
   });
 }
diff --git a/test/functional/apps/management/exports/_import_higher_version.ndjson b/test/functional/apps/management/exports/_import_higher_version.ndjson
@@ -0,0 +1 @@
+{"attributes":{"description":"","kibanaSavedObjectMeta":{"searchSourceJSON":"{\"filter\":[],\"query\":{\"query\":\"\",\"language\":\"lucene\"},\"indexRefName\":\"kibanaSavedObjectMeta.searchSourceJSON.index\"}"},"title":"Log Agents","uiStateJSON":"{}","visState":"{\"title\":\"Log Agents\",\"type\":\"area\",\"params\":{\"type\":\"area\",\"grid\":{\"categoryLines\":false,\"style\":{\"color\":\"#eee\"}},\"categoryAxes\":[{\"id\":\"CategoryAxis-1\",\"type\":\"category\",\"position\":\"bottom\",\"show\":true,\"style\":{},\"scale\":{\"type\":\"linear\"},\"labels\":{\"show\":true,\"truncate\":100},\"title\":{\"text\":\"agent.raw: Descending\"}}],\"valueAxes\":[{\"id\":\"ValueAxis-1\",\"name\":\"LeftAxis-1\",\"type\":\"value\",\"position\":\"left\",\"show\":true,\"style\":{},\"scale\":{\"type\":\"linear\",\"mode\":\"normal\"},\"labels\":{\"show\":true,\"rotate\":0,\"filter\":false,\"truncate\":100},\"title\":{\"text\":\"Count\"}}],\"seriesParams\":[{\"show\":\"true\",\"type\":\"area\",\"mode\":\"stacked\",\"data\":{\"label\":\"Count\",\"id\":\"1\"},\"drawLinesBetweenPoints\":true,\"showCircles\":true,\"interpolate\":\"linear\",\"valueAxis\":\"ValueAxis-1\"}],\"addTooltip\":true,\"addLegend\":true,\"legendPosition\":\"right\",\"times\":[],\"addTimeMarker\":false},\"aggs\":[{\"id\":\"1\",\"enabled\":true,\"type\":\"count\",\"schema\":\"metric\",\"params\":{}},{\"id\":\"2\",\"enabled\":true,\"type\":\"terms\",\"schema\":\"segment\",\"params\":{\"field\":\"agent.raw\",\"size\":5,\"order\":\"desc\",\"orderBy\":\"1\"}}]}"},"id":"082f1d60-a2e7-11e7-bb30-233be9be6a15","migrationVersion":{"visualization":"9.15.82"},"references":[],"type":"visualization","version":1}
diff --git a/test/functional/apps/management/exports/_import_invalid_format.ndjson b/test/functional/apps/management/exports/_import_invalid_format.ndjson
@@ -0,0 +1 @@
+This is NOT a ndjson file!
diff --git a/test/functional/apps/management/exports/_import_too_big.ndjson b/test/functional/apps/management/exports/_import_too_big.ndjson
diff --git a/test/functional/config.js b/test/functional/config.js
diff --git a/test/functional/page_objects/management/saved_objects_page.ts b/test/functional/page_objects/management/saved_objects_page.ts
diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json
diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json
