zlib different on OSX and Linux in newest releases

[flippmoke]

It appears that after #10980, there are some issues with zlib specifically producing different results on OSX during gzip compression. This can be reproduced by the small test below, where there is a single byte difference in the zlib results. This same test does not produce different then expected results on Linux.

var zlib = require('zlib');

var uncompressed = Buffer.from("1ad40178020a05776f726c642880201a0441524541220420b8f3371a044649505322040a0255531a0449534f321a0449534f3322050a035553411a034c4154220919560e2db29dcf43401a034c4f4e220919105839b4c8a658c01a044e414d45220f0a0d556e69746564205374617465731a07504f5032303035220620b196fd8e011a06524547494f4e220220131a09535542524547494f4e220220151a02554e220320c806122f18032210098042ff011a008044ff430000ff430f08cf01121600000101020103020403050406050706080709080a091ad50178020a06776f726c64322880201a0441524541220420b8f3371a044649505322040a0255531a0449534f321a0449534f3322050a035553411a034c4154220919560e2db29dcf43401a034c4f4e220919105839b4c8a658c01a044e414d45220f0a0d556e69746564205374617465731a07504f5032303035220620b196fd8e011a06524547494f4e220220131a09535542524547494f4e220220151a02554e220320c806122f18032210098042ff011a008044ff430000ff430f08cf01121600000101020103020403050406050706080709080a09", "hex");

var compressed = Buffer.from("1f8b080000000000000393bac258c1c4c55a9e5f9493a2d1a020c5e218e4eaa8c4a2b0e3b3b9148b9b6740b0120b175368b0148b67b0bf1198345662e5620e0d769462f6710c51e2940ce3d3dd34f7bcb30390efef07e40b44586e39b12ce280148b9fa3afab123f176f685e66496a8a42704962496ab1147b807f80918181a9129bc2c6697ffb18a5d8825cdd3d815a991484a53883439de05c5129a6503f256685136c42fa12cc4a029c0d4eff19a5181a5cfe3b3330fc77e6e738cf2824c6c0c0c8c8c4c8ccc4c2cccac2c6cacec6c1cec9c1c5297515e42d36b0b78c8691bf005ccccc9baf010000", "hex");

zlib.gzip(uncompressed, {strategy: zlib.Z_DEFAULT_STRATEGY}, function(err, buffer) {
    if (buffer.toString('hex') === compressed.toString('hex')) {
        console.log("Results are the same");
    } else {
        console.log("Results are different");
    }
});

• Version: v4.8.1
• Platform: OS-X
• Output: Results are the same

---

• Version: v4.8.1
• Platform: Linux
• Output: Results are the same

---

• Version: v4.8.2
• Platform: OS-X
• Output: Results are different

---

• Version: v4.8.2
• Platform: Linux
• Output: Results are the same

I believe this also affects the v6 latest release, but have not tested it yet.

[gibfahn]

cc/ @sam-github

[rmg]

This seems to only be compression, not decompression.

zlib.gunzip(compressed, function(err, buffer) {
    if (buffer.toString('hex') === uncompressed.toString('hex')) {
        console.log("Decompression results are the same");
    } else {
        console.log("Decompression results are different");
    }
});

• Version: v4.8.2
• Platform: macOS
• Output: Decompression results are the same

Does zlib/gzip make any guarantees about always producing the same compressed output? It shouldn't be required as long as the compressed results can be decompressed by a compatible implementation to produce the same input.

[rmg]

I think this may be an upstream change.

In the new version of zlib, if __APPLE__ is defined (which it seems to be on our macOS builds) then OS_CODE is 19 instead of 3. Since this value is included in the gzip header, it would slightly change the resulting compression output.

Commented: https://github.com/nodejs/node/pull/10980/files#r110047791

That's my current theory, at least.

[MylesBorins]

/cc @nodejs/lts

[shigeki]

The change of version made of header was made in https://github.com/madler/zlib/commit/ce12c5cd00628bf8f680c98123a369974d32df15 in order to follow the spec of https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT. I don't think this is an issue because a compressed binary is platform dependent as it was already different between Linux and Windows and it does not affect compress/decompress functionaries.

[rmg]

@MylesBorins I'm not sure it would be worth the effort to float a patch for this on an lts branch.

@flippmoke is this causing an actual problem? Or is it just a change in behaviour that wasn't expected?

[flippmoke]

@rmg It is not an actual issue with out code base, we were just using node.js zlib to compare against our C++ implementation inside a node application (https://github.com/mapnik/node-mapnik) for regular testing. We have been building off the system zlib 1.2.8 in OSX so suddenly the results were not the same. I would have just figured it was a slight difference due to a new zlib version but the fact that linux results were still the same made me wonder if it was a bug.

We could change our testing techniques and perhaps and ignore the header of the gzip compressed binary when we compare the results.

[sam-github]

Or perhaps don't compare the zipped data, but unzip it and compare that to an expected value? That would be robust against implementation.

[flippmoke]

@sam-github we could definitely do that, but part of the testing was to compare that all of our options that could be passed in for different configurations of zlib were all going in and creating reasonable results as well, so it really is nice to compare directly to another compressed result. I think if I can properly determine where the header of the zlib compression ends that I should be able to only test against the results after that point and achieve the same results.

[rmg]

I just saw another instance of this causing an inexplicable test failure and the only reason I knew the cause was because I had dug in to this issue, and even that was only possible because @flippmoke had already done the hard work of identifying that it was a gzip header change.

Spelling it out, I now realize how unreasonable it would be to expect the average node user to figure out what happened.

@nodejs/lts I take back my previous stance. This part of the zlib upgrade should be reverted as part of an LTS patch release.

[MylesBorins]

@rmg could you please send a PR with the revert to both 4.x and 6.x?

[sam-github]

@MylesBorins I am looking at this now. I'll PR a test and revert of the header change.

[Trott]

Given that #12404 was closed without merging, should this remain open? I strongly suspect 4.x is going to go EOL before this is fixed, but someone is welcome to prove me wrong by fixing it. :-D

[sam-github]

should close, the PR was rejected.