Skip to content

Commit

Permalink
fix: assure we don't stop early on NAK if READY was sent to handle V1…
Browse files Browse the repository at this point in the history
… 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.
  • Loading branch information
Byron committed Jun 7, 2023
1 parent 28d2cf5 commit 53fb504
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 5 deletions.
9 changes: 6 additions & 3 deletions gix-protocol/src/fetch/response/async_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ impl Response {
let mut line = String::new();
let mut acks = Vec::<Acknowledgement>::new();
let mut shallows = Vec::<ShallowUpdate>::new();
let mut saw_ready = false;
let has_pack = 'lines: loop {
line.clear();
let peeked_line = match reader.peek_data_line().await {
Expand Down Expand Up @@ -81,14 +82,16 @@ impl Response {
if Response::parse_v1_ack_or_shallow_or_assume_pack(&mut acks, &mut shallows, &peeked_line) {
break 'lines true;
}
if let Some(Acknowledgement::Nak) = acks.last().filter(|_| !client_expects_pack) {
break 'lines false;
}
assert_ne!(
reader.readline_str(&mut line).await?,
0,
"consuming a peeked line works"
);
// When the server sends ready, we know there is going to be a pack so no need to stop early.
saw_ready |= matches!(acks.last(), Some(Acknowledgement::Ready));
if let Some(Acknowledgement::Nak) = acks.last().filter(|_| !client_expects_pack && !saw_ready) {
break 'lines false;
}
};
Ok(Response {
acks,
Expand Down
7 changes: 5 additions & 2 deletions gix-protocol/src/fetch/response/blocking_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ impl Response {
let mut line = String::new();
let mut acks = Vec::<Acknowledgement>::new();
let mut shallows = Vec::<ShallowUpdate>::new();
let mut saw_ready = false;
let has_pack = 'lines: loop {
line.clear();
let peeked_line = match reader.peek_data_line() {
Expand Down Expand Up @@ -81,10 +82,12 @@ impl Response {
if Response::parse_v1_ack_or_shallow_or_assume_pack(&mut acks, &mut shallows, &peeked_line) {
break 'lines true;
}
if let Some(Acknowledgement::Nak) = acks.last().filter(|_| !client_expects_pack) {
assert_ne!(reader.readline_str(&mut line)?, 0, "consuming a peeked line works");
// When the server sends ready, we know there is going to be a pack so no need to stop early.
saw_ready |= matches!(acks.last(), Some(Acknowledgement::Ready));
if let Some(Acknowledgement::Nak) = acks.last().filter(|_| !client_expects_pack && !saw_ready) {
break 'lines false;
}
assert_ne!(reader.readline_str(&mut line)?, 0, "consuming a peeked line works");
};
Ok(Response {
acks,
Expand Down

0 comments on commit 53fb504

Please sign in to comment.