processZlib uncompresses incorrect byterange

[TanninOne]

In this line https://github.com/kaitai-io/kaitai_struct_javascript_runtime/blob/7a3533f0e66297abe99c2d619cbf4b3c0a32dd48/KaitaiStream.js#L568

the buffer to inflate is retrieved from Uint8Array from buf.buffer, but that property returns the whole buffer, ignoring byteOffset and length. Thus when only part of a file is compressed and only that slice is to be inflated, processZlib will instead try to inflate the whole file. It should be fine to create the buffer with "var b = new Buffer(buf)"

[GreyCat]

Technically, ksc never compiles the code that would make any use of byteOffset, although as long as it exists, it can be used manually.

Anyway, at the very least, we should create additional test in tests repo for this case, i.e. when the KaitaiStream is created externally with trivial byteOffsets and length. @koczkatamas, what do you think?

[koczkatamas]

Sadly I never really went deep into the JS runtime's implementation. I feel I should understand it better to make valid decisions (eg. on whether byteOffset is required or not).

But generally I think more tests cannot hurt.

[TanninOne]

I can't tell you why, I don't understand the code well enough (yet), but I definitively ran into this problem. The parser crashes with an error message from zlib:

Error: incorrect header check                                                                              
    at Zlib._handle.onerror (zlib.js:355:17)                                                               
    at Inflate.Zlib._processChunk (zlib.js:526:30)                                                         
    at zlibBufferSync (zlib.js:225:17)                                                                     
    at Object.exports.inflateSync (zlib.js:149:10)                                                         
    at Function.KaitaiStream.processZlib (c:\Work\esp\js\node_modules\kaitai-struct\KaitaiStream.js:573:29)

Debugging into it it's trying to extract the whole file instead of the slice in question. replacing "new Buffer(buf.buffer)" with "new Buffer(buf)" fixes that problem

EDIT: the generated parser calls this._raw__raw_fields = this._io.readBytes((this.header.dataSize - 4)); KaitaiStream.readBytes calls return this.mapUint8Array(len);. KaitaiStream.mapUint8Array does new Uint8Array(this._buffer, this.byteOffset + this.pos, length);

[GreyCat]

Um, then it's more serious than I've thought. Can you share the example then?

[TanninOne]

esp.zip

Sorry, I don't have time to create a minimal sample so I've attached the current version of my parser spec (esp.ksy is the top-level file) and a real-world sample file.

[cherue]

I can confirm @TanninOne's findings: inflating uses the whole underlying buffer on Node. His proposed fix also works and is in fact already used for Browsers.

[generalmimon]

Fixed by the above merged PRs. The related test ZlibSurrounded in our CI dashboard passes for all JS targets, so I'm closing this issue.