Sign in

flagon-io/g1t

public

Commit

A fetch whose haves the store does not know gets its pack: g1t ends the store's early answer where git expects

The git store answers a protocol v2 fetch that is still negotiating, and whose haves it does not know, with acknowledgments, NAK, and then a pack anyway. git refuses that ("expected no other sections to be sent after no 'ready'"). It broke g1t's reviews: the sandbox clones the pull request's fork one commit deep, so its only have is the new commit, which the upstream repository has never seen. For such a request the repos service now reads the start of the answer; acknowledgments without ready are ended with a flush and response-end, as upload-pack does, and the client negotiates again with done. Anything else streams through as before.

syntaqxcommitted Parent70ff875Browse files
1 file+165−10/1 viewed
+165−1
695695 }
696696 }
697697
698+/// Whether a request is a protocol v2 `fetch` still negotiating: it sends
699+/// `have` lines and no `done`, so the answer may be acknowledgments only.
700+fn negotiating(body: &[u8]) -> bool {
701+ let Some(packets) = packets(body) else { return false };
702+ let lines: Vec<&[u8]> = packets
703+ .iter()
704+ .filter_map(|packet| match packet {
705+ Packet::Data(data) => Some(data.strip_suffix(b"\n").unwrap_or(data)),
706+ Packet::Special(_) => None,
707+ })
708+ .collect();
709+ lines.contains(&b"command=fetch".as_slice())
710+ && lines.iter().any(|line| line.starts_with(b"have "))
711+ && !lines.contains(&b"done".as_slice())
712+}
713+
714+/// What to do with the start of a store's answer to a negotiating fetch.
715+#[derive(Debug, PartialEq, Eq)]
716+enum Acknowledged {
717+ /// Not enough of it yet to tell.
718+ NeedMore,
719+ /// Send it on as it is.
720+ Whole,
721+ /// Acknowledgments without `ready`, followed by more sections: the
722+ /// store's answer to keep is these first bytes, ended by a flush.
723+ CutAt(usize),
724+}
725+
726+/// How much of the answer to keep. The git store answers a fetch whose
727+/// `have`s it does not know with `acknowledgments`, `NAK`, then a pack
728+/// anyway; git refuses that ("expected no other sections to be sent after
729+/// no 'ready'"), since a server that is not ready must end the response
730+/// there and let the client negotiate again. Lines may be `sideband-all`
731+/// framed (band 1, `\x01`).
732+fn acknowledged(head: &[u8]) -> Acknowledged {
733+ let mut position = 0;
734+ let mut first = true;
735+ loop {
736+ let Some(header) = head.get(position..position + 4) else { return Acknowledged::NeedMore };
737+ let Some(length) = std::str::from_utf8(header).ok().and_then(|hex| usize::from_str_radix(hex, 16).ok()) else {
738+ return Acknowledged::Whole;
739+ };
740+ if length < 4 {
741+ // The acknowledgments section's end: a delimiter means more
742+ // sections follow, which only `ready` allows.
743+ return match (first, header) {
744+ (false, b"0001") => Acknowledged::CutAt(position),
745+ _ => Acknowledged::Whole,
746+ };
747+ }
748+ let Some(payload) = head.get(position + 4..position + length) else { return Acknowledged::NeedMore };
749+ let line = payload.strip_prefix(b"\x01").unwrap_or(payload);
750+ let line = line.strip_suffix(b"\n").unwrap_or(line);
751+ if first && line != b"acknowledgments" {
752+ return Acknowledged::Whole;
753+ }
754+ if line == b"ready" {
755+ return Acknowledged::Whole;
756+ }
757+ first = false;
758+ position += length;
759+ }
760+}
761+
762+/// The answer to a negotiating fetch, with the sections the store sent
763+/// after acknowledgments without `ready` left off (see [`acknowledged`]).
764+/// Reads only the start of the answer; the rest streams through.
765+async fn without_early_pack(mut response: Response) -> Result<Response> {
766+ const LOOK: usize = 64 * 1024;
767+ let headers = response.headers().clone();
768+ headers.delete("content-length")?;
769+ let mut stream = response.stream()?;
770+ let mut head = Vec::new();
771+ loop {
772+ match acknowledged(&head) {
773+ Acknowledged::CutAt(at) => {
774+ head.truncate(at);
775+ // A flush ends the acknowledgments; a response-end packet
776+ // ends the stateless answer, as upload-pack's does.
777+ head.extend_from_slice(b"00000002");
778+ return Ok(Response::from_bytes(head)?.with_headers(headers));
779+ }
780+ Acknowledged::Whole => break,
781+ Acknowledged::NeedMore if head.len() >= LOOK => break,
782+ Acknowledged::NeedMore => match stream.next().await {
783+ Some(chunk) => head.extend_from_slice(&chunk?),
784+ None => break,
785+ },
786+ }
787+ }
788+ let rest = futures_util::stream::once(async move { Ok::<Vec<u8>, worker::Error>(head) }).chain(stream);
789+ Ok(Response::from_stream(rest)?.with_headers(headers))
790+}
791+
698792 /// Sends the request on to the git store and returns its response as is,
699793 /// unless it is a push that would change the `protected` branch, one the
700794 /// store could not hold (`limits`, pack_limits.rs), or one that `scan`
797891 let body = with_head(&body, branch).unwrap_or(body);
798892 response = Response::from_bytes(body)?.with_headers(headers);
799893 }
894+ if git.endpoint == "git-upload-pack"
895+ && response.status_code() == 200
896+ && body.as_deref().is_some_and(negotiating)
897+ {
898+ response = without_early_pack(response).await?;
899+ }
800900 Ok(Push::Forwarded(Forwarded {
801901 response,
802902 pushed: Vec::new(),
9481048
9491049 #[cfg(test)]
9501050 mod tests {
951− use super::{Pushed, RepoPath, Url, ZERO_ID, framed, pack_bytes, pushed_branches, refusal, server_timing, transferred, with_head, with_namespace};
1051+ use super::{Acknowledged, Pushed, RepoPath, Url, ZERO_ID, acknowledged, framed, negotiating, pack_bytes, pushed_branches, refusal, server_timing, transferred, with_head, with_namespace};
9521052
9531053 #[test]
9541054 fn server_timing_names_each_step_and_the_total() {
10741174 format!("{:04x}{payload}", payload.len() + 4).into_bytes()
10751175 }
10761176
1177+ fn joined(parts: &[&[u8]]) -> Vec<u8> {
1178+ parts.concat()
1179+ }
1180+
1181+ #[test]
1182+ fn a_fetch_with_haves_and_no_done_is_negotiating() {
1183+ let request = |lines: &[&str]| {
1184+ let mut body = joined(&[&pkt("command=fetch\n"), &pkt("object-format=sha1\n"), b"0001"]);
1185+ for line in lines {
1186+ body.extend(pkt(&format!("{line}\n")));
1187+ }
1188+ body.extend(b"0000");
1189+ body
1190+ };
1191+ let want = "want 8407eba58b925619274d012258c2b474a5dbf012";
1192+ let have = "have 55cd670a89a80df4fa9d9f0244c44fbd2ed1db8b";
1193+ assert!(negotiating(&request(&["deepen 1", want, have])));
1194+ assert!(!negotiating(&request(&[want, have, "done"])));
1195+ assert!(!negotiating(&request(&[want, "done"])));
1196+ let ls_refs = joined(&[&pkt("command=ls-refs\n"), b"0001", &pkt("have nothing\n"), b"0000"]);
1197+ assert!(!negotiating(&ls_refs));
1198+ }
1199+
1200+ #[test]
1201+ fn acknowledgments_without_ready_end_the_answer() {
1202+ // What the store sent a shallow fetch whose only `have` it did not
1203+ // know, sideband-all framed: a NAK, then a pack anyway.
1204+ let answer = joined(&[
1205+ &pkt("\x01acknowledgments\n"),
1206+ &pkt("\x01NAK\n"),
1207+ b"0001",
1208+ &pkt("\x01shallow-info\n"),
1209+ &pkt("\x01shallow 8407eba58b925619274d012258c2b474a5dbf012\n"),
1210+ b"0001",
1211+ &pkt("\x01packfile\n"),
1212+ ]);
1213+ let cut = joined(&[&pkt("\x01acknowledgments\n"), &pkt("\x01NAK\n")]).len();
1214+ assert_eq!(acknowledged(&answer), Acknowledged::CutAt(cut));
1215+ // Not yet at the section's end.
1216+ assert_eq!(acknowledged(&answer[..cut - 2]), Acknowledged::NeedMore);
1217+ assert_eq!(acknowledged(&answer[..cut]), Acknowledged::NeedMore);
1218+ // Without sideband framing too.
1219+ let plain = joined(&[&pkt("acknowledgments\n"), &pkt("ACK abc\n"), b"0001", &pkt("packfile\n")]);
1220+ assert!(matches!(acknowledged(&plain), Acknowledged::CutAt(_)));
1221+ }
1222+
1223+ #[test]
1224+ fn a_ready_store_or_a_plain_pack_streams_through() {
1225+ let ready = joined(&[
1226+ &pkt("\x01acknowledgments\n"),
1227+ &pkt("\x01ACK bab14ff1b6d9c4918100098009747d776759a967\n"),
1228+ &pkt("\x01ready\n"),
1229+ b"0001",
1230+ &pkt("\x01packfile\n"),
1231+ ]);
1232+ assert_eq!(acknowledged(&ready), Acknowledged::Whole);
1233+ // Acknowledgments only, ended by a flush: already right.
1234+ let only = joined(&[&pkt("acknowledgments\n"), &pkt("NAK\n"), b"0000"]);
1235+ assert_eq!(acknowledged(&only), Acknowledged::Whole);
1236+ let pack = joined(&[&pkt("\x01packfile\n"), b"0000"]);
1237+ assert_eq!(acknowledged(&pack), Acknowledged::Whole);
1238+ assert_eq!(acknowledged(b"00"), Acknowledged::NeedMore);
1239+ }
1240+
10771241 #[test]
10781242 fn pushed_branches_are_read_from_the_commands() {
10791243 let old = "c71546fcd893ef8b0f57388b65e620d759705dda";