New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 611738 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit 29 days ago
Closed: Aug 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

[source maps] _parseMap in SourceMap.js does not strictly follow specs

Reported by antonin....@gmail.com, May 13 2016

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/50.0.2661.94 Safari/537.36

Steps to reproduce the problem:
Currently mappings parser reuses previous name if nameIndex is not present in current segment. But this is wrong behaviour.

According to specs[1] name should be assigned to segment only if nameIndex is present.

Current behaviour:
https://github.com/binaryage/dirac/blob/15c21bc73f606ab8791219279d4dbbecb900718b/resources/unpacked/devtools/front_end/sdk/SourceMap.js#L460-L463

Proposed fix:
https://github.com/binaryage/dirac/commit/5d12448fafb8720a00aeee90f5d21113db059f16

Background story:
I have encountered this issue when consuming ClojureScript source maps. ClojureScript being a LISP can generate code via macros during compilation. Because the source was expanded, there is no mapping from generated javascript variables to original source code variables. This _parseMap bug caused assignment of immediate previous name to these "unmapped" variables which should have no name assigned.

I have also consulted Mozilla's implementation of their source-map[2] library and they strictly follow the specs[3].

[1] https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/edit?pli=1#heading=h.qz3o9nc69um5
[2] https://github.com/mozilla/source-map
[3] https://github.com/mozilla/source-map/blob/aa0398ced67beebea34f0d36f766505656c344f6/lib/source-map-consumer.js#L505-L509

What is the expected behavior?

What went wrong?
Resulted in incorrect name mappings read from ClojureScript source maps.

Did this work before? N/A 

Chrome version: 52.0.2734.0  Channel: canary
OS Version: OS X 10.11.5
Flash Version: Shockwave Flash 21.0 r0
 
Also added this sanity check modelled after previous code:
https://github.com/binaryage/dirac/commit/ec141071b0cea3a03826145a2bf71c080bbccbaf
Owner: lushnikov@chromium.org
Status: Assigned (was: Unconfirmed)
Andrey, please take a look.
Would you like to upload a patch? The link on how to do this: https://docs.google.com/document/d/1WNF-KqRSzPLUUfZqQG5AFeU_Ll8TfWYcJasa_XGf7ro/view#heading=h.cpwtdeg8y01b
Status: Fixed (was: Assigned)
Thanks very much for hunting this down and contributing this fix!
Thanks. Now I will be likely to attach patches to reported issues in future :-)

Sign in to add a comment