Z_DATA_ERROR during push parsing

[coopernetes]

The following stack trace is observed when GitProxy is receiving a push. It's not clear what the cause of it is but it seems related to the parsing logic in parsePush step. The same error is hit multiple times in the process of a single push.

Error: incorrect header check
    at genericNodeError (node:internal/errors:984:15)
    at wrappedFn (node:internal/errors:538:14)
    at Zlib.zlibOnError [as onerror] (node:zlib:189:17)
    at Zlib.callbackTrampoline (node:internal/async_hooks:130:17)
    at processChunkSync (node:zlib:415:12)
    at zlibBufferSync (node:zlib:178:12)
    at Object.syncBufferWrapper [as inflateSync] (node:zlib:767:14)
    at unpack (/app/src/proxy/processors/push-action/parsePush.js:234:25)
    at getContent (/app/src/proxy/processors/push-action/parsePush.js:212:35)
    at getContents (/app/src/proxy/processors/push-action/parsePush.js:145:37) {
  errno: -3,
  code: 'Z_DATA_ERROR'
}

It seems to be mostly cosmetic as the remaining steps still proceed as expected. Parsing of the pushed data is only used for extracting certain metadata out of the payload for the purposes of checking out the remote repo, performing diffs against HEAD and base branches and other operations.

The receive pack from git usually contains "delta-fied"¹ chunks of data and I suspect it is not actually zlib compressed hence this error message. We can likely rely solely on the first part of the payload which contains the following info:

• tree + commit sha
• parent + commit sha
• author & committer names & email addresses
• commit message
• GPG signature

This is the expected payload which is successfully parsed even with these errors raised:

{
  x: {
    item: 0,
    value: 159,
    type: 1,
    size: 543,
    deflatedSize: 397,
    objectRef: null,
    content: 'tree {commit sha redacted}\n' +
      'parent {commit sha redacted}\n' +
      'author {redacted} 1747147000 -0400\n' +
      'committer Thomas Cooper <{redacted}> 1748440293 -0400\n' +
      'gpgsig -----BEGIN PGP SIGNATURE-----\n' +
      ' \n' +
      ' {redacted}\n' +
      ' {redacted}\n' +
      ' {redacted}\n' +
      ' {redacted}\n' +
      ' -----END PGP SIGNATURE-----\n' +
      '\n' +
      "{commit message}\n" +
      '\n' +
      'Signed-off-by: {commit sha redacted}\n'
  }
}

Footnotes

1. https://github.com/git/git/blob/master/Documentation/gitprotocol-capabilities.adoc#ofs-delta ↩

[kriswest]

@coopernetes @jescalada I'm investigating why main is only finding the last commit in each push for me and I think it may be related to this. Only the last commit is making it into the parsed commitData in the action.

The Pack metadata is saying theres 16 entries (for 5 commits I'm pushing) to unpack:

[0] Pack Metadata: {
[0]   "sig": "PACK",
[0]   "version": 2,
[0]   "entries": 16
[0] }

The first entry unpacks successfully and appears to contain the details for the last commit I'm pushing. That gets trimmed off and the remaining buffer processed next, however, all subsequent entries fail to unpack with header errors.

If I add another commit and push again I've got 19 entries to unpack (3 per commit + 1?), and the new commit will be the only one to parse before header errors wipe out the rest. Hence, I suspect we're not moving through the buffer correctly somewhere ... Thats got be an issue in parsePush unpack and/or getContents - probably related to how it calculates where the buffer starts again (it deflates the content again to get the compressed length, which is probably a questionable estimate). I'm poking at that with AI and think it might be possible to replace that code with something that works better - it'll need a switch from zlib to another lib like pako that does a better job of exposing how much of the stream its consumed.

I'll give it a go and report back. However, if either of you have any thoughts on what else might be going on, let me know.

[jescalada]

@kriswest What do you mean by "finding only the last commit in each push"? I tried making multiple commits and then pushing and am able to see all the commits (currently testing on your 1175-push-tables-to-render-email branch):

[kriswest]

I'm not able to see the content of all commits. Check out these two screenshots - I'm pushing the same branch, but have added a 6th commit in the second image. Only the laast commit (which is the first in the stream of compressed content) will ever unpack (either the headers or content):

The diff shows the extra changes (as thats created using a checkout), but we're failing to unpack the commits or their headers.

When you unpack commits on your end, presumably you get Z_DATA_ERROR some of the time, I'm getting it for every commit after the first. The headers need to unpack to get the type and size, then the content contains the references to identify the commits - so I assume some must be unpacking for you?

I spent far too many hours hacking on the decompression steps over the weekend trying to resolve this (unsuccessfully) I did learn a few useful things:

• Although the size given in the header is for the uncompressed content, whats supposed to happen is that you compress the stream ofd content until you have that much output, then you consider that object done and move onto the next.
• Our current approach involves inflating the content, then deflating it again to work out how much space it would have taken up
  • This is of course error prone and could be the cause of the errors - if you were out of sync with the stream/cut it in the wrong place, thats the error you would get.
  • There are few Javascript Zlib implementations that are well setup to handle inflation like this
    • need to feed data in a N bytes at a time, then stop/flush when output is expected length, then start on the next object
    • No obvious solution to this with fflate or pako as alternatives to Zlib
    • There is a stream based approach to Zlib, but we'd need to manage the read stream carefully...

[kriswest]

P.S. i'm using git version 2.39.5 (Apple Git-154)

[kriswest]

I've cracked this and have a PR on the way. There were two things at play:

1. We weren't handling variable length headers for type 6 (ofs_delta) correctly
2. Re-deflating the content after inflating it only gives a rough estimate of the correct offset.
   Once you are 1-2 bytes off, the stream will no longer decode.

We were 'getting away with this' most of the time as the ofs_delta objects seem to come after the commit objects (at least they do in the small number of commits I've tested). In my case I think a poor estimate of the deflated size was triggering it early so only the first object would decode. There are myriad of other small nuances that have to be exactly right - its tough!

Re-writing the decompression from the ground-up, I've been able to get everything to decode, including all the deltas. I've got some tests to fix or remove/replace (for example checking inflateSync was called) before I raise the PR.