From 360c42e8a6f8aea7f820a5df5e41ccbf306ce136 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bazyli=20Brz=C3=B3ska?= Date: Sat, 27 Jan 2018 12:46:58 +0100 Subject: [PATCH 1/4] Prevent a ENOENT crash by checking for existence Jest tries to load the stack-trace based on the top frame. The problem is, the filename may be provided by the source-map, which is unreliable since it can point to a non-existent file. In such a case, the code would cause a hard crash of "Test suite failed to run" with a stack-trace pointing at "exports.formatStackTrace", which I have corrected here. --- packages/jest-message-util/src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jest-message-util/src/index.js b/packages/jest-message-util/src/index.js index ec9461dff078..c2c00d005406 100644 --- a/packages/jest-message-util/src/index.js +++ b/packages/jest-message-util/src/index.js @@ -227,7 +227,7 @@ export const formatStackTrace = ( if (topFrame) { const filename = topFrame.file; - if (path.isAbsolute(filename)) { + if (path.isAbsolute(filename) && fs.existsSync(filename)) { renderedCallsite = codeFrameColumns( fs.readFileSync(filename, 'utf8'), { From 76f299953bc65aa80a00dd4189ad4b8f14ba5852 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bazyli=20Brzo=CC=81ska?= Date: Sat, 27 Jan 2018 14:38:45 +0100 Subject: [PATCH 2/4] add an integration test for bad-source-map case --- CHANGELOG.md | 1 + .../__tests__/bad_source_map.test.js | 16 ++++++++++++++++ .../bad-source-map/__tests__/bad-source-map.js | 15 +++++++++++++++ .../__tests__/bad-source-map.js.map | 1 + integration-tests/bad-source-map/package.json | 5 +++++ 5 files changed, 38 insertions(+) create mode 100644 integration-tests/__tests__/bad_source_map.test.js create mode 100644 integration-tests/bad-source-map/__tests__/bad-source-map.js create mode 100644 integration-tests/bad-source-map/__tests__/bad-source-map.js.map create mode 100644 integration-tests/bad-source-map/package.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 2822ebb21d9d..e3100f502865 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Fixes +* `[jest-message-util]` Prevent an `ENOENT` crash when the test file contained a malformed source-map. * `[jest]` Add `import-local` to `jest` package. ([#5353](https://github.com/facebook/jest/pull/5353)) * `[expect]` Support class instances in `.toHaveProperty()` matcher. diff --git a/integration-tests/__tests__/bad_source_map.test.js b/integration-tests/__tests__/bad_source_map.test.js new file mode 100644 index 000000000000..f2ac3a4050c0 --- /dev/null +++ b/integration-tests/__tests__/bad_source_map.test.js @@ -0,0 +1,16 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. All rights reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ +'use strict'; + +const runJest = require('../runJest'); + +test('suite with test cases that contain malformed sourcemaps', () => { + const result = runJest('bad-source-map'); + expect(result.stderr).not.toMatch('ENOENT'); +}); diff --git a/integration-tests/bad-source-map/__tests__/bad-source-map.js b/integration-tests/bad-source-map/__tests__/bad-source-map.js new file mode 100644 index 000000000000..db9e5136a166 --- /dev/null +++ b/integration-tests/bad-source-map/__tests__/bad-source-map.js @@ -0,0 +1,15 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. All rights reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ +'use strict'; + +it('should fail with a proper stacktrace', () => { + expect(true).toBe(false); +}); + +//# sourceMappingURL=bad-source-map.js.map diff --git a/integration-tests/bad-source-map/__tests__/bad-source-map.js.map b/integration-tests/bad-source-map/__tests__/bad-source-map.js.map new file mode 100644 index 000000000000..6d7809e16409 --- /dev/null +++ b/integration-tests/bad-source-map/__tests__/bad-source-map.js.map @@ -0,0 +1 @@ +{"version":3,"file":"bad-source-map.js","sources":["dummy:///./index.js"],"sourcesContent":[""],"mappings":";AAAA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;;;;;;;;;;;;ACzEA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;AACA;;;;;;;;;;;;ACRA;;;;;;;;;;;;;;;;A","sourceRoot":""} diff --git a/integration-tests/bad-source-map/package.json b/integration-tests/bad-source-map/package.json new file mode 100644 index 000000000000..148788b25446 --- /dev/null +++ b/integration-tests/bad-source-map/package.json @@ -0,0 +1,5 @@ +{ + "jest": { + "testEnvironment": "node" + } +} From 1b2477678d36c6ff74b623f5a8b7854a2ce78676 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bazyli=20Brzo=CC=81ska?= Date: Sun, 28 Jan 2018 14:14:33 +0100 Subject: [PATCH 3/4] lint CHANGELOG --- CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3100f502865..a0607a004208 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,13 +1,14 @@ ## master -### features +### Features * `[jest-mock]` Add util methods to create async functions. ([#5318](https://github.com/facebook/jest/pull/5318)) ### Fixes -* `[jest-message-util]` Prevent an `ENOENT` crash when the test file contained a malformed source-map. +* `[jest-message-util]` Prevent an `ENOENT` crash when the test file contained a + malformed source-map. ([#5405](https://github.com/facebook/jest/pull/5405)). * `[jest]` Add `import-local` to `jest` package. ([#5353](https://github.com/facebook/jest/pull/5353)) * `[expect]` Support class instances in `.toHaveProperty()` matcher. From f1057010fa1207e96ec2edc82ba5ea0801b3093e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bazyli=20Brzo=CC=81ska?= Date: Sun, 28 Jan 2018 14:14:57 +0100 Subject: [PATCH 4/4] use a try-catch rather than fs.existsSync --- packages/jest-message-util/src/index.js | 41 ++++++++++++++++--------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/packages/jest-message-util/src/index.js b/packages/jest-message-util/src/index.js index c2c00d005406..0f93bd54408d 100644 --- a/packages/jest-message-util/src/index.js +++ b/packages/jest-message-util/src/index.js @@ -70,6 +70,22 @@ const trim = string => (string || '').replace(/^\s+/, '').replace(/\s+$/, ''); const trimPaths = string => string.match(STACK_PATH_REGEXP) ? trim(string) : string; +const getRenderedCallsite = (fileContent: string, line: number) => { + let renderedCallsite = codeFrameColumns( + fileContent, + {start: {line}}, + {highlightCode: true}, + ); + + renderedCallsite = renderedCallsite + .split('\n') + .map(line => MESSAGE_INDENT + line) + .join('\n'); + + renderedCallsite = `\n${renderedCallsite}\n`; + return renderedCallsite; +}; + // ExecError is an error thrown outside of the test suite (not inside an `it` or // `before/after each` hooks). If it's thrown, none of the tests in the file // are executed. @@ -227,21 +243,16 @@ export const formatStackTrace = ( if (topFrame) { const filename = topFrame.file; - if (path.isAbsolute(filename) && fs.existsSync(filename)) { - renderedCallsite = codeFrameColumns( - fs.readFileSync(filename, 'utf8'), - { - start: {line: topFrame.line}, - }, - {highlightCode: true}, - ); - - renderedCallsite = renderedCallsite - .split('\n') - .map(line => MESSAGE_INDENT + line) - .join('\n'); - - renderedCallsite = `\n${renderedCallsite}\n`; + if (path.isAbsolute(filename)) { + let fileContent; + try { + // TODO: check & read HasteFS instead of reading the filesystem: + // see: https://github.com/facebook/jest/pull/5405#discussion_r164281696 + fileContent = fs.readFileSync(filename, 'utf8'); + renderedCallsite = getRenderedCallsite(fileContent, topFrame.line); + } catch (e) { + // the file does not exist or is inaccessible, we ignore + } } }