mozilla / source-map

Consume and generate source maps.
Other
3.56k stars 359 forks source link

SourceMapConsumer prints random error on decoded mappings #422

Closed milahu closed 3 years ago

milahu commented 4 years ago

SMC fails on decoded mappings

// pseudo code

import { SourceMapConsumer } from 'source-map';
const decoded_map = {
  mappings: [[[0,0,0,0]]]
};
const smc = await new SourceMapConsumer(decoded_map);
smc.originalPositionFor({ line: 1, column: 0 });

error is

TypeError: aStr.charCodeAt is not a function
at BasicSourceMapConsumer._parseMappings
  (node_modules/source-map/lib/source-map-consumer.js:336:29)
at BasicSourceMapConsumer._getMappingsPtr
  (node_modules/source-map/lib/source-map-consumer.js:315:12)
at node_modules/source-map/lib/source-map-consumer.js:514:14
at Object.withMappingCallback
  (node_modules/source-map/lib/wasm.js:95:11)
at BasicSourceMapConsumer.originalPositionFor
  (node_modules/source-map/lib/source-map-consumer.js:512:16)

this should not hurt, no? the test costs 2 nanoseconds on my old laptop

if (typeof aStr != 'string')
  throw new Error('mappings must be encoded');
jkrems commented 3 years ago

Right now the library doesn't do exhaustive type checks on arguments. To prevent these kinds of bugs, it may be better to use a tool like TypeScript or flow to check the code (they cost 0 nanoseconds at runtime). I'll close this issue for now but we may revisit adding type checks on the boundaries in the future.

milahu commented 3 years ago

they cost 0 nanoseconds at runtime

unnecessary encoding + decoding of sourcemaps is expensive, much more than 2 ns

jkrems commented 3 years ago

unnecessary encoding + decoding of sourcemaps is expensive, much more than 2 ns

Definitely! I think a feature request with specific scenarios/test cases where source maps are encoded and/or decoded unnecessarily (and if possible the performance impact) could be very interesting. But I don't think adding the assertion for string would be the solution (and likely also not accepting arrays with numbers directly).