Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mutating options.format is unsafe when config is re-used #1341

Closed
robhogan opened this issue Feb 17, 2023 · 0 comments · Fixed by #1342
Closed

Mutating options.format is unsafe when config is re-used #1341

robhogan opened this issue Feb 17, 2023 · 0 comments · Fixed by #1342

Comments

@robhogan
Copy link
Contributor

robhogan commented Feb 17, 2023

Bug report or Feature request?
Bug report

Version (complete output of terser -V or specific git commit)
Confirmed on 5.15.0 and current main (e062dc8)

Description
Terser stores its source map output on the input config object, under options.format.source_map. The returned value includes a getter for a map property, which uses options.format.source_map. If options.format.source_map changes between the minify result and the use of the getter, the output may be incorrect.

In particular, a second call to minify using the same config object will clobber source_map.

Failing test

import {minify} from 'terser';

test('subsequent calls do not clobber previous output', async () => {
  const config = {
    format: {},
    sourceMap: true,
  };

  const fooResult = await minify('"foo";', config);
  const barResult = await minify('module.exports = "bar";', config);

  const fooMap = fooResult.map; // Uses a getter which uses config.format.source_map, which was overwritten by the second call.
  const barMap = barResult.map;

  expect(barMap).toEqual(
    '{"version":3,"names":["module","exports"],"sources":["0"],"mappings":"AAAAA,OAAOC,QAAU"}',
  ); // Succeeds - `barMap` is correct
  expect(fooMap).not.toEqual(barMap); // <-- Fails - `fooMap` is `barMap`
});

Expected result
Test passes - reusing a config object should not affect previous output.

robhogan added a commit to robhogan/terser that referenced this issue Feb 17, 2023
robhogan added a commit to robhogan/metro that referenced this issue Feb 17, 2023
Summary:
Mitigates terser/terser#1341

Terser sets properties inlcuding `source_map` and `_destroy_ast` on the given `options.output` (or `options.format`) object, which can affect subsequent calls where we re-use the same config object.

Here we take a shallow copy of the given configuration, so `terser` doesn't mutate `metro-minify-terser`'s input.

Changelog: [Fix] Mitigate potential source map mismatches with concurrent transformations (Terser [facebook#1341](terser/terser#1341))

Reviewed By: jacdebug, motiz88

Differential Revision: D43362977

fbshipit-source-id: 019f417aa8cc7897c71a6ab2c7db5d0cf916e59d
robhogan added a commit to robhogan/terser that referenced this issue Feb 17, 2023
facebook-github-bot pushed a commit to facebook/metro that referenced this issue Feb 17, 2023
Summary:
Mitigates terser/terser#1341

Terser sets properties inlcuding `source_map` and `_destroy_ast` on the given `options.output` (or `options.format`) object, which can affect subsequent calls where we re-use the same config object.

Here we take a shallow copy of the given configuration, so `terser` doesn't mutate `metro-minify-terser`'s input.

Changelog: [Fix] Mitigate potential source map mismatches with concurrent transformations (Terser [#1341](terser/terser#1341))

Reviewed By: jacdebug, motiz88

Differential Revision: D43362977

fbshipit-source-id: 940e6209273cdd382513aea2f2a8aeca12daa2e3
robhogan added a commit to facebook/metro that referenced this issue Feb 18, 2023
Summary:
Mitigates terser/terser#1341

Terser sets properties inlcuding `source_map` and `_destroy_ast` on the given `options.output` (or `options.format`) object, which can affect subsequent calls where we re-use the same config object.

Here we take a shallow copy of the given configuration, so `terser` doesn't mutate `metro-minify-terser`'s input.

Changelog: [Fix] Mitigate potential source map mismatches with concurrent transformations (Terser [#1341](terser/terser#1341))

Reviewed By: jacdebug, motiz88

Differential Revision: D43362977

fbshipit-source-id: 940e6209273cdd382513aea2f2a8aeca12daa2e3
robhogan added a commit to facebook/metro that referenced this issue Feb 19, 2023
Summary:
Mitigates terser/terser#1341

Terser sets properties inlcuding `source_map` and `_destroy_ast` on the given `options.output` (or `options.format`) object, which can affect subsequent calls where we re-use the same config object.

Here we take a shallow copy of the given configuration, so `terser` doesn't mutate `metro-minify-terser`'s input.

Changelog: [Fix] Mitigate potential source map mismatches with concurrent transformations (Terser [#1341](terser/terser#1341))

Reviewed By: jacdebug, motiz88

Differential Revision: D43362977

fbshipit-source-id: 940e6209273cdd382513aea2f2a8aeca12daa2e3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant