ref:b18689e7d5623f5e91387a7e467f03bb60f74565

fix: buffer partial data in UploadPack v1 to prevent split-data loss

UploadPack v1 had the same class of bug we fixed in v2: when SSH splits protocol data mid-pkt-line, parse_want_lines and parse_have_lines silently dropped the incomplete data and either prematurely ended negotiation (wants phase) or lost have SHAs (haves phase). Fix: propagate {:need_more, data} from the parsers back to feed/2 so it buffers full_data in negotiate_buffer for the next call — matching the pattern ReceivePack already uses correctly. Also adds 6 new split-data tests: - UploadPack v1: 4 tests (2-part, 3-part, boundary, haves-split) - ReceivePack: 2 tests (pack-phase split, mid-pkt-line command split) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
SHA: b18689e7d5623f5e91387a7e467f03bb60f74565
Author: Cole Christensen <cole.christensen@macmillan.com>
Date: 2026-03-14 02:27
Parents: 1bd54cd
3 files changed +250 -2
Type
lib/ex_git_objectstore/protocol/upload_pack.ex +12 −2
@@ -84,6 +84,10 @@
{:ok, wants, haves, :continue} ->
handle_negotiation_continue(%{state | negotiate_buffer: <<>>}, wants, haves)
{:need_more, _} ->
# Not enough data yet — buffer and wait for more
{<<>>, %{state | negotiate_buffer: full_data}}
{:error, _reason} ->
nak = PktLine.encode("NAK")
{nak, %{state | phase: :done, negotiate_buffer: <<>>}}
@@ -230,11 +234,17 @@
{:ok, haves, :continue} ->
{:ok, wants, haves, :continue}
{:need_more, _} ->
{:need_more, data}
{:error, _} = err ->
err
end
{:need_more, _} ->
{:need_more, data}
{:error, _} = err ->
err
end
@@ -262,7 +272,7 @@
parse_want_lines(rest, acc)
{:need_more, _} ->
{:need_more, data}
{:ok, Enum.reverse(acc), <<>>}
{:error, _} = err ->
err
@@ -295,7 +305,7 @@
parse_have_lines(rest, acc)
{:need_more, _} ->
{:need_more, data}
{:ok, Enum.reverse(acc), :continue}
{:error, _} = err ->
err
test/ex_git_objectstore/protocol/receive_pack_test.exs +105 −0
@@ -282,6 +282,111 @@
end
describe "split data handling" do
test "handles pack data split across multiple feed calls" do
repo = RepoHelper.memory_repo()
ExGitObjectstore.init(repo)
blob = Blob.from_content("hello\n")
{:ok, blob_sha} = Object.write(repo, blob)
tree = Tree.new([%{mode: "100644", name: "file.txt", sha: blob_sha}])
{:ok, tree_sha} = Object.write(repo, tree)
commit = %Commit{
tree: tree_sha,
parents: [],
author: "Test <t@t.com> 1000000000 +0000",
committer: "Test <t@t.com> 1000000000 +0000",
message: "init\n"
}
{:ok, commit_sha} = Object.write(repo, commit)
{_advert, state} = ReceivePack.init(repo)
zero = String.duplicate("0", 40)
commands = PktLine.encode("#{zero} #{commit_sha} refs/heads/main") <> PktLine.flush()
objects = [
{:blob, "hello\n", blob_sha},
{:tree, Tree.encode_content(tree), tree_sha},
{:commit, Commit.encode_content(commit), commit_sha}
]
{pack_data, _} = Writer.generate(objects)
# Send commands in one feed, then split pack across two feeds
{r1, state} = ReceivePack.feed(state, commands)
assert r1 == <<>>
assert state.phase == :pack
# Split the pack data
split_point = div(byte_size(pack_data), 2)
<<pack_part1::binary-size(split_point), pack_part2::binary>> = pack_data
{r2, state} = ReceivePack.feed(state, pack_part1)
assert r2 == <<>>
refute ReceivePack.done?(state)
{r3, state} = ReceivePack.feed(state, pack_part2)
assert ReceivePack.done?(state)
{:ok, packets, _} = PktLine.decode(r3)
data_lines = for {:data, d} <- packets, do: d
assert "unpack ok" in data_lines
assert "ok refs/heads/main" in data_lines
assert {:ok, ^commit_sha} = Ref.get(repo, "refs/heads/main")
end
test "handles command split mid-pkt-line" do
repo = RepoHelper.memory_repo()
ExGitObjectstore.init(repo)
blob = Blob.from_content("hello\n")
{:ok, blob_sha} = Object.write(repo, blob)
tree = Tree.new([%{mode: "100644", name: "file.txt", sha: blob_sha}])
{:ok, tree_sha} = Object.write(repo, tree)
commit = %Commit{
tree: tree_sha,
parents: [],
author: "Test <t@t.com> 1000000000 +0000",
committer: "Test <t@t.com> 1000000000 +0000",
message: "init\n"
}
{:ok, commit_sha} = Object.write(repo, commit)
{_advert, state} = ReceivePack.init(repo)
zero = String.duplicate("0", 40)
commands = PktLine.encode("#{zero} #{commit_sha} refs/heads/main") <> PktLine.flush()
objects = [
{:blob, "hello\n", blob_sha},
{:tree, Tree.encode_content(tree), tree_sha},
{:commit, Commit.encode_content(commit), commit_sha}
]
{pack_data, _} = Writer.generate(objects)
full_data = commands <> pack_data
# Split mid-way through the command pkt-line (before flush)
split_point = 10
<<part1::binary-size(split_point), part2::binary>> = full_data
{r1, state} = ReceivePack.feed(state, part1)
assert r1 == <<>>
assert state.phase == :commands
{r2, state} = ReceivePack.feed(state, part2)
assert ReceivePack.done?(state)
{:ok, packets, _} = PktLine.decode(r2)
data_lines = for {:data, d} <- packets, do: d
assert "unpack ok" in data_lines
end
test "handles split command data across multiple feed calls" do
repo = RepoHelper.memory_repo()
ExGitObjectstore.init(repo)
test/ex_git_objectstore/protocol/upload_pack_test.exs +133 −0
@@ -197,6 +197,139 @@
end
end
describe "buffering split data" do
test "handles clone data split across two feed calls" do
repo = RepoHelper.memory_repo()
ExGitObjectstore.init(repo)
{commit_sha, _, _} = create_commit(repo, "hello\n", "init\n")
:ok = Ref.put(repo, "refs/heads/main", commit_sha, nil)
{_advert, state} = UploadPack.init(repo)
# Build complete clone request
full_data =
PktLine.encode("want #{commit_sha}") <>
PktLine.flush() <>
PktLine.encode("done")
# Split at an arbitrary point mid-pkt-line
split_point = div(byte_size(full_data), 2)
<<part1::binary-size(split_point), part2::binary>> = full_data
# First feed: should buffer, return empty
{response1, state1} = UploadPack.feed(state, part1)
assert response1 == <<>>
assert state1.phase == :negotiation
assert state1.negotiate_buffer == part1
# Second feed: should complete and return packfile
{response2, state2} = UploadPack.feed(state1, part2)
assert UploadPack.done?(state2)
assert byte_size(response2) > 0
# Verify pack data is present and parseable
pack_data = extract_sideband_pack(response2)
assert pack_data != nil
{:ok, entries} = Reader.parse(pack_data)
types = Enum.map(entries, & &1.type) |> MapSet.new()
assert :commit in types
assert :tree in types
assert :blob in types
end
test "handles clone data split into three feed calls" do
repo = RepoHelper.memory_repo()
ExGitObjectstore.init(repo)
{commit_sha, _, _} = create_commit(repo, "content\n", "commit\n")
:ok = Ref.put(repo, "refs/heads/main", commit_sha, nil)
{_advert, state} = UploadPack.init(repo)
full_data =
PktLine.encode("want #{commit_sha}") <>
PktLine.flush() <>
PktLine.encode("done")
# Split into three parts
size = byte_size(full_data)
s1 = div(size, 3)
s2 = div(size * 2, 3)
<<p1::binary-size(s1), rest::binary>> = full_data
<<p2::binary-size(s2 - s1), p3::binary>> = rest
# Feed part 1
{r1, st1} = UploadPack.feed(state, p1)
assert r1 == <<>>
assert st1.phase == :negotiation
# Feed part 2
{r2, st2} = UploadPack.feed(st1, p2)
assert r2 == <<>>
assert st2.phase == :negotiation
# Feed part 3 — should complete
{r3, st3} = UploadPack.feed(st2, p3)
assert UploadPack.done?(st3)
assert byte_size(r3) > 0
end
test "handles split at exact pkt-line boundary" do
repo = RepoHelper.memory_repo()
ExGitObjectstore.init(repo)
{commit_sha, _, _} = create_commit(repo, "test\n", "init\n")
:ok = Ref.put(repo, "refs/heads/main", commit_sha, nil)
{_advert, state} = UploadPack.init(repo)
# Split exactly between the want line and the flush
want_line = PktLine.encode("want #{commit_sha}")
rest_of_data = PktLine.flush() <> PktLine.encode("done")
{r1, st1} = UploadPack.feed(state, want_line)
assert r1 == <<>>
assert st1.phase == :negotiation
{r2, st2} = UploadPack.feed(st1, rest_of_data)
assert UploadPack.done?(st2)
assert byte_size(r2) > 0
end
test "handles fetch with haves split across feeds" do
repo = RepoHelper.memory_repo()
ExGitObjectstore.init(repo)
{c1_sha, _, _} = create_commit(repo, "v1\n", "first\n")
:ok = Ref.put(repo, "refs/heads/main", c1_sha, nil)
{c2_sha, _, _} = create_commit(repo, "v2\n", "second\n", [c1_sha])
:ok = Ref.put(repo, "refs/heads/main", c2_sha, c1_sha)
{_advert, state} = UploadPack.init(repo)
full_data =
PktLine.encode("want #{c2_sha}") <>
PktLine.encode("have #{c1_sha}") <>
PktLine.flush() <>
PktLine.encode("done")
# Split mid-way through the "have" line
want_size = byte_size(PktLine.encode("want #{c2_sha}"))
split_point = want_size + 10
<<part1::binary-size(split_point), part2::binary>> = full_data
{r1, st1} = UploadPack.feed(state, part1)
assert r1 == <<>>
assert st1.phase == :negotiation
{r2, st2} = UploadPack.feed(st1, part2)
assert UploadPack.done?(st2)
assert byte_size(r2) > 0
end
end
# Extract pack data from sideband-encoded pkt-line response.
# The response contains NAK text lines and sideband data lines (band byte 1 = pack).
# PktLine.decode strips newlines from data, so we need raw parsing for sideband.