diff --git a/git-packetline/src/line/mod.rs b/git-packetline/src/line/mod.rs index e3777812807..538630ecc7a 100644 --- a/git-packetline/src/line/mod.rs +++ b/git-packetline/src/line/mod.rs @@ -4,14 +4,14 @@ use crate::{decode, BandRef, Channel, ErrorRef, PacketLineRef, TextRef, ERR_PREF impl<'a> PacketLineRef<'a> { /// Return this instance as slice if it's [`Data`][PacketLineRef::Data]. - pub fn as_slice(&self) -> Option<&[u8]> { + pub fn as_slice(&self) -> Option<&'a [u8]> { match self { PacketLineRef::Data(d) => Some(d), PacketLineRef::Flush | PacketLineRef::Delimiter | PacketLineRef::ResponseEnd => None, } } /// Return this instance's [`as_slice()`][PacketLineRef::as_slice()] as [`BStr`]. - pub fn as_bstr(&self) -> Option<&BStr> { + pub fn as_bstr(&self) -> Option<&'a BStr> { self.as_slice().map(Into::into) } /// Interpret this instance's [`as_slice()`][PacketLineRef::as_slice()] as [`ErrorRef`]. @@ -20,13 +20,13 @@ impl<'a> PacketLineRef<'a> { /// /// Note that this creates an unchecked error using the slice verbatim, which is useful to [serialize it][ErrorRef::write_to()]. /// See [`check_error()`][PacketLineRef::check_error()] for a version that assures the error information is in the expected format. - pub fn as_error(&self) -> Option> { + pub fn as_error(&self) -> Option> { self.as_slice().map(ErrorRef) } /// Check this instance's [`as_slice()`][PacketLineRef::as_slice()] is a valid [`ErrorRef`] and return it. /// /// This works for any data received in an error [channel][crate::Channel]. - pub fn check_error(&self) -> Option> { + pub fn check_error(&self) -> Option> { self.as_slice().and_then(|data| { if data.len() >= ERR_PREFIX.len() && &data[..ERR_PREFIX.len()] == ERR_PREFIX { Some(ErrorRef(&data[ERR_PREFIX.len()..])) @@ -36,7 +36,7 @@ impl<'a> PacketLineRef<'a> { }) } /// Return this instance as text, with the trailing newline truncated if present. - pub fn as_text(&self) -> Option> { + pub fn as_text(&self) -> Option> { self.as_slice().map(Into::into) } @@ -44,7 +44,7 @@ impl<'a> PacketLineRef<'a> { /// /// Note that this is only relevant in a side-band channel. /// See [`decode_band()`][PacketLineRef::decode_band()] in case `kind` is unknown. - pub fn as_band(&self, kind: Channel) -> Option> { + pub fn as_band(&self, kind: Channel) -> Option> { self.as_slice().map(|d| match kind { Channel::Data => BandRef::Data(d), Channel::Progress => BandRef::Progress(d), @@ -53,7 +53,7 @@ impl<'a> PacketLineRef<'a> { } /// Decode the band of this [`slice`][PacketLineRef::as_slice()] - pub fn decode_band(&self) -> Result, decode::band::Error> { + pub fn decode_band(&self) -> Result, decode::band::Error> { let d = self.as_slice().ok_or(decode::band::Error::NonDataLine)?; Ok(match d[0] { 1 => BandRef::Data(&d[1..]), @@ -73,11 +73,11 @@ impl<'a> From<&'a [u8]> for TextRef<'a> { impl<'a> TextRef<'a> { /// Return this instance's data. - pub fn as_slice(&self) -> &[u8] { + pub fn as_slice(&self) -> &'a [u8] { self.0 } /// Return this instance's data as [`BStr`]. - pub fn as_bstr(&self) -> &BStr { + pub fn as_bstr(&self) -> &'a BStr { self.0.into() } } diff --git a/git-packetline/src/read/sidebands/async_io.rs b/git-packetline/src/read/sidebands/async_io.rs index 5c67c16fa78..c8d009ec4a1 100644 --- a/git-packetline/src/read/sidebands/async_io.rs +++ b/git-packetline/src/read/sidebands/async_io.rs @@ -29,7 +29,10 @@ where { fn drop(&mut self) { if let State::Idle { ref mut parent } = self.state { - parent.as_mut().unwrap().reset(); + parent + .as_mut() + .expect("parent is always available if we are idle") + .reset(); } } } @@ -118,14 +121,22 @@ where /// Forwards to the parent [StreamingPeekableIter::reset_with()] pub fn reset_with(&mut self, delimiters: &'static [PacketLineRef<'static>]) { if let State::Idle { ref mut parent } = self.state { - parent.as_mut().unwrap().reset_with(delimiters) + parent + .as_mut() + .expect("parent is always available if we are idle") + .reset_with(delimiters) } } /// Forwards to the parent [StreamingPeekableIter::stopped_at()] pub fn stopped_at(&self) -> Option> { match self.state { - State::Idle { ref parent } => parent.as_ref().unwrap().stopped_at, + State::Idle { ref parent } => { + parent + .as_ref() + .expect("parent is always available if we are idle") + .stopped_at + } _ => None, } } @@ -137,10 +148,19 @@ where /// Effectively forwards to the parent [StreamingPeekableIter::peek_line()], allowing to see what would be returned /// next on a call to [`read_line()`][io::BufRead::read_line()]. - pub async fn peek_data_line(&mut self) -> Option>> { + /// + /// # Warning + /// + /// This skips all sideband handling and may return an unprocessed line with sidebands still contained in it. + pub async fn peek_data_line(&mut self) -> Option>> { match self.state { - State::Idle { ref mut parent } => match parent.as_mut().unwrap().peek_line().await { - Some(Ok(Ok(crate::PacketLineRef::Data(line)))) => Some(Ok(Ok(line))), + State::Idle { ref mut parent } => match parent + .as_mut() + .expect("parent is always available if we are idle") + .peek_line() + .await + { + Some(Ok(Ok(PacketLineRef::Data(line)))) => Some(Ok(Ok(line))), Some(Ok(Err(err))) => Some(Ok(Err(err))), Some(Err(err)) => Some(Err(err)), _ => None, @@ -149,10 +169,55 @@ where } } - /// Read a packet line as line. + /// Read a packet line as string line. pub fn read_line<'b>(&'b mut self, buf: &'b mut String) -> ReadLineFuture<'a, 'b, T, F> { ReadLineFuture { parent: self, buf } } + + /// Read a packet line from the underlying packet reader, returning empty lines if a stop-packetline was reached. + /// + /// # Warning + /// + /// This skips all sideband handling and may return an unprocessed line with sidebands still contained in it. + pub async fn read_data_line(&mut self) -> Option, decode::Error>>> { + match &mut self.state { + State::Idle { parent: Some(parent) } => { + assert_eq!( + self.cap, 0, + "we don't support partial buffers right now - read-line must be used consistently" + ); + parent.read_line().await + } + _ => None, + } + } +} + +pub struct ReadDataLineFuture<'a, 'b, T: AsyncRead, F> { + parent: &'b mut WithSidebands<'a, T, F>, + buf: &'b mut Vec, +} + +impl<'a, 'b, T, F> Future for ReadDataLineFuture<'a, 'b, T, F> +where + T: AsyncRead + Unpin, + F: FnMut(bool, &[u8]) + Unpin, +{ + type Output = std::io::Result; + + fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + assert_eq!( + self.parent.cap, 0, + "we don't support partial buffers right now - read-line must be used consistently" + ); + let Self { buf, parent } = &mut *self; + let line = ready!(Pin::new(parent).poll_fill_buf(cx))?; + buf.clear(); + buf.extend_from_slice(line); + let bytes = line.len(); + self.parent.cap = 0; + Poll::Ready(Ok(bytes)) + } } pub struct ReadLineFuture<'a, 'b, T: AsyncRead, F> { @@ -174,8 +239,7 @@ where ); let Self { buf, parent } = &mut *self; let line = std::str::from_utf8(ready!(Pin::new(parent).poll_fill_buf(cx))?) - .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err)) - .unwrap(); + .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?; buf.clear(); buf.push_str(line); let bytes = line.len(); diff --git a/git-packetline/src/read/sidebands/blocking_io.rs b/git-packetline/src/read/sidebands/blocking_io.rs index e8a83466841..ea67dc09cef 100644 --- a/git-packetline/src/read/sidebands/blocking_io.rs +++ b/git-packetline/src/read/sidebands/blocking_io.rs @@ -84,14 +84,31 @@ where /// Effectively forwards to the parent [StreamingPeekableIter::peek_line()], allowing to see what would be returned /// next on a call to [`read_line()`][io::BufRead::read_line()]. + /// + /// # Warning + /// + /// This skips all sideband handling and may return an unprocessed line with sidebands still contained in it. pub fn peek_data_line(&mut self) -> Option>> { match self.parent.peek_line() { - Some(Ok(Ok(crate::PacketLineRef::Data(line)))) => Some(Ok(Ok(line))), + Some(Ok(Ok(PacketLineRef::Data(line)))) => Some(Ok(Ok(line))), Some(Ok(Err(err))) => Some(Ok(Err(err))), Some(Err(err)) => Some(Err(err)), _ => None, } } + + /// Read a whole packetline from the underlying reader, with empty lines indicating a stop packetline. + /// + /// # Warning + /// + /// This skips all sideband handling and may return an unprocessed line with sidebands still contained in it. + pub fn read_data_line(&mut self) -> Option, crate::decode::Error>>> { + assert_eq!( + self.cap, 0, + "we don't support partial buffers right now - read-line must be used consistently" + ); + self.parent.read_line() + } } impl<'a, T, F> BufRead for WithSidebands<'a, T, F> @@ -157,9 +174,7 @@ where self.cap, 0, "we don't support partial buffers right now - read-line must be used consistently" ); - let line = std::str::from_utf8(self.fill_buf()?) - .map_err(|err| io::Error::new(io::ErrorKind::Other, err)) - .unwrap(); + let line = std::str::from_utf8(self.fill_buf()?).map_err(|err| io::Error::new(io::ErrorKind::Other, err))?; buf.push_str(line); let bytes = line.len(); self.cap = 0; diff --git a/git-packetline/tests/read/sideband.rs b/git-packetline/tests/read/sideband.rs index 5484b598216..6762ced03d2 100644 --- a/git-packetline/tests/read/sideband.rs +++ b/git-packetline/tests/read/sideband.rs @@ -144,6 +144,52 @@ async fn read_line_trait_method_reads_one_packet_line_at_a_time() -> crate::Resu Ok(()) } +#[maybe_async::test(feature = "blocking-io", async(feature = "async-io", async_std::test))] +async fn readline_reads_one_packet_line_at_a_time() -> crate::Result { + let buf = fixture_bytes("v1/01-clone.combined-output-no-binary"); + + let mut rd = git_packetline::StreamingPeekableIter::new(&buf[..], &[PacketLineRef::Flush]); + + let mut r = rd.as_read(); + let line = r.read_data_line().await.unwrap()??.as_bstr().unwrap(); + assert_eq!(line, "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\n"); + let line = r.read_data_line().await.unwrap()??.as_bstr().unwrap(); + assert_eq!(line, "808e50d724f604f69ab93c6da2919c014667bedb refs/heads/master\n"); + let line = r.read_data_line().await; + assert!(line.is_none(), "flush means `None`"); + let line = r.read_data_line().await; + assert!(line.is_none(), "…which can't be overcome unless the reader is reset"); + assert_eq!( + r.stopped_at(), + Some(PacketLineRef::Flush), + "it knows what stopped the reader" + ); + + drop(r); + rd.reset(); + + let mut r = rd.as_read(); + let line = r.read_data_line().await.unwrap()??.as_bstr().unwrap(); + assert_eq!(line.as_bstr(), "NAK\n"); + + drop(r); + + let mut r = rd.as_read_with_sidebands(|_, _| ()); + let line = r.read_data_line().await.unwrap()??.as_bstr().unwrap(); + assert_eq!( + line.as_bstr(), + "\x02Enumerating objects: 3, done.\n", + "sidebands are ignored entirely here" + ); + for _ in 0..6 { + let _discard_more_progress = r.read_data_line().await.unwrap()??.as_bstr().unwrap(); + } + let line = r.read_data_line().await; + assert!(line.is_none(), "and we have reached the end"); + + Ok(()) +} + #[maybe_async::test(feature = "blocking-io", async(feature = "async-io", async_std::test))] async fn peek_past_an_actual_eof_is_an_error() -> crate::Result { let input = b"0009ERR e"; diff --git a/git-protocol/Cargo.toml b/git-protocol/Cargo.toml index 295ead89660..445da74e67f 100644 --- a/git-protocol/Cargo.toml +++ b/git-protocol/Cargo.toml @@ -46,7 +46,7 @@ git-credentials = { version = "^0.7.0", path = "../git-credentials" } thiserror = "1.0.32" serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"]} -bstr = { version = "1.0.1", default-features = false, features = ["std"] } +bstr = { version = "1.0.1", default-features = false, features = ["std", "unicode"] } nom = { version = "7", default-features = false, features = ["std"]} btoi = "0.4.2" diff --git a/git-protocol/src/command/tests.rs b/git-protocol/src/command/tests.rs index b921fda2eac..68a93afb7e9 100644 --- a/git-protocol/src/command/tests.rs +++ b/git-protocol/src/command/tests.rs @@ -53,7 +53,7 @@ mod v2 { use git_transport::client::Capabilities; fn capabilities(command: &str, input: &str) -> Capabilities { - Capabilities::from_lines(Some(Ok("version 2".into())), format!("{}={}", command, input)) + Capabilities::from_lines(format!("version 2\n{}={}", command, input).into()) .expect("valid input for V2 capabilities") } diff --git a/git-protocol/src/fetch/tests.rs b/git-protocol/src/fetch/tests.rs new file mode 100644 index 00000000000..bc142bbfdd8 --- /dev/null +++ b/git-protocol/src/fetch/tests.rs @@ -0,0 +1,344 @@ +#[cfg(any(feature = "async-client", feature = "blocking-client"))] +mod arguments { + use bstr::ByteSlice; + use git_transport::Protocol; + + use crate::fetch; + + fn arguments_v1(features: impl IntoIterator) -> fetch::Arguments { + fetch::Arguments::new(Protocol::V1, features.into_iter().map(|n| (n, None)).collect()) + } + + fn arguments_v2(features: impl IntoIterator) -> fetch::Arguments { + fetch::Arguments::new(Protocol::V2, features.into_iter().map(|n| (n, None)).collect()) + } + + struct Transport { + inner: T, + stateful: bool, + } + + #[cfg(feature = "blocking-client")] + mod impls { + use std::borrow::Cow; + + use bstr::BStr; + use git_transport::{ + client, + client::{Error, MessageKind, RequestWriter, SetServiceResponse, WriteMode}, + Protocol, Service, + }; + + use crate::fetch::tests::arguments::Transport; + + impl client::TransportWithoutIO for Transport { + fn set_identity(&mut self, identity: client::Account) -> Result<(), Error> { + self.inner.set_identity(identity) + } + + fn request( + &mut self, + write_mode: WriteMode, + on_into_read: MessageKind, + ) -> Result, Error> { + self.inner.request(write_mode, on_into_read) + } + + fn to_url(&self) -> Cow<'_, BStr> { + self.inner.to_url() + } + + fn supported_protocol_versions(&self) -> &[Protocol] { + self.inner.supported_protocol_versions() + } + + fn connection_persists_across_multiple_requests(&self) -> bool { + self.stateful + } + + fn configure( + &mut self, + config: &dyn std::any::Any, + ) -> Result<(), Box> { + self.inner.configure(config) + } + } + + impl client::Transport for Transport { + fn handshake<'a>( + &mut self, + service: Service, + extra_parameters: &'a [(&'a str, Option<&'a str>)], + ) -> Result, Error> { + self.inner.handshake(service, extra_parameters) + } + } + } + + #[cfg(feature = "async-client")] + mod impls { + use std::borrow::Cow; + + use async_trait::async_trait; + use bstr::BStr; + use git_transport::{ + client, + client::{Error, MessageKind, RequestWriter, SetServiceResponse, WriteMode}, + Protocol, Service, + }; + + use crate::fetch::tests::arguments::Transport; + impl client::TransportWithoutIO for Transport { + fn set_identity(&mut self, identity: client::Account) -> Result<(), Error> { + self.inner.set_identity(identity) + } + + fn request( + &mut self, + write_mode: WriteMode, + on_into_read: MessageKind, + ) -> Result, Error> { + self.inner.request(write_mode, on_into_read) + } + + fn to_url(&self) -> Cow<'_, BStr> { + self.inner.to_url() + } + + fn supported_protocol_versions(&self) -> &[Protocol] { + self.inner.supported_protocol_versions() + } + + fn connection_persists_across_multiple_requests(&self) -> bool { + self.stateful + } + + fn configure( + &mut self, + config: &dyn std::any::Any, + ) -> Result<(), Box> { + self.inner.configure(config) + } + } + + #[async_trait(?Send)] + impl client::Transport for Transport { + async fn handshake<'a>( + &mut self, + service: Service, + extra_parameters: &'a [(&'a str, Option<&'a str>)], + ) -> Result, Error> { + self.inner.handshake(service, extra_parameters).await + } + } + } + + fn transport( + out: &mut Vec, + stateful: bool, + ) -> Transport>> { + Transport { + inner: git_transport::client::git::Connection::new( + &[], + out, + Protocol::V1, // does not matter + b"does/not/matter".as_bstr().to_owned(), + None::<(&str, _)>, + git_transport::client::git::ConnectMode::Process, // avoid header to be sent + ), + stateful, + } + } + + fn id(hex: &str) -> git_hash::ObjectId { + git_hash::ObjectId::from_hex(hex.as_bytes()).expect("expect valid hex id") + } + + mod v1 { + use bstr::ByteSlice; + + use crate::fetch::tests::arguments::{arguments_v1, id, transport}; + + #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] + async fn haves_and_wants_for_clone() { + let mut out = Vec::new(); + let mut t = transport(&mut out, true); + let mut arguments = arguments_v1(["feature-a", "feature-b"].iter().cloned()); + + arguments.want(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c907")); + arguments.want(id("ff333369de1221f9bfbbe03a3a13e9a09bc1ffff")); + arguments.send(&mut t, true).await.expect("sending to buffer to work"); + assert_eq!( + out.as_bstr(), + b"0046want 7b333369de1221f9bfbbe03a3a13e9a09bc1c907 feature-a feature-b +0032want ff333369de1221f9bfbbe03a3a13e9a09bc1ffff +00000009done +" + .as_bstr() + ); + } + + #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] + async fn haves_and_wants_for_fetch_stateless() { + let mut out = Vec::new(); + let mut t = transport(&mut out, false); + let mut arguments = arguments_v1(["feature-a", "shallow", "deepen-since", "deepen-not"].iter().copied()); + + arguments.deepen(1); + arguments.shallow(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c9ff")); + arguments.want(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c907")); + arguments.deepen_since(12345); + arguments.deepen_not("refs/heads/main".into()); + arguments.have(id("0000000000000000000000000000000000000000")); + arguments.send(&mut t, false).await.expect("sending to buffer to work"); + + arguments.have(id("1111111111111111111111111111111111111111")); + arguments.send(&mut t, true).await.expect("sending to buffer to work"); + assert_eq!( + out.as_bstr(), + b"005cwant 7b333369de1221f9bfbbe03a3a13e9a09bc1c907 feature-a shallow deepen-since deepen-not +0035shallow 7b333369de1221f9bfbbe03a3a13e9a09bc1c9ff +000ddeepen 1 +0017deepen-since 12345 +001fdeepen-not refs/heads/main +00000032have 0000000000000000000000000000000000000000 +0000005cwant 7b333369de1221f9bfbbe03a3a13e9a09bc1c907 feature-a shallow deepen-since deepen-not +0035shallow 7b333369de1221f9bfbbe03a3a13e9a09bc1c9ff +000ddeepen 1 +0017deepen-since 12345 +001fdeepen-not refs/heads/main +00000032have 1111111111111111111111111111111111111111 +0009done +" + .as_bstr() + ); + } + + #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] + async fn haves_and_wants_for_fetch_stateful() { + let mut out = Vec::new(); + let mut t = transport(&mut out, true); + let mut arguments = arguments_v1(["feature-a", "shallow"].iter().copied()); + + arguments.deepen(1); + arguments.want(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c907")); + arguments.have(id("0000000000000000000000000000000000000000")); + arguments.send(&mut t, false).await.expect("sending to buffer to work"); + + arguments.have(id("1111111111111111111111111111111111111111")); + arguments.send(&mut t, true).await.expect("sending to buffer to work"); + assert_eq!( + out.as_bstr(), + b"0044want 7b333369de1221f9bfbbe03a3a13e9a09bc1c907 feature-a shallow +000ddeepen 1 +00000032have 0000000000000000000000000000000000000000 +00000032have 1111111111111111111111111111111111111111 +0009done +" + .as_bstr() + ); + } + } + + mod v2 { + use bstr::ByteSlice; + + use crate::fetch::tests::arguments::{arguments_v2, id, transport}; + + #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] + async fn haves_and_wants_for_clone_stateful() { + let mut out = Vec::new(); + let mut t = transport(&mut out, true); + let mut arguments = arguments_v2(["feature-a", "shallow"].iter().copied()); + + arguments.deepen(1); + arguments.deepen_relative(); + arguments.want(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c907")); + arguments.want(id("ff333369de1221f9bfbbe03a3a13e9a09bc1ffff")); + arguments.send(&mut t, true).await.expect("sending to buffer to work"); + assert_eq!( + out.as_bstr(), + b"0012command=fetch +0001000ethin-pack +0010include-tag +000eofs-delta +000ddeepen 1 +0014deepen-relative +0032want 7b333369de1221f9bfbbe03a3a13e9a09bc1c907 +0032want ff333369de1221f9bfbbe03a3a13e9a09bc1ffff +0009done +0000" + .as_bstr(), + "we filter features/capabilities without value as these apparently shouldn't be listed (remote dies otherwise)" + ); + } + + #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] + async fn haves_and_wants_for_fetch_stateless_and_stateful() { + for is_stateful in &[false, true] { + let mut out = Vec::new(); + let mut t = transport(&mut out, *is_stateful); + let mut arguments = arguments_v2(Some("shallow")); + + arguments.deepen(1); + arguments.deepen_since(12345); + arguments.shallow(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c9ff")); + arguments.want(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c907")); + arguments.deepen_not("refs/heads/main".into()); + arguments.have(id("0000000000000000000000000000000000000000")); + arguments.send(&mut t, false).await.expect("sending to buffer to work"); + + arguments.have(id("1111111111111111111111111111111111111111")); + arguments.send(&mut t, true).await.expect("sending to buffer to work"); + assert_eq!( + out.as_bstr(), + b"0012command=fetch +0001000ethin-pack +0010include-tag +000eofs-delta +000ddeepen 1 +0017deepen-since 12345 +0035shallow 7b333369de1221f9bfbbe03a3a13e9a09bc1c9ff +0032want 7b333369de1221f9bfbbe03a3a13e9a09bc1c907 +001fdeepen-not refs/heads/main +0032have 0000000000000000000000000000000000000000 +00000012command=fetch +0001000ethin-pack +0010include-tag +000eofs-delta +000ddeepen 1 +0017deepen-since 12345 +0035shallow 7b333369de1221f9bfbbe03a3a13e9a09bc1c9ff +0032want 7b333369de1221f9bfbbe03a3a13e9a09bc1c907 +001fdeepen-not refs/heads/main +0032have 1111111111111111111111111111111111111111 +0009done +0000" + .as_bstr(), + "V2 is stateless by default, so it repeats all but 'haves' in each request" + ); + } + } + + #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] + async fn ref_in_want() { + let mut out = Vec::new(); + let mut t = transport(&mut out, false); + let mut arguments = arguments_v2(["ref-in-want"].iter().copied()); + + arguments.want_ref(b"refs/heads/main".as_bstr()); + arguments.send(&mut t, true).await.expect("sending to buffer to work"); + assert_eq!( + out.as_bstr(), + b"0012command=fetch +0001000ethin-pack +0010include-tag +000eofs-delta +001dwant-ref refs/heads/main +0009done +0000" + .as_bstr() + ) + } + } +} diff --git a/git-protocol/src/fetch/tests/arguments.rs b/git-protocol/src/fetch/tests/arguments.rs deleted file mode 100644 index 334e571ab15..00000000000 --- a/git-protocol/src/fetch/tests/arguments.rs +++ /dev/null @@ -1,333 +0,0 @@ -use bstr::ByteSlice; -use git_transport::Protocol; - -use crate::fetch; - -fn arguments_v1(features: impl IntoIterator) -> fetch::Arguments { - fetch::Arguments::new(Protocol::V1, features.into_iter().map(|n| (n, None)).collect()) -} - -fn arguments_v2(features: impl IntoIterator) -> fetch::Arguments { - fetch::Arguments::new(Protocol::V2, features.into_iter().map(|n| (n, None)).collect()) -} - -struct Transport { - inner: T, - stateful: bool, -} - -#[cfg(feature = "blocking-client")] -mod impls { - use std::borrow::Cow; - - use bstr::BStr; - use git_transport::{ - client, - client::{Error, MessageKind, RequestWriter, SetServiceResponse, WriteMode}, - Protocol, Service, - }; - - use crate::fetch::tests::arguments::Transport; - - impl client::TransportWithoutIO for Transport { - fn set_identity(&mut self, identity: client::Account) -> Result<(), Error> { - self.inner.set_identity(identity) - } - - fn request(&mut self, write_mode: WriteMode, on_into_read: MessageKind) -> Result, Error> { - self.inner.request(write_mode, on_into_read) - } - - fn to_url(&self) -> Cow<'_, BStr> { - self.inner.to_url() - } - - fn supported_protocol_versions(&self) -> &[Protocol] { - self.inner.supported_protocol_versions() - } - - fn connection_persists_across_multiple_requests(&self) -> bool { - self.stateful - } - - fn configure( - &mut self, - config: &dyn std::any::Any, - ) -> Result<(), Box> { - self.inner.configure(config) - } - } - - impl client::Transport for Transport { - fn handshake<'a>( - &mut self, - service: Service, - extra_parameters: &'a [(&'a str, Option<&'a str>)], - ) -> Result, Error> { - self.inner.handshake(service, extra_parameters) - } - } -} - -#[cfg(feature = "async-client")] -mod impls { - use std::borrow::Cow; - - use async_trait::async_trait; - use bstr::BStr; - use git_transport::{ - client, - client::{Error, MessageKind, RequestWriter, SetServiceResponse, WriteMode}, - Protocol, Service, - }; - - use crate::fetch::tests::arguments::Transport; - impl client::TransportWithoutIO for Transport { - fn set_identity(&mut self, identity: client::Account) -> Result<(), Error> { - self.inner.set_identity(identity) - } - - fn request(&mut self, write_mode: WriteMode, on_into_read: MessageKind) -> Result, Error> { - self.inner.request(write_mode, on_into_read) - } - - fn to_url(&self) -> Cow<'_, BStr> { - self.inner.to_url() - } - - fn supported_protocol_versions(&self) -> &[Protocol] { - self.inner.supported_protocol_versions() - } - - fn connection_persists_across_multiple_requests(&self) -> bool { - self.stateful - } - - fn configure( - &mut self, - config: &dyn std::any::Any, - ) -> Result<(), Box> { - self.inner.configure(config) - } - } - - #[async_trait(?Send)] - impl client::Transport for Transport { - async fn handshake<'a>( - &mut self, - service: Service, - extra_parameters: &'a [(&'a str, Option<&'a str>)], - ) -> Result, Error> { - self.inner.handshake(service, extra_parameters).await - } - } -} - -fn transport( - out: &mut Vec, - stateful: bool, -) -> Transport>> { - Transport { - inner: git_transport::client::git::Connection::new( - &[], - out, - Protocol::V1, // does not matter - b"does/not/matter".as_bstr().to_owned(), - None::<(&str, _)>, - git_transport::client::git::ConnectMode::Process, // avoid header to be sent - ), - stateful, - } -} - -fn id(hex: &str) -> git_hash::ObjectId { - git_hash::ObjectId::from_hex(hex.as_bytes()).expect("expect valid hex id") -} - -mod v1 { - use bstr::ByteSlice; - - use crate::fetch::tests::arguments::{arguments_v1, id, transport}; - - #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] - async fn haves_and_wants_for_clone() { - let mut out = Vec::new(); - let mut t = transport(&mut out, true); - let mut arguments = arguments_v1(["feature-a", "feature-b"].iter().cloned()); - - arguments.want(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c907")); - arguments.want(id("ff333369de1221f9bfbbe03a3a13e9a09bc1ffff")); - arguments.send(&mut t, true).await.expect("sending to buffer to work"); - assert_eq!( - out.as_bstr(), - b"0046want 7b333369de1221f9bfbbe03a3a13e9a09bc1c907 feature-a feature-b -0032want ff333369de1221f9bfbbe03a3a13e9a09bc1ffff -00000009done -" - .as_bstr() - ); - } - - #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] - async fn haves_and_wants_for_fetch_stateless() { - let mut out = Vec::new(); - let mut t = transport(&mut out, false); - let mut arguments = arguments_v1(["feature-a", "shallow", "deepen-since", "deepen-not"].iter().copied()); - - arguments.deepen(1); - arguments.shallow(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c9ff")); - arguments.want(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c907")); - arguments.deepen_since(12345); - arguments.deepen_not("refs/heads/main".into()); - arguments.have(id("0000000000000000000000000000000000000000")); - arguments.send(&mut t, false).await.expect("sending to buffer to work"); - - arguments.have(id("1111111111111111111111111111111111111111")); - arguments.send(&mut t, true).await.expect("sending to buffer to work"); - assert_eq!( - out.as_bstr(), - b"005cwant 7b333369de1221f9bfbbe03a3a13e9a09bc1c907 feature-a shallow deepen-since deepen-not -0035shallow 7b333369de1221f9bfbbe03a3a13e9a09bc1c9ff -000ddeepen 1 -0017deepen-since 12345 -001fdeepen-not refs/heads/main -00000032have 0000000000000000000000000000000000000000 -0000005cwant 7b333369de1221f9bfbbe03a3a13e9a09bc1c907 feature-a shallow deepen-since deepen-not -0035shallow 7b333369de1221f9bfbbe03a3a13e9a09bc1c9ff -000ddeepen 1 -0017deepen-since 12345 -001fdeepen-not refs/heads/main -00000032have 1111111111111111111111111111111111111111 -0009done -" - .as_bstr() - ); - } - - #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] - async fn haves_and_wants_for_fetch_stateful() { - let mut out = Vec::new(); - let mut t = transport(&mut out, true); - let mut arguments = arguments_v1(["feature-a", "shallow"].iter().copied()); - - arguments.deepen(1); - arguments.want(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c907")); - arguments.have(id("0000000000000000000000000000000000000000")); - arguments.send(&mut t, false).await.expect("sending to buffer to work"); - - arguments.have(id("1111111111111111111111111111111111111111")); - arguments.send(&mut t, true).await.expect("sending to buffer to work"); - assert_eq!( - out.as_bstr(), - b"0044want 7b333369de1221f9bfbbe03a3a13e9a09bc1c907 feature-a shallow -000ddeepen 1 -00000032have 0000000000000000000000000000000000000000 -00000032have 1111111111111111111111111111111111111111 -0009done -" - .as_bstr() - ); - } -} - -mod v2 { - use bstr::ByteSlice; - - use crate::fetch::tests::arguments::{arguments_v2, id, transport}; - - #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] - async fn haves_and_wants_for_clone_stateful() { - let mut out = Vec::new(); - let mut t = transport(&mut out, true); - let mut arguments = arguments_v2(["feature-a", "shallow"].iter().copied()); - - arguments.deepen(1); - arguments.deepen_relative(); - arguments.want(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c907")); - arguments.want(id("ff333369de1221f9bfbbe03a3a13e9a09bc1ffff")); - arguments.send(&mut t, true).await.expect("sending to buffer to work"); - assert_eq!( - out.as_bstr(), - b"0012command=fetch -0001000ethin-pack -0010include-tag -000eofs-delta -000ddeepen 1 -0014deepen-relative -0032want 7b333369de1221f9bfbbe03a3a13e9a09bc1c907 -0032want ff333369de1221f9bfbbe03a3a13e9a09bc1ffff -0009done -0000" - .as_bstr(), - "we filter features/capabilities without value as these apparently shouldn't be listed (remote dies otherwise)" - ); - } - - #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] - async fn haves_and_wants_for_fetch_stateless_and_stateful() { - for is_stateful in &[false, true] { - let mut out = Vec::new(); - let mut t = transport(&mut out, *is_stateful); - let mut arguments = arguments_v2(Some("shallow")); - - arguments.deepen(1); - arguments.deepen_since(12345); - arguments.shallow(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c9ff")); - arguments.want(id("7b333369de1221f9bfbbe03a3a13e9a09bc1c907")); - arguments.deepen_not("refs/heads/main".into()); - arguments.have(id("0000000000000000000000000000000000000000")); - arguments.send(&mut t, false).await.expect("sending to buffer to work"); - - arguments.have(id("1111111111111111111111111111111111111111")); - arguments.send(&mut t, true).await.expect("sending to buffer to work"); - assert_eq!( - out.as_bstr(), - b"0012command=fetch -0001000ethin-pack -0010include-tag -000eofs-delta -000ddeepen 1 -0017deepen-since 12345 -0035shallow 7b333369de1221f9bfbbe03a3a13e9a09bc1c9ff -0032want 7b333369de1221f9bfbbe03a3a13e9a09bc1c907 -001fdeepen-not refs/heads/main -0032have 0000000000000000000000000000000000000000 -00000012command=fetch -0001000ethin-pack -0010include-tag -000eofs-delta -000ddeepen 1 -0017deepen-since 12345 -0035shallow 7b333369de1221f9bfbbe03a3a13e9a09bc1c9ff -0032want 7b333369de1221f9bfbbe03a3a13e9a09bc1c907 -001fdeepen-not refs/heads/main -0032have 1111111111111111111111111111111111111111 -0009done -0000" - .as_bstr(), - "V2 is stateless by default, so it repeats all but 'haves' in each request" - ); - } - } - - #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] - async fn ref_in_want() { - let mut out = Vec::new(); - let mut t = transport(&mut out, false); - let mut arguments = arguments_v2(["ref-in-want"].iter().copied()); - - arguments.want_ref(b"refs/heads/main".as_bstr()); - arguments.send(&mut t, true).await.expect("sending to buffer to work"); - assert_eq!( - out.as_bstr(), - b"0012command=fetch -0001000ethin-pack -0010include-tag -000eofs-delta -001dwant-ref refs/heads/main -0009done -0000" - .as_bstr() - ) - } -} diff --git a/git-protocol/src/fetch/tests/mod.rs b/git-protocol/src/fetch/tests/mod.rs deleted file mode 100644 index 465ac0dc320..00000000000 --- a/git-protocol/src/fetch/tests/mod.rs +++ /dev/null @@ -1,4 +0,0 @@ -#[cfg(any(feature = "async-client", feature = "blocking-client"))] -mod arguments; -#[cfg(any(feature = "blocking-client", feature = "async-client"))] -mod refs; diff --git a/git-protocol/src/handshake/refs/async_io.rs b/git-protocol/src/handshake/refs/async_io.rs index 474b893ef49..f98e1011cf6 100644 --- a/git-protocol/src/handshake/refs/async_io.rs +++ b/git-protocol/src/handshake/refs/async_io.rs @@ -1,19 +1,17 @@ use futures_io::AsyncBufRead; -use futures_lite::AsyncBufReadExt; +use futures_lite::AsyncReadExt; use crate::handshake::{refs, refs::parse::Error, Ref}; +use bstr::ByteSlice; /// Parse refs from the given input line by line. Protocol V2 is required for this to succeed. pub async fn from_v2_refs(in_refs: &mut (dyn AsyncBufRead + Unpin)) -> Result, Error> { let mut out_refs = Vec::new(); - let mut line = String::new(); - loop { - line.clear(); - let bytes_read = in_refs.read_line(&mut line).await?; - if bytes_read == 0 { - break; - } - out_refs.push(refs::shared::parse_v2(&line)?); + let mut buf = Vec::new(); + + in_refs.read_to_end(&mut buf).await?; + for line in ByteSlice::lines(buf.as_slice()) { + out_refs.push(refs::shared::parse_v2(line.into())?); } Ok(out_refs) } @@ -32,14 +30,11 @@ pub async fn from_v1_refs_received_as_part_of_handshake_and_capabilities<'a>( ) -> Result, refs::parse::Error> { let mut out_refs = refs::shared::from_capabilities(capabilities)?; let number_of_possible_symbolic_refs_for_lookup = out_refs.len(); - let mut line = String::new(); - loop { - line.clear(); - let bytes_read = in_refs.read_line(&mut line).await?; - if bytes_read == 0 { - break; - } - refs::shared::parse_v1(number_of_possible_symbolic_refs_for_lookup, &mut out_refs, &line)?; + + let mut buf = Vec::new(); + in_refs.read_to_end(&mut buf).await?; + for line in buf.as_slice().lines() { + refs::shared::parse_v1(number_of_possible_symbolic_refs_for_lookup, &mut out_refs, line.into())?; } Ok(out_refs.into_iter().map(Into::into).collect()) } diff --git a/git-protocol/src/handshake/refs/blocking_io.rs b/git-protocol/src/handshake/refs/blocking_io.rs index 5d55663bf85..7001bf3760b 100644 --- a/git-protocol/src/handshake/refs/blocking_io.rs +++ b/git-protocol/src/handshake/refs/blocking_io.rs @@ -1,18 +1,10 @@ -use std::io; - use crate::handshake::{refs, refs::parse::Error, Ref}; /// Parse refs from the given input line by line. Protocol V2 is required for this to succeed. -pub fn from_v2_refs(in_refs: &mut dyn io::BufRead) -> Result, Error> { +pub fn from_v2_refs(in_refs: &mut dyn git_transport::client::ReadlineBufRead) -> Result, Error> { let mut out_refs = Vec::new(); - let mut line = String::new(); - loop { - line.clear(); - let bytes_read = in_refs.read_line(&mut line)?; - if bytes_read == 0 { - break; - } - out_refs.push(refs::shared::parse_v2(&line)?); + while let Some(line) = in_refs.readline().transpose()?.transpose()?.and_then(|l| l.as_bstr()) { + out_refs.push(refs::shared::parse_v2(line)?); } Ok(out_refs) } @@ -26,19 +18,14 @@ pub fn from_v2_refs(in_refs: &mut dyn io::BufRead) -> Result, Error> { /// Symbolic refs are shoe-horned into server capabilities whereas refs (without symbolic ones) are sent automatically as /// part of the handshake. Both symbolic and peeled refs need to be combined to fit into the [`Ref`] type provided here. pub fn from_v1_refs_received_as_part_of_handshake_and_capabilities<'a>( - in_refs: &mut dyn io::BufRead, + in_refs: &mut dyn git_transport::client::ReadlineBufRead, capabilities: impl Iterator>, ) -> Result, Error> { let mut out_refs = refs::shared::from_capabilities(capabilities)?; let number_of_possible_symbolic_refs_for_lookup = out_refs.len(); - let mut line = String::new(); - loop { - line.clear(); - let bytes_read = in_refs.read_line(&mut line)?; - if bytes_read == 0 { - break; - } - refs::shared::parse_v1(number_of_possible_symbolic_refs_for_lookup, &mut out_refs, &line)?; + + while let Some(line) = in_refs.readline().transpose()?.transpose()?.and_then(|l| l.as_bstr()) { + refs::shared::parse_v1(number_of_possible_symbolic_refs_for_lookup, &mut out_refs, line)?; } Ok(out_refs.into_iter().map(Into::into).collect()) } diff --git a/git-protocol/src/handshake/refs/mod.rs b/git-protocol/src/handshake/refs/mod.rs index 5eeabf6d3d5..209fc90d7d8 100644 --- a/git-protocol/src/handshake/refs/mod.rs +++ b/git-protocol/src/handshake/refs/mod.rs @@ -13,17 +13,19 @@ pub mod parse { #[error(transparent)] Io(#[from] std::io::Error), #[error(transparent)] + DecodePacketline(#[from] git_transport::packetline::decode::Error), + #[error(transparent)] Id(#[from] git_hash::decode::Error), #[error("{symref:?} could not be parsed. A symref is expected to look like :.")] MalformedSymref { symref: BString }, #[error("{0:?} could not be parsed. A V1 ref line should be ' '.")] - MalformedV1RefLine(String), + MalformedV1RefLine(BString), #[error( "{0:?} could not be parsed. A V2 ref line should be ' [ (peeled|symref-target):'." )] - MalformedV2RefLine(String), + MalformedV2RefLine(BString), #[error("The ref attribute {attribute:?} is unknown. Found in line {line:?}")] - UnkownAttribute { attribute: String, line: String }, + UnkownAttribute { attribute: BString, line: BString }, #[error("{message}")] InvariantViolation { message: &'static str }, } @@ -65,3 +67,6 @@ pub use async_io::{from_v1_refs_received_as_part_of_handshake_and_capabilities, mod blocking_io; #[cfg(feature = "blocking-client")] pub use blocking_io::{from_v1_refs_received_as_part_of_handshake_and_capabilities, from_v2_refs}; + +#[cfg(test)] +mod tests; diff --git a/git-protocol/src/handshake/refs/shared.rs b/git-protocol/src/handshake/refs/shared.rs index 4ba356465b4..5e0d6c75aff 100644 --- a/git-protocol/src/handshake/refs/shared.rs +++ b/git-protocol/src/handshake/refs/shared.rs @@ -1,4 +1,4 @@ -use bstr::{BString, ByteSlice}; +use bstr::{BStr, BString, ByteSlice}; use crate::handshake::{refs::parse::Error, Ref}; @@ -70,7 +70,7 @@ impl InternalRef { _ => None, } } - fn lookup_symbol_has_path(&self, predicate_path: &str) -> bool { + fn lookup_symbol_has_path(&self, predicate_path: &BStr) -> bool { matches!(self, InternalRef::SymbolicForLookup { path, .. } if path == predicate_path) } } @@ -109,19 +109,19 @@ pub(crate) fn from_capabilities<'a>( pub(in crate::handshake::refs) fn parse_v1( num_initial_out_refs: usize, out_refs: &mut Vec, - line: &str, + line: &BStr, ) -> Result<(), Error> { let trimmed = line.trim_end(); let (hex_hash, path) = trimmed.split_at( trimmed - .find(' ') - .ok_or_else(|| Error::MalformedV1RefLine(trimmed.to_owned()))?, + .find(b" ") + .ok_or_else(|| Error::MalformedV1RefLine(trimmed.to_owned().into()))?, ); let path = &path[1..]; if path.is_empty() { - return Err(Error::MalformedV1RefLine(trimmed.to_owned())); + return Err(Error::MalformedV1RefLine(trimmed.to_owned().into())); } - match path.strip_suffix("^{}") { + match path.strip_suffix(b"^{}") { Some(stripped) => { let (previous_path, tag) = out_refs @@ -146,7 +146,7 @@ pub(in crate::handshake::refs) fn parse_v1( match out_refs .iter() .take(num_initial_out_refs) - .position(|r| r.lookup_symbol_has_path(path)) + .position(|r| r.lookup_symbol_has_path(path.into())) { Some(position) => match out_refs.swap_remove(position) { InternalRef::SymbolicForLookup { path: _, target } => out_refs.push(InternalRef::Symbolic { @@ -166,36 +166,36 @@ pub(in crate::handshake::refs) fn parse_v1( Ok(()) } -pub(in crate::handshake::refs) fn parse_v2(line: &str) -> Result { +pub(in crate::handshake::refs) fn parse_v2(line: &BStr) -> Result { let trimmed = line.trim_end(); - let mut tokens = trimmed.splitn(3, ' '); + let mut tokens = trimmed.splitn(3, |b| *b == b' '); match (tokens.next(), tokens.next()) { (Some(hex_hash), Some(path)) => { - let id = if hex_hash == "unborn" { + let id = if hex_hash == b"unborn" { None } else { Some(git_hash::ObjectId::from_hex(hex_hash.as_bytes())?) }; if path.is_empty() { - return Err(Error::MalformedV2RefLine(trimmed.to_owned())); + return Err(Error::MalformedV2RefLine(trimmed.to_owned().into())); } Ok(if let Some(attribute) = tokens.next() { - let mut tokens = attribute.splitn(2, ':'); + let mut tokens = attribute.splitn(2, |b| *b == b':'); match (tokens.next(), tokens.next()) { (Some(attribute), Some(value)) => { if value.is_empty() { - return Err(Error::MalformedV2RefLine(trimmed.to_owned())); + return Err(Error::MalformedV2RefLine(trimmed.to_owned().into())); } match attribute { - "peeled" => Ref::Peeled { + b"peeled" => Ref::Peeled { full_ref_name: path.into(), object: git_hash::ObjectId::from_hex(value.as_bytes())?, tag: id.ok_or(Error::InvariantViolation { message: "got 'unborn' as tag target", })?, }, - "symref-target" => match value { - "(null)" => Ref::Direct { + b"symref-target" => match value { + b"(null)" => Ref::Direct { full_ref_name: path.into(), object: id.ok_or(Error::InvariantViolation { message: "got 'unborn' while (null) was a symref target", @@ -215,13 +215,13 @@ pub(in crate::handshake::refs) fn parse_v2(line: &str) -> Result { }, _ => { return Err(Error::UnkownAttribute { - attribute: attribute.to_owned(), - line: trimmed.to_owned(), + attribute: attribute.to_owned().into(), + line: trimmed.to_owned().into(), }) } } } - _ => return Err(Error::MalformedV2RefLine(trimmed.to_owned())), + _ => return Err(Error::MalformedV2RefLine(trimmed.to_owned().into())), } } else { Ref::Direct { @@ -232,6 +232,6 @@ pub(in crate::handshake::refs) fn parse_v2(line: &str) -> Result { } }) } - _ => Err(Error::MalformedV2RefLine(trimmed.to_owned())), + _ => Err(Error::MalformedV2RefLine(trimmed.to_owned().into())), } } diff --git a/git-protocol/src/fetch/tests/refs.rs b/git-protocol/src/handshake/refs/tests.rs similarity index 80% rename from git-protocol/src/fetch/tests/refs.rs rename to git-protocol/src/handshake/refs/tests.rs index 35424af270e..fc76cb85a91 100644 --- a/git-protocol/src/fetch/tests/refs.rs +++ b/git-protocol/src/handshake/refs/tests.rs @@ -15,6 +15,8 @@ unborn refs/heads/symbolic symref-target:refs/heads/target " .as_bytes(); + #[cfg(feature = "blocking-client")] + let input = &mut Fixture(input); let out = refs::from_v2_refs(input).await.expect("no failure on valid input"); assert_eq!( @@ -56,6 +58,7 @@ unborn refs/heads/symbolic symref-target:refs/heads/target #[maybe_async::test(feature = "blocking-client", async(feature = "async-client", async_std::test))] async fn extract_references_from_v1_refs() { + #[cfg_attr(feature = "blocking-client", allow(unused_mut))] let input = &mut "73a6868963993a3328e7d8fe94e5a6ac5078a944 HEAD 21c9b7500cb144b3169a6537961ec2b9e865be81 MISSING_NAMESPACE_TARGET 73a6868963993a3328e7d8fe94e5a6ac5078a944 refs/heads/main @@ -63,6 +66,8 @@ async fn extract_references_from_v1_refs() { dce0ea858eef7ff61ad345cc5cdac62203fb3c10 refs/tags/git-commitgraph-v0.0.0 21c9b7500cb144b3169a6537961ec2b9e865be81 refs/tags/git-commitgraph-v0.0.0^{}" .as_bytes(); + #[cfg(feature = "blocking-client")] + let input = &mut Fixture(input); let out = refs::from_v1_refs_received_as_part_of_handshake_and_capabilities( input, Capabilities::from_bytes(b"\0symref=HEAD:refs/heads/main symref=MISSING_NAMESPACE_TARGET:(null)") @@ -106,7 +111,7 @@ fn extract_symbolic_references_from_capabilities() -> Result<(), client::Error> let caps = client::Capabilities::from_bytes( b"\0unrelated symref=HEAD:refs/heads/main symref=ANOTHER:refs/heads/foo symref=MISSING_NAMESPACE_TARGET:(null) agent=git/2.28.0", )? - .0; + .0; let out = refs::shared::from_capabilities(caps.iter()).expect("a working example"); assert_eq!( @@ -128,3 +133,38 @@ fn extract_symbolic_references_from_capabilities() -> Result<(), client::Error> ); Ok(()) } + +#[cfg(feature = "blocking-client")] +struct Fixture<'a>(&'a [u8]); + +#[cfg(feature = "blocking-client")] +impl<'a> std::io::Read for Fixture<'a> { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + self.0.read(buf) + } +} + +#[cfg(feature = "blocking-client")] +impl<'a> std::io::BufRead for Fixture<'a> { + fn fill_buf(&mut self) -> std::io::Result<&[u8]> { + self.0.fill_buf() + } + + fn consume(&mut self, amt: usize) { + self.0.consume(amt) + } +} + +#[cfg(feature = "blocking-client")] +impl<'a> git_transport::client::ReadlineBufRead for Fixture<'a> { + fn readline( + &mut self, + ) -> Option, git_packetline::decode::Error>>> { + use bstr::{BStr, ByteSlice}; + let bytes: &BStr = self.0.into(); + let mut lines = bytes.lines(); + let res = lines.next()?; + self.0 = lines.as_bytes(); + Some(Ok(Ok(git_packetline::PacketLineRef::Data(res)))) + } +} diff --git a/git-repository/src/config/cache/init.rs b/git-repository/src/config/cache/init.rs index cd1bf05fbba..368c8075a1e 100644 --- a/git-repository/src/config/cache/init.rs +++ b/git-repository/src/config/cache/init.rs @@ -34,7 +34,7 @@ impl Cache { ssh_prefix: _, http_transport, identity, - gitoxide_prefix, + objects, }: repository::permissions::Environment, repository::permissions::Config { git_binary: use_installation, @@ -136,7 +136,7 @@ impl Cache { source: git_config::Source::Api, })?; } - apply_environment_overrides(&mut globals, *git_prefix, http_transport, identity, gitoxide_prefix)?; + apply_environment_overrides(&mut globals, *git_prefix, http_transport, identity, objects)?; globals }; @@ -253,7 +253,7 @@ fn apply_environment_overrides( git_prefix: Permission, http_transport: Permission, identity: Permission, - gitoxide_prefix: Permission, + objects: Permission, ) -> Result<(), Error> { fn var_as_bstring(var: &str, perm: Permission) -> Option { perm.check_opt(var) @@ -266,15 +266,15 @@ fn apply_environment_overrides( let mut section = env_override .new_section("http", None) .expect("statically known valid section name"); - for (var, key, permission) in [ - ("GIT_HTTP_LOW_SPEED_LIMIT", "lowSpeedLimit", git_prefix), - ("GIT_HTTP_LOW_SPEED_TIME", "lowSpeedTime", git_prefix), - ("GIT_HTTP_USER_AGENT", "userAgent", git_prefix), - ("GIT_HTTP_PROXY_AUTHMETHOD", "proxyAuthMethod", git_prefix), - ("all_proxy", "all-proxy-lower", http_transport), - ("ALL_PROXY", "all-proxy", http_transport), + for (var, key) in [ + ("GIT_HTTP_LOW_SPEED_LIMIT", "lowSpeedLimit"), + ("GIT_HTTP_LOW_SPEED_TIME", "lowSpeedTime"), + ("GIT_HTTP_USER_AGENT", "userAgent"), + ("GIT_HTTP_PROXY_AUTHMETHOD", "proxyAuthMethod"), + ("all_proxy", "all-proxy-lower"), + ("ALL_PROXY", "all-proxy"), ] { - if let Some(value) = var_as_bstring(var, permission) { + if let Some(value) = var_as_bstring(var, http_transport) { section.push_with_comment( key.try_into().expect("statically known to be valid"), Some(value.as_ref()), @@ -318,7 +318,7 @@ fn apply_environment_overrides( ("GIT_COMMITTER_NAME", "nameFallback"), ("GIT_COMMITTER_EMAIL", "emailFallback"), ] { - if let Some(value) = var_as_bstring(var, git_prefix) { + if let Some(value) = var_as_bstring(var, identity) { section.push_with_comment( key.try_into().expect("statically known to be valid"), Some(value.as_ref()), @@ -342,7 +342,7 @@ fn apply_environment_overrides( ("GIT_AUTHOR_NAME", "nameFallback"), ("GIT_AUTHOR_EMAIL", "emailFallback"), ] { - if let Some(value) = var_as_bstring(var, git_prefix) { + if let Some(value) = var_as_bstring(var, identity) { section.push_with_comment( key.try_into().expect("statically known to be valid"), Some(value.as_ref()), @@ -387,7 +387,7 @@ fn apply_environment_overrides( .expect("statically known valid section name"); for (var, key) in [("GIT_PROTOCOL_FROM_USER", "protocolFromUser")] { - if let Some(value) = var_as_bstring(var, git_prefix) { + if let Some(value) = var_as_bstring(var, http_transport) { section.push_with_comment( key.try_into().expect("statically known to be valid"), Some(value.as_ref()), @@ -429,9 +429,9 @@ fn apply_environment_overrides( .expect("statically known valid section name"); for (var, key, permission) in [ - ("GIT_NO_REPLACE_OBJECTS", "noReplace", git_prefix), - ("GIT_REPLACE_REF_BASE", "replaceRefBase", git_prefix), - ("GITOXIDE_OBJECT_CACHE_MEMORY", "cacheLimit", gitoxide_prefix), + ("GIT_NO_REPLACE_OBJECTS", "noReplace", objects), + ("GIT_REPLACE_REF_BASE", "replaceRefBase", objects), + ("GITOXIDE_OBJECT_CACHE_MEMORY", "cacheLimit", objects), ] { if let Some(value) = var_as_bstring(var, permission) { section.push_with_comment( @@ -454,7 +454,7 @@ fn apply_environment_overrides( .expect("statically known valid section name"); for (var, key) in [("GITOXIDE_PACK_CACHE_MEMORY", "deltaBaseCacheLimit")] { - if let Some(value) = var_as_bstring(var, gitoxide_prefix) { + if let Some(value) = var_as_bstring(var, objects) { section.push_with_comment( key.try_into().expect("statically known to be valid"), Some(value.as_ref()), @@ -474,15 +474,15 @@ fn apply_environment_overrides( .new_section("gitoxide", Some(Cow::Borrowed("http".into()))) .expect("statically known valid section name"); - for (var, key, permission) in [ - ("ALL_PROXY", "allProxy", http_transport), - ("all_proxy", "allProxy", http_transport), - ("NO_PROXY", "noProxy", http_transport), - ("no_proxy", "noProxy", http_transport), - ("http_proxy", "proxy", http_transport), - ("GIT_CURL_VERBOSE", "verbose", git_prefix), + for (var, key) in [ + ("ALL_PROXY", "allProxy"), + ("all_proxy", "allProxy"), + ("NO_PROXY", "noProxy"), + ("no_proxy", "noProxy"), + ("http_proxy", "proxy"), + ("GIT_CURL_VERBOSE", "verbose"), ] { - if let Some(value) = var_as_bstring(var, permission) { + if let Some(value) = var_as_bstring(var, http_transport) { section.push_with_comment( key.try_into().expect("statically known to be valid"), Some(value.as_ref()), diff --git a/git-repository/src/repository/permissions.rs b/git-repository/src/repository/permissions.rs index 45dfa046e6d..aab98c6799b 100644 --- a/git-repository/src/repository/permissions.rs +++ b/git-repository/src/repository/permissions.rs @@ -68,35 +68,38 @@ pub struct Environment { pub xdg_config_home: git_sec::Permission, /// Control the way resources pointed to by the home directory (similar to `xdg_config_home`) may be used. pub home: git_sec::Permission, - /// Control if resources pointed to by `GIT_*` prefixed environment variables can be used. - pub git_prefix: git_sec::Permission, - /// Control if resources pointed to by `SSH_*` prefixed environment variables can be used (like `SSH_ASKPASS`) - pub ssh_prefix: git_sec::Permission, /// Control if environment variables to configure the HTTP transport, like `http_proxy` may be used. /// - /// Note that http-transport related environment variables prefixed with `GIT_` are falling under the - /// `git_prefix` permission, like `GIT_HTTP_USER_AGENT`. + /// Note that http-transport related environment variables prefixed with `GIT_` may also be included here + /// if they match this category like `GIT_HTTP_USER_AGENT`. pub http_transport: git_sec::Permission, /// Control if the `EMAIL` environment variables may be read. /// - /// Note that identity related environment variables prefixed with `GIT_` are falling under the - /// `git_prefix` permission, like `GIT_AUTHOR_NAME`. + /// Note that identity related environment variables prefixed with `GIT_` may also be included here + /// if they match this category. pub identity: git_sec::Permission, - /// Decide if `gitoxide` specific variables may be read, prefixed with `GITOXIDE_`. - pub gitoxide_prefix: git_sec::Permission, + /// Control if environment variables related to the object database are handled. This includes features and performance + /// options alike. + pub objects: git_sec::Permission, + /// Control if resources pointed to by `GIT_*` prefixed environment variables can be used, **but only** if they + /// are not contained in any other category. This is a catch-all section. + pub git_prefix: git_sec::Permission, + /// Control if resources pointed to by `SSH_*` prefixed environment variables can be used (like `SSH_ASKPASS`) + pub ssh_prefix: git_sec::Permission, } impl Environment { /// Allow access to the entire environment. pub fn all() -> Self { + let allow = git_sec::Permission::Allow; Environment { - xdg_config_home: git_sec::Permission::Allow, - home: git_sec::Permission::Allow, - git_prefix: git_sec::Permission::Allow, - ssh_prefix: git_sec::Permission::Allow, - http_transport: git_sec::Permission::Allow, - identity: git_sec::Permission::Allow, - gitoxide_prefix: git_sec::Permission::Allow, + xdg_config_home: allow, + home: allow, + git_prefix: allow, + ssh_prefix: allow, + http_transport: allow, + identity: allow, + objects: allow, } } } @@ -143,7 +146,7 @@ impl Permissions { git_prefix: deny, http_transport: deny, identity: deny, - gitoxide_prefix: deny, + objects: deny, } }, } diff --git a/git-repository/tests/repository/open.rs b/git-repository/tests/repository/open.rs index 4cbd04bc333..a0fdc013027 100644 --- a/git-repository/tests/repository/open.rs +++ b/git-repository/tests/repository/open.rs @@ -123,7 +123,7 @@ mod with_overrides { opts.permissions.env.git_prefix = Permission::Allow; opts.permissions.env.http_transport = Permission::Allow; opts.permissions.env.identity = Permission::Allow; - opts.permissions.env.gitoxide_prefix = Permission::Allow; + opts.permissions.env.objects = Permission::Allow; let repo = named_subrepo_opts("make_config_repos.sh", "http-config", opts)?; let config = repo.config_snapshot(); assert_eq!( diff --git a/git-repository/tests/util/mod.rs b/git-repository/tests/util/mod.rs index f2aac4176de..3518ed5926e 100644 --- a/git-repository/tests/util/mod.rs +++ b/git-repository/tests/util/mod.rs @@ -35,6 +35,7 @@ pub fn restricted() -> open::Options { pub fn restricted_and_git() -> open::Options { let mut opts = open::Options::isolated(); opts.permissions.env.git_prefix = git_sec::Permission::Allow; + opts.permissions.env.identity = git_sec::Permission::Allow; opts } diff --git a/git-transport/Cargo.toml b/git-transport/Cargo.toml index 9b2f86e4d27..8847012fc81 100644 --- a/git-transport/Cargo.toml +++ b/git-transport/Cargo.toml @@ -59,7 +59,7 @@ git-packetline = { version = "^0.14.0", path = "../git-packetline" } git-credentials = { version = "^0.7.0", path = "../git-credentials", optional = true } serde = { version = "1.0.114", optional = true, default-features = false, features = ["std", "derive"]} -bstr = { version = "1.0.1", default-features = false, features = ["std"] } +bstr = { version = "1.0.1", default-features = false, features = ["std", "unicode"] } thiserror = "1.0.26" # for async-client diff --git a/git-transport/src/client/async_io/bufread_ext.rs b/git-transport/src/client/async_io/bufread_ext.rs index b681be8a564..12993fe776c 100644 --- a/git-transport/src/client/async_io/bufread_ext.rs +++ b/git-transport/src/client/async_io/bufread_ext.rs @@ -5,6 +5,7 @@ use std::{ use async_trait::async_trait; use futures_io::{AsyncBufRead, AsyncRead}; +use git_packetline::PacketLineRef; use crate::{ client::{Error, MessageKind}, @@ -16,11 +17,28 @@ use crate::{ /// it onto an executor. pub type HandleProgress = Box; -/// This trait exists to get a version of a `git_packetline::Provider` without type parameters. -/// For the sake of usability, it also implements [`std::io::BufRead`] making it trivial to (eventually) -/// read pack files while keeping the possibility to read individual lines with low overhead. +/// This trait exists to get a version of a `git_packetline::Provider` without type parameters, +/// but leave support for reading lines directly without forcing them through `String`. +/// +/// For the sake of usability, it also implements [`std::io::BufRead`] making it trivial to +/// read pack files while keeping open the option to read individual lines with low overhead. #[async_trait(?Send)] -pub trait ExtendedBufRead: AsyncBufRead { +pub trait ReadlineBufRead: AsyncBufRead { + /// Read a packet line into the internal buffer and return it. + /// + /// Returns `None` if the end of iteration is reached because of one of the following: + /// + /// * natural EOF + /// * ERR packet line encountered + /// * A `delimiter` packet line encountered + async fn readline( + &mut self, + ) -> Option, git_packetline::decode::Error>>>; +} + +/// Provide even more access to the underlying packet reader. +#[async_trait(?Send)] +pub trait ExtendedBufRead: ReadlineBufRead { /// Set the handler to which progress will be delivered. /// /// Note that this is only possible if packet lines are sent in side band mode. @@ -35,6 +53,13 @@ pub trait ExtendedBufRead: AsyncBufRead { fn stopped_at(&self) -> Option; } +#[async_trait(?Send)] +impl<'a, T: ReadlineBufRead + ?Sized + 'a + Unpin> ReadlineBufRead for Box { + async fn readline(&mut self) -> Option, git_packetline::decode::Error>>> { + self.deref_mut().readline().await + } +} + #[async_trait(?Send)] impl<'a, T: ExtendedBufRead + ?Sized + 'a + Unpin> ExtendedBufRead for Box { fn set_progress_handler(&mut self, handle_progress: Option) { @@ -54,6 +79,20 @@ impl<'a, T: ExtendedBufRead + ?Sized + 'a + Unpin> ExtendedBufRead for Box { } } +#[async_trait(?Send)] +impl ReadlineBufRead for git_packetline::read::WithSidebands<'_, T, for<'b> fn(bool, &'b [u8])> { + async fn readline(&mut self) -> Option, git_packetline::decode::Error>>> { + self.readline().await + } +} + +#[async_trait(?Send)] +impl<'a, T: AsyncRead + Unpin> ReadlineBufRead for git_packetline::read::WithSidebands<'a, T, HandleProgress> { + async fn readline(&mut self) -> Option, git_packetline::decode::Error>>> { + self.readline().await + } +} + #[async_trait(?Send)] impl<'a, T: AsyncRead + Unpin> ExtendedBufRead for git_packetline::read::WithSidebands<'a, T, HandleProgress> { fn set_progress_handler(&mut self, handle_progress: Option) { diff --git a/git-transport/src/client/async_io/mod.rs b/git-transport/src/client/async_io/mod.rs index 2b12f2da802..6cb1a500e18 100644 --- a/git-transport/src/client/async_io/mod.rs +++ b/git-transport/src/client/async_io/mod.rs @@ -1,5 +1,5 @@ mod bufread_ext; -pub use bufread_ext::{ExtendedBufRead, HandleProgress}; +pub use bufread_ext::{ExtendedBufRead, HandleProgress, ReadlineBufRead}; mod request; pub use request::RequestWriter; diff --git a/git-transport/src/client/async_io/traits.rs b/git-transport/src/client/async_io/traits.rs index a4e818412c1..e64ab0f4920 100644 --- a/git-transport/src/client/async_io/traits.rs +++ b/git-transport/src/client/async_io/traits.rs @@ -16,7 +16,7 @@ pub struct SetServiceResponse<'a> { /// The capabilities parsed from the server response. pub capabilities: Capabilities, /// In protocol version one, this is set to a list of refs and their peeled counterparts. - pub refs: Option>, + pub refs: Option>, } /// All methods provided here must be called in the correct order according to the [communication protocol][Protocol] diff --git a/git-transport/src/client/blocking_io/bufread_ext.rs b/git-transport/src/client/blocking_io/bufread_ext.rs index e894d51374f..1e6386cb2af 100644 --- a/git-transport/src/client/blocking_io/bufread_ext.rs +++ b/git-transport/src/client/blocking_io/bufread_ext.rs @@ -1,3 +1,4 @@ +use git_packetline::PacketLineRef; use std::{ io, ops::{Deref, DerefMut}, @@ -10,10 +11,26 @@ use crate::{ /// A function `f(is_error, text)` receiving progress or error information. pub type HandleProgress = Box; -/// This trait exists to get a version of a `git_packetline::Provider` without type parameters. +/// This trait exists to get a version of a `git_packetline::Provider` without type parameters, +/// but leave support for reading lines directly without forcing them through `String`. +/// /// For the sake of usability, it also implements [`std::io::BufRead`] making it trivial to /// read pack files while keeping open the option to read individual lines with low overhead. -pub trait ExtendedBufRead: io::BufRead { +pub trait ReadlineBufRead: io::BufRead { + /// Read a packet line into the internal buffer and return it. + /// + /// Returns `None` if the end of iteration is reached because of one of the following: + /// + /// * natural EOF + /// * ERR packet line encountered + /// * A `delimiter` packet line encountered + fn readline( + &mut self, + ) -> Option, git_packetline::decode::Error>>>; +} + +/// Provide even more access to the underlying packet reader. +pub trait ExtendedBufRead: ReadlineBufRead { /// Set the handler to which progress will be delivered. /// /// Note that this is only possible if packet lines are sent in side band mode. @@ -28,6 +45,12 @@ pub trait ExtendedBufRead: io::BufRead { fn stopped_at(&self) -> Option; } +impl<'a, T: ReadlineBufRead + ?Sized + 'a> ReadlineBufRead for Box { + fn readline(&mut self) -> Option, git_packetline::decode::Error>>> { + ReadlineBufRead::readline(self.deref_mut()) + } +} + impl<'a, T: ExtendedBufRead + ?Sized + 'a> ExtendedBufRead for Box { fn set_progress_handler(&mut self, handle_progress: Option) { self.deref_mut().set_progress_handler(handle_progress) @@ -46,6 +69,18 @@ impl<'a, T: ExtendedBufRead + ?Sized + 'a> ExtendedBufRead for Box { } } +impl ReadlineBufRead for git_packetline::read::WithSidebands<'_, T, fn(bool, &[u8])> { + fn readline(&mut self) -> Option, git_packetline::decode::Error>>> { + self.read_data_line() + } +} + +impl<'a, T: io::Read> ReadlineBufRead for git_packetline::read::WithSidebands<'a, T, HandleProgress> { + fn readline(&mut self) -> Option, git_packetline::decode::Error>>> { + self.read_data_line() + } +} + impl<'a, T: io::Read> ExtendedBufRead for git_packetline::read::WithSidebands<'a, T, HandleProgress> { fn set_progress_handler(&mut self, handle_progress: Option) { self.set_progress_handler(handle_progress) diff --git a/git-transport/src/client/blocking_io/http/mod.rs b/git-transport/src/client/blocking_io/http/mod.rs index a75dcccc50c..b4a8e0b449d 100644 --- a/git-transport/src/client/blocking_io/http/mod.rs +++ b/git-transport/src/client/blocking_io/http/mod.rs @@ -9,6 +9,7 @@ use bstr::BStr; use git_packetline::PacketLineRef; pub use traits::{Error, GetResponse, Http, PostResponse}; +use crate::client::blocking_io::bufread_ext::ReadlineBufRead; use crate::{ client::{self, capabilities, Capabilities, ExtendedBufRead, HandleProgress, MessageKind, RequestWriter}, Protocol, Service, @@ -387,14 +388,14 @@ impl HeadersThenBody { } } -impl Read for HeadersThenBody { +impl Read for HeadersThenBody { fn read(&mut self, buf: &mut [u8]) -> std::io::Result { self.handle_headers()?; self.body.read(buf) } } -impl BufRead for HeadersThenBody { +impl BufRead for HeadersThenBody { fn fill_buf(&mut self) -> std::io::Result<&[u8]> { self.handle_headers()?; self.body.fill_buf() @@ -405,6 +406,12 @@ impl BufRead for HeadersThenBody { } } +impl ReadlineBufRead for HeadersThenBody { + fn readline(&mut self) -> Option, git_packetline::decode::Error>>> { + self.body.readline() + } +} + impl ExtendedBufRead for HeadersThenBody { fn set_progress_handler(&mut self, handle_progress: Option) { self.body.set_progress_handler(handle_progress) diff --git a/git-transport/src/client/blocking_io/mod.rs b/git-transport/src/client/blocking_io/mod.rs index 48f4073cf3a..dfb3752af95 100644 --- a/git-transport/src/client/blocking_io/mod.rs +++ b/git-transport/src/client/blocking_io/mod.rs @@ -8,7 +8,7 @@ pub mod file; pub mod http; mod bufread_ext; -pub use bufread_ext::{ExtendedBufRead, HandleProgress}; +pub use bufread_ext::{ExtendedBufRead, HandleProgress, ReadlineBufRead}; mod request; pub use request::RequestWriter; diff --git a/git-transport/src/client/blocking_io/traits.rs b/git-transport/src/client/blocking_io/traits.rs index 0b1bae1cc1b..d0c91175b61 100644 --- a/git-transport/src/client/blocking_io/traits.rs +++ b/git-transport/src/client/blocking_io/traits.rs @@ -1,4 +1,4 @@ -use std::{io, io::Write, ops::DerefMut}; +use std::{io::Write, ops::DerefMut}; use bstr::BString; @@ -14,7 +14,7 @@ pub struct SetServiceResponse<'a> { /// The capabilities parsed from the server response. pub capabilities: Capabilities, /// In protocol version one, this is set to a list of refs and their peeled counterparts. - pub refs: Option>, + pub refs: Option>, } /// All methods provided here must be called in the correct order according to the [communication protocol][Protocol] diff --git a/git-transport/src/client/capabilities.rs b/git-transport/src/client/capabilities.rs index 3f1c2e93922..89b60a96dbb 100644 --- a/git-transport/src/client/capabilities.rs +++ b/git-transport/src/client/capabilities.rs @@ -15,9 +15,9 @@ pub enum Error { #[error("a version line was expected, but none was retrieved")] MissingVersionLine, #[error("expected 'version X', got {0:?}")] - MalformattedVersionLine(String), + MalformattedVersionLine(BString), #[error("Got unsupported version '{}', expected {actual:?}", *desired as u8)] - UnsupportedVersion { desired: Protocol, actual: String }, + UnsupportedVersion { desired: Protocol, actual: BString }, #[error("An IO error occurred while reading V2 lines")] Io(#[from] std::io::Error), } @@ -81,34 +81,32 @@ impl Capabilities { )) } - /// Parse capabilities from the given a `first_line` and the rest of the lines as single newline - /// separated string via `remaining_lines`. + /// Parse capabilities from the given a `lines_buf` which is expected to be all newline separated lines + /// from the server. /// /// Useful for parsing capabilities from a data sent from a server, and to avoid having to deal with /// blocking and async traits for as long as possible. There is no value in parsing a few bytes /// in a non-blocking fashion. - pub fn from_lines( - first_line: Option>>, - remaining_lines: impl Into, - ) -> Result { - let version_line = first_line.map(Into::into).ok_or(Error::MissingVersionLine)??; + pub fn from_lines(lines_buf: BString) -> Result { + let mut lines = <_ as bstr::ByteSlice>::lines(lines_buf.as_slice().trim()); + let version_line = lines.next().ok_or(Error::MissingVersionLine)?; let (name, value) = version_line.split_at( version_line - .find(' ') - .ok_or_else(|| Error::MalformattedVersionLine(version_line.clone()))?, + .find(b" ") + .ok_or_else(|| Error::MalformattedVersionLine(version_line.to_owned().into()))?, ); - if name != "version" { - return Err(Error::MalformattedVersionLine(version_line)); + if name != b"version" { + return Err(Error::MalformattedVersionLine(version_line.to_owned().into())); } - if value != " 2" { + if value != b" 2" { return Err(Error::UnsupportedVersion { desired: Protocol::V2, - actual: value.to_owned(), + actual: value.to_owned().into(), }); } Ok(Capabilities { value_sep: b'\n', - data: remaining_lines.into().into(), + data: lines.as_bytes().into(), }) } @@ -154,7 +152,8 @@ impl Capabilities { #[cfg(feature = "blocking-client")] /// pub mod recv { - use std::{io, io::BufRead}; + use std::io; + use std::io::Read; use crate::{client, client::Capabilities, Protocol}; @@ -166,7 +165,7 @@ pub mod recv { /// /// This is `Some` only when protocol v1 is used. The [`io::BufRead`] must be exhausted by /// the caller. - pub refs: Option>, + pub refs: Option>, /// The [`Protocol`] the remote advertised. pub protocol: Protocol, } @@ -203,9 +202,10 @@ pub mod recv { } Protocol::V2 => Ok(Outcome { capabilities: { - let rd = rd.as_read(); - let mut lines = rd.lines(); - Capabilities::from_lines(lines.next(), lines.collect::, _>>()?.join("\n"))? + let mut rd = rd.as_read(); + let mut buf = Vec::new(); + rd.read_to_end(&mut buf)?; + Capabilities::from_lines(buf.into())? }, refs: None, protocol: Protocol::V2, @@ -219,8 +219,8 @@ pub mod recv { #[allow(missing_docs)] /// pub mod recv { - use futures_io::{AsyncBufRead, AsyncRead}; - use futures_lite::{AsyncBufReadExt, StreamExt}; + use futures_io::AsyncRead; + use futures_lite::AsyncReadExt; use crate::{client, client::Capabilities, Protocol}; @@ -232,7 +232,7 @@ pub mod recv { /// /// This is `Some` only when protocol v1 is used. The [`AsyncBufRead`] must be exhausted by /// the caller. - pub refs: Option>, + pub refs: Option>, /// The [`Protocol`] the remote advertised. pub protocol: Protocol, } @@ -270,14 +270,10 @@ pub mod recv { } Protocol::V2 => Ok(Outcome { capabilities: { - let rd = rd.as_read(); - let mut lines_with_err = rd.lines(); - let mut lines = Vec::new(); - while let Some(line) = lines_with_err.next().await { - lines.push(line?); - } - let mut lines = lines.into_iter(); - Capabilities::from_lines(lines.next().map(Ok), lines.collect::>().join("\n"))? + let mut rd = rd.as_read(); + let mut buf = Vec::new(); + rd.read_to_end(&mut buf).await?; + Capabilities::from_lines(buf.into())? }, refs: None, protocol: Protocol::V2, diff --git a/git-transport/src/client/mod.rs b/git-transport/src/client/mod.rs index 9285167387c..0eeb6f145d6 100644 --- a/git-transport/src/client/mod.rs +++ b/git-transport/src/client/mod.rs @@ -2,7 +2,8 @@ mod async_io; #[cfg(feature = "async-client")] pub use async_io::{ - connect, ExtendedBufRead, HandleProgress, RequestWriter, SetServiceResponse, Transport, TransportV2Ext, + connect, ExtendedBufRead, HandleProgress, ReadlineBufRead, RequestWriter, SetServiceResponse, Transport, + TransportV2Ext, }; mod traits; @@ -14,7 +15,8 @@ mod blocking_io; pub use blocking_io::http; #[cfg(feature = "blocking-client")] pub use blocking_io::{ - connect, file, ssh, ExtendedBufRead, HandleProgress, RequestWriter, SetServiceResponse, Transport, TransportV2Ext, + connect, file, ssh, ExtendedBufRead, HandleProgress, ReadlineBufRead, RequestWriter, SetServiceResponse, Transport, + TransportV2Ext, }; #[cfg(feature = "blocking-client")] #[doc(inline)]