Skip to content

Commit 88670c2

Browse files
committed
fix: assure we don't stop early on NAK if READY was sent to handle V1 specialty (#882).
Seeing READY means a pack will follow, which is exactly the kind of knowledge we want. Thus we now either listen to the client OR to what the server says. Note that this remedy relies on `multi-ack-detailed`, which some very old servers might not support. For those we would be out-of-luck, but that seems acceptable.
1 parent 28d2cf5 commit 88670c2

File tree

2 files changed

+11
-5
lines changed

2 files changed

+11
-5
lines changed

gix-protocol/src/fetch/response/async_io.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ impl Response {
4747
let mut line = String::new();
4848
let mut acks = Vec::<Acknowledgement>::new();
4949
let mut shallows = Vec::<ShallowUpdate>::new();
50+
let mut saw_ready = false;
5051
let has_pack = 'lines: loop {
5152
line.clear();
5253
let peeked_line = match reader.peek_data_line().await {
@@ -81,14 +82,16 @@ impl Response {
8182
if Response::parse_v1_ack_or_shallow_or_assume_pack(&mut acks, &mut shallows, &peeked_line) {
8283
break 'lines true;
8384
}
84-
if let Some(Acknowledgement::Nak) = acks.last().filter(|_| !client_expects_pack) {
85-
break 'lines false;
86-
}
8785
assert_ne!(
8886
reader.readline_str(&mut line).await?,
8987
0,
9088
"consuming a peeked line works"
9189
);
90+
// When the server sends ready, we know there is going to be a pack so no need to stop early.
91+
saw_ready |= matches!(acks.last(), Some(Acknowledgement::Ready));
92+
if let Some(Acknowledgement::Nak) = acks.last().filter(|_| !client_expects_pack && !saw_ready) {
93+
break 'lines false;
94+
}
9295
};
9396
Ok(Response {
9497
acks,

gix-protocol/src/fetch/response/blocking_io.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ impl Response {
4747
let mut line = String::new();
4848
let mut acks = Vec::<Acknowledgement>::new();
4949
let mut shallows = Vec::<ShallowUpdate>::new();
50+
let mut saw_ready = false;
5051
let has_pack = 'lines: loop {
5152
line.clear();
5253
let peeked_line = match reader.peek_data_line() {
@@ -81,10 +82,12 @@ impl Response {
8182
if Response::parse_v1_ack_or_shallow_or_assume_pack(&mut acks, &mut shallows, &peeked_line) {
8283
break 'lines true;
8384
}
84-
if let Some(Acknowledgement::Nak) = acks.last().filter(|_| !client_expects_pack) {
85+
assert_ne!(reader.readline_str(&mut line)?, 0, "consuming a peeked line works");
86+
// When the server sends ready, we know there is going to be a pack so no need to stop early.
87+
saw_ready |= matches!(acks.last(), Some(Acknowledgement::Ready));
88+
if let Some(Acknowledgement::Nak) = acks.last().filter(|_| !client_expects_pack && !saw_ready) {
8589
break 'lines false;
8690
}
87-
assert_ne!(reader.readline_str(&mut line)?, 0, "consuming a peeked line works");
8891
};
8992
Ok(Response {
9093
acks,

0 commit comments

Comments
 (0)