ref:main

UploadPackV2.feed treats delim (0001) as command terminator; output diverges under TCP chunking #29

open Opened by cole.christensen@gmail.com

Links

No links yet.

Problem

`UploadPackV2.feed/2` calls `has_complete_command?/1` which matches on either flush (`0000`) or delim (`0001`). Per protocol v2, a command is terminated by flush; delim is an intra-command separator between the `command=` line and the argument block.

When the input arrives in multiple TCP chunks and the first chunk contains only the `command=…` + delim prefix (no arguments yet, no flush), `has_complete_command?` returns true, the state machine eagerly processes an incomplete request, transitions to `:done`, and ignores subsequent chunks.

Reproduction

New state-machine test in `test/ex_git_objectstore/integration/upload_pack_v2_dataplane_test.exs`:

  • Build a complete fetch request (command=fetch, delim, want , done, flush)
  • Feed it whole → record the response
  • Feed it split at arbitrary random offsets → record the response
  • Compare

With seed=1 and chunk boundaries `[17, 12, 2, 32, 1, 2, 3, 16]`, the chunked response differs from the whole-blob response.

Impact

Latent production bug. Doesn’t typically fire because most git clients send the whole fetch command in a single TCP write that fits in one kernel buffer. Any real TCP fragmentation (high-latency transport, slow client, intermediate proxy) will cause a malformed response that the client can’t parse.

The comment on `has_complete_command?` is also incorrect and misleadingly describes the current behaviour:

```elixir

A complete v2 command is terminated by a flush (0000) or delim (0001) packet.

```

Delim is NOT a terminator.

Fix sketch

`has_complete_command?` should look for a flush packet `0000`, not a delim. The parsing should accept delim anywhere inside the buffer as a section separator.

Tracking test

`test “fetch request response is invariant under TCP chunking”` in the new dataplane integration test file is currently marked `@tag skip:` and references this issue.