crates/gunnar-lfs/tests/batch_answers.rs
//! **What a `batch` answers for each object**, asserted on the wire.
//!
//! Four cells, and until 2026-08-05 one of them was wrong in a way no test in
//! this crate could see: a `download` batch for an object the server does not
//! hold answered `noop`, and git-lfs reads `noop` as *nothing to do* rather than
//! *not here*, so `git lfs fetch` reported success having fetched nothing.
//!
//! ```text
//! held not held
//! upload noop upload
//! download download download ← was `noop`
//! ```
//!
//! The pure-SSH grammar has no per-object error to say it with —
//! `batch-oid-line = PKT-LINE(oid size action *(key=[value]) LF)`, and unknown
//! keys are specified to be ignored — so the truth has to be told one command
//! later, by `get-object`'s 404. Handing back the `download` action is what makes
//! the client ask. See `Session::batch` for the full reasoning and the sources.
//!
//! `tests/interop.rs` proves the end of that chain with the real client;
//! this file pins the bytes, which is where a regression would start.
use std::io::BufRead;
use gunnar_lfs::{MemLfsStore, Operation, Session};
/// `version` → `batch` naming `held` and one oid the store does not have →
/// `quit`.
fn batch_script(held: &str, absent: &str, hash_algo: &str) -> Vec<u8> {
let mut s = Vec::new();
gunnar_pktline::write_text(&mut s, b"version 1").unwrap();
gunnar_pktline::write_flush(&mut s).unwrap();
gunnar_pktline::write_text(&mut s, b"batch").unwrap();
gunnar_pktline::write_text(&mut s, b"transfer=ssh").unwrap();
gunnar_pktline::write_text(&mut s, format!("hash-algo={hash_algo}").as_bytes()).unwrap();
gunnar_pktline::write_delim(&mut s).unwrap();
gunnar_pktline::write_text(&mut s, format!("{held} 3").as_bytes()).unwrap();
gunnar_pktline::write_text(&mut s, format!("{absent} 99").as_bytes()).unwrap();
gunnar_pktline::write_flush(&mut s).unwrap();
gunnar_pktline::write_text(&mut s, b"quit").unwrap();
gunnar_pktline::write_flush(&mut s).unwrap();
s
}
/// The action the server named for `oid`, read back out of the wire.
///
/// Parsed from the bytes rather than taken from a return value: the return value
/// was right the whole time this was broken.
fn action_for(wire: &[u8], oid: &str) -> String {
String::from_utf8_lossy(wire)
.lines()
.find_map(|line| {
let rest = line.split_once(oid)?.1;
// `<oid> <size> <action>[ key=value…]`
let mut fields = rest.split_whitespace();
let _size = fields.next()?;
Some(fields.next()?.to_string())
})
.unwrap_or_else(|| {
panic!(
"the batch response never mentioned {oid}:\n{}",
String::from_utf8_lossy(wire).escape_debug()
)
})
}
fn statuses(wire: &[u8]) -> Vec<u16> {
String::from_utf8_lossy(wire)
.split("status ")
.skip(1)
.filter_map(|rest| rest.split('\n').next()?.trim().parse().ok())
.collect()
}
fn serve(script: Vec<u8>, store: &MemLfsStore, operation: Operation) -> Vec<u8> {
let mut input = std::io::BufReader::new(std::io::Cursor::new(script));
let mut output = Vec::new();
let reader: &mut dyn BufRead = &mut input;
Session::new(store, operation)
.serve(reader, &mut output)
.expect("the session must not fail");
output
}
/// A store holding `abc`, plus an oid it does not hold. Returns
/// `(store, held, absent)`.
fn fixture() -> (MemLfsStore, String, String) {
let store = MemLfsStore::new();
let held = store.insert(b"abc").to_hex();
let absent = "0".repeat(64);
assert_ne!(held, absent);
(store, held, absent)
}
/// **The defect, at the byte level.** Both cells of the `download` row.
#[test]
fn a_download_batch_offers_download_for_an_object_it_does_not_hold() {
let (store, held, absent) = fixture();
let wire = serve(
batch_script(&held, &absent, "sha256"),
&store,
Operation::Download,
);
assert_eq!(
action_for(&wire, &absent),
"download",
"a download batch answered `noop` for an absent object. git-lfs leaves \
such an object with no action and skips it silently \
(tq/transfer_queue.go), so the fetch reports success having fetched \
nothing. There is no per-object error in the SSH grammar; the 404 has \
to come from `get-object`, and it never will unless the client is \
given an action to ask with.\nwire: {}",
String::from_utf8_lossy(&wire).escape_debug()
);
// The other cell of the same row, so this is a row and not a constant.
assert_eq!(action_for(&wire, &held), "download");
assert_eq!(statuses(&wire), vec![200, 200, 200]);
}
/// **The `upload` row, which is the red half of the one above.** `noop` is still
/// correct here and still sent: an object the server already holds needs no
/// action, and that is the answer that stops the client re-uploading it. A fix
/// that simply deleted `noop` would pass the test above and break every push of
/// an object the server already has.
#[test]
fn an_upload_batch_still_answers_noop_for_what_it_already_holds() {
let (store, held, absent) = fixture();
let wire = serve(
batch_script(&held, &absent, "sha256"),
&store,
Operation::Upload,
);
assert_eq!(
action_for(&wire, &held),
"noop",
"an upload batch must say `noop` for an object the server already \
holds, or the client re-sends every object on every push.\nwire: {}",
String::from_utf8_lossy(&wire).escape_debug()
);
assert_eq!(
action_for(&wire, &absent),
"upload",
"an upload batch must ask for what is missing.\nwire: {}",
String::from_utf8_lossy(&wire).escape_debug()
);
}
/// The two rows must **differ**. Without this, a server that answered the same
/// thing for every (operation, presence) pair would satisfy both tests above
/// one at a time and neither would notice.
#[test]
fn the_operation_changes_the_answer() {
let (store, held, absent) = fixture();
let up = serve(
batch_script(&held, &absent, "sha256"),
&store,
Operation::Upload,
);
let down = serve(
batch_script(&held, &absent, "sha256"),
&store,
Operation::Download,
);
assert_ne!(
action_for(&up, &held),
action_for(&down, &held),
"the same object gets the same action on an upload and a download \
session; the operation is not being read"
);
}
/// **409, not 405.** `docs/api/batch.md` gives this condition a code by name:
/// *"409 - The specified hash algorithm disagrees with the server's acceptable
/// options."* It is the only code the specification assigns it. The reference
/// server puts it in a generic not-allowed bucket and answers 405; git-lfs
/// treats any non-200 batch status identically (`tq/ssh.go`, `Batch`), so saying
/// what the specification says costs nothing and reads correctly in a log.
#[test]
fn a_hash_algorithm_the_server_cannot_speak_is_409() {
let (store, held, absent) = fixture();
let wire = serve(
batch_script(&held, &absent, "sha1"),
&store,
Operation::Download,
);
assert_eq!(
statuses(&wire),
// version 200, batch 409, quit 200 — and the 200 on `quit` is the proof
// that the refusal read the object lines out and left the stream in
// sync, rather than leaving them to be parsed as commands.
vec![200, 409, 200],
"wire: {}",
String::from_utf8_lossy(&wire).escape_debug()
);
assert!(
String::from_utf8_lossy(&wire).contains("sha1"),
"the 409 must name the algorithm that was refused:\n{}",
String::from_utf8_lossy(&wire).escape_debug()
);
// And no object line escaped alongside the refusal.
assert!(
!String::from_utf8_lossy(&wire).contains(&held),
"a refused batch still listed an object:\n{}",
String::from_utf8_lossy(&wire).escape_debug()
);
// The red half: `sha256` and the empty value are both accepted, so 409 is
// not simply what this server says to every batch.
for accepted in ["sha256", ""] {
let ok = serve(
batch_script(&held, &absent, accepted),
&store,
Operation::Download,
);
assert_eq!(
statuses(&ok),
vec![200, 200, 200],
"`hash-algo={accepted}` must be accepted.\nwire: {}",
String::from_utf8_lossy(&ok).escape_debug()
);
}
}