Skip to content

Commit a89a443

Browse files
committed
no panics in packet line to let caller handle invariants; read…
…read wont' skip over non-data lines either, but fail with an unexpected EOF. This is usually an error that really has to be handled or should abort the current operation.
1 parent e214df5 commit a89a443

File tree

4 files changed

+49
-31
lines changed

4 files changed

+49
-31
lines changed

git-packetline/src/borrowed.rs

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,43 +21,31 @@ impl<'a> Borrowed<'a> {
2121
}
2222
}
2323

24-
pub fn as_slice(&self) -> &[u8] {
24+
pub fn as_slice(&self) -> Option<&[u8]> {
2525
match self {
26-
Borrowed::Data(d) => d,
27-
Borrowed::Flush | Borrowed::Delimiter | Borrowed::ResponseEnd => &[],
26+
Borrowed::Data(d) => Some(d),
27+
Borrowed::Flush | Borrowed::Delimiter | Borrowed::ResponseEnd => None,
2828
}
2929
}
30-
pub fn as_bstr(&self) -> &BStr {
31-
self.as_slice().into()
30+
pub fn as_bstr(&self) -> Option<&BStr> {
31+
self.as_slice().map(Into::into)
3232
}
33-
pub fn to_error(&self) -> Error {
34-
Error(self.as_slice())
33+
pub fn to_error(&self) -> Option<Error> {
34+
self.as_slice().map(Error)
3535
}
36-
pub fn to_text(&self) -> Text {
37-
let d = match self {
38-
Borrowed::Data(d) => *d,
39-
_ => panic!("cannot convert non-data to text"),
40-
};
41-
d.into()
36+
pub fn to_text(&self) -> Option<Text> {
37+
self.as_slice().map(Into::into)
4238
}
43-
pub fn to_band(&self, kind: Channel) -> Band {
44-
let d = match self {
45-
Borrowed::Data(d) => d,
46-
_ => panic!("cannot side-channel non-data lines"),
47-
};
48-
49-
match kind {
39+
pub fn to_band(&self, kind: Channel) -> Option<Band> {
40+
self.as_slice().map(|d| match kind {
5041
Channel::Data => Band::Data(d),
5142
Channel::Progress => Band::Progress(d),
5243
Channel::Error => Band::Error(d),
53-
}
44+
})
5445
}
5546
/// Decode the band of the line, or panic if it is not actually a side-band line
5647
pub fn decode_band(&self) -> Result<Band, DecodeBandError> {
57-
let d = match self {
58-
Borrowed::Data(d) => d,
59-
_ => panic!("cannot decode side-channel information from non-data lines"),
60-
};
48+
let d = self.as_slice().ok_or(DecodeBandError::NonDataLine)?;
6149
Ok(match d[0] {
6250
1 => Band::Data(&d[1..]),
6351
2 => Band::Progress(&d[1..]),
@@ -74,6 +62,9 @@ quick_error! {
7462
InvalidSideBand(band: u8) {
7563
display("attempt to decode a non-side channel line or input was malformed: {}", band)
7664
}
65+
NonDataLine {
66+
display("attempt to decode a non-data line into a side-channel band")
67+
}
7768
}
7869
}
7970

git-packetline/src/read.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,17 @@ where
170170
Band::Error(d) => progress.fail(progress_name(None, Text::from(d).0)),
171171
};
172172
}
173-
None => break line.as_slice().read(&mut self.buf)?,
173+
None => {
174+
break match line.as_slice() {
175+
Some(ref mut d) => d.read(&mut self.buf)?,
176+
None => {
177+
return Err(io::Error::new(
178+
io::ErrorKind::UnexpectedEof,
179+
"encountered non-data line in a data-line only context",
180+
))
181+
}
182+
}
183+
}
174184
}
175185
};
176186
self.pos = 0;

git-packetline/tests/packet_line/decode.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,13 @@ mod streaming {
3535
fn trailing_line_feeds_are_removed_explicitly_roundtrip() -> crate::Result {
3636
match streaming(b"0006a\n")? {
3737
Stream::Complete { line, .. } => {
38-
assert_eq!(line.to_text().0.as_bstr(), b"a".as_bstr());
38+
assert_eq!(line.to_text().expect("text").0.as_bstr(), b"a".as_bstr());
3939
let mut out = Vec::new();
40-
line.to_text().to_write(&mut out).expect("write to memory works");
40+
line.to_text()
41+
.expect("text")
42+
.to_write(&mut out)
43+
.expect("write to memory works");
44+
assert_eq!(out, b"0006a\n", "it appends a newline in text mode");
4145
}
4246
Stream::Incomplete { .. } => panic!("should be complete"),
4347
}
@@ -96,7 +100,10 @@ mod streaming {
96100
#[test]
97101
fn roundtrip_error_line() -> crate::Result {
98102
let mut out = Vec::new();
99-
PacketLine::Data(b"the error").to_error().to_write(&mut out)?;
103+
PacketLine::Data(b"the error")
104+
.to_error()
105+
.expect("data line")
106+
.to_write(&mut out)?;
100107
assert_err_display(streaming(&out), "the error");
101108
Ok(())
102109
}
@@ -105,7 +112,9 @@ mod streaming {
105112
fn roundtrip_side_bands() -> crate::Result {
106113
for channel in &[Channel::Data, Channel::Error, Channel::Progress] {
107114
let mut out = Vec::new();
108-
let band = PacketLine::Data(b"band data").to_band(*channel);
115+
let band = PacketLine::Data(b"band data")
116+
.to_band(*channel)
117+
.expect("data is valid for band");
109118
band.to_write(&mut out)?;
110119
match streaming(&out)? {
111120
Stream::Complete { line, bytes_consumed } => {

git-packetline/tests/packet_line/read.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,15 @@ mod to_read {
2828
assert_eq!(out.as_bstr(), b"808e50d724f604f69ab93c6da2919c014667bedb HEAD\0multi_ack thin-pack side-band side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative no-progress include-tag multi_ack_detailed symref=HEAD:refs/heads/master object-format=sha1 agent=git/2.28.0\n808e50d724f604f69ab93c6da2919c014667bedb refs/heads/master\n".as_bstr());
2929

3030
rd.reset();
31-
assert_eq!(rd.read_line().expect("line")??.to_text().0.as_bstr(), b"NAK".as_bstr());
31+
assert_eq!(
32+
rd.read_line()
33+
.expect("line")??
34+
.to_text()
35+
.expect("data line")
36+
.0
37+
.as_bstr(),
38+
b"NAK".as_bstr()
39+
);
3240
fn no_parsing(_: &[u8]) -> Option<RemoteProgress> {
3341
None
3442
}

0 commit comments

Comments
 (0)