Excessive memory use when parsing certain ID3 headers about music-metadata

[bjorn-nesby]

As a temporary fix, I've modified the ID3v2Parser.parseMetadata method to bail out when more than 100K errors have been generated. The metadata is still extracted from the test file I supplied, and the library is passing all mocha tests.

If someone wants me to put up a PR, I'd happy to do so.

[Borewit]

I had look to your sample. In Node.js in debug mode, it takes 3.9 seconds to parse the file, no issues. A browser may be more sensitive. To have this kind of ID3 header does not make any sense. There is always reverse engineer some data to entertain the parser.

[bjorn-nesby]

Just to clarify, I didn't engineer these files, they appeared as part of a commercial sample pack*. And as things are, a node.js application might very well freeze or crash when the V8 engine isn't able to allocate more resources.

For example, here are some details from such a crash:

<--- Last few GCs --->

[3425:0x7fc070231000]   177340 ms: Scavenge 1995.2 (2064.7) -> 1992.8 (2065.9) MB, 3.6 / 0.0 ms  (average mu = 0.659, current mu = 0.404) allocation failure; 
[3425:0x7fc070231000]   177348 ms: Scavenge 1996.1 (2065.9) -> 1993.7 (2066.7) MB, 3.4 / 0.0 ms  (average mu = 0.659, current mu = 0.404) allocation failure; 
[3425:0x7fc070231000]   179108 ms: Scavenge 1997.0 (2066.7) -> 1994.5 (2074.9) MB, 1755.3 / 0.0 ms  (average mu = 0.659, current mu = 0.404) allocation failure; 


<--- JS stacktrace --->

[3425:0229/105646.552138:ERROR:v8_initializer.cc(714)] V8 javascript OOM (Reached heap limit).
[0229/105646.566016:WARNING:process_memory_mac.cc(93)] mach_vm_read(0x7ffeedcf0000, 0x2000): (os/kern) invalid address (1)
[3417:0229/105646.852725:INFO:render_frame_host_impl.cc(11664)] RenderFrameHostImpl::MaybeGenerateCrashReport url = file:///index.html, status = 3, exit_code = 5

There is always reverse engineer some data to entertain the parser.

Indeed. But isn't that to be expected when parsing random files in the wild? Some are bound to be broken in one or the other way (I guess you have plenty of experience here). And your library even recognizes that something isn't right with the file I provided, because it's logging a zillion errors while parsing it.

So I guess my approach is just a different one: once huge amounts of errors start piling up, I believe it's good to have a way to bail out.

[*] I silenced the audio to avoid copyright issues