Skip to content

Commit

Permalink
Fix an issue with using non-reusable body types with redirects (#19134)
Browse files Browse the repository at this point in the history
Closes #19131
Closes #19039

fixes the broken auto-updater.

I had the bright idea of using streams as the most common unit of data
transfer. Unfortunately, streams are not re-usable. So HTTP redirects
that have a stream body (like our remote server and auto update
downloads), don't redirect, as they can't reuse the stream. This PR
fixes the problem and simplifies the AsyncBody implementation now that
we're not using Isahc.

Release Notes:

- N/A
  • Loading branch information
mikayla-maki authored Oct 12, 2024
1 parent 9e14fd9 commit b2e844f
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 67 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ blade-graphics = { git = "https://github.com/kvark/blade", rev = "e142a3a5e678eb
blade-macros = { git = "https://github.com/kvark/blade", rev = "e142a3a5e678eb6a13e642ad8401b1f3aa38e969" }
blade-util = { git = "https://github.com/kvark/blade", rev = "e142a3a5e678eb6a13e642ad8401b1f3aa38e969" }
blake3 = "1.5.3"
bytes = "1.0"
cargo_metadata = "0.18"
cargo_toml = "0.20"
chrono = { version = "0.4", features = ["serde"] }
Expand Down
2 changes: 1 addition & 1 deletion crates/http_client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ path = "src/http_client.rs"
doctest = true

[dependencies]
bytes.workspace = true
anyhow.workspace = true
derive_more.workspace = true
futures.workspace = true
http = "1.1"
log.workspace = true
serde.workspace = true
serde_json.workspace = true
smol.workspace = true
url.workspace = true
57 changes: 32 additions & 25 deletions crates/http_client/src/async_body.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
use std::{borrow::Cow, io::Read, pin::Pin, task::Poll};
use std::{
io::{Cursor, Read},
pin::Pin,
task::Poll,
};

use futures::{AsyncRead, AsyncReadExt};
use bytes::Bytes;
use futures::AsyncRead;

/// Based on the implementation of AsyncBody in
/// https://github.com/sagebind/isahc/blob/5c533f1ef4d6bdf1fd291b5103c22110f41d0bf0/src/body/mod.rs
Expand All @@ -11,7 +16,7 @@ pub enum Inner {
Empty,

/// A body stored in memory.
SyncReader(std::io::Cursor<Cow<'static, [u8]>>),
Bytes(std::io::Cursor<Bytes>),

/// An asynchronous reader.
AsyncReader(Pin<Box<dyn futures::AsyncRead + Send + Sync>>),
Expand All @@ -32,6 +37,10 @@ impl AsyncBody {
{
Self(Inner::AsyncReader(Box::pin(read)))
}

pub fn from_bytes(bytes: Bytes) -> Self {
Self(Inner::Bytes(Cursor::new(bytes.clone())))
}
}

impl Default for AsyncBody {
Expand All @@ -46,45 +55,43 @@ impl From<()> for AsyncBody {
}
}

impl From<Vec<u8>> for AsyncBody {
fn from(body: Vec<u8>) -> Self {
Self(Inner::SyncReader(std::io::Cursor::new(Cow::Owned(body))))
impl From<Bytes> for AsyncBody {
fn from(bytes: Bytes) -> Self {
Self::from_bytes(bytes)
}
}

impl From<&'_ [u8]> for AsyncBody {
fn from(body: &[u8]) -> Self {
body.to_vec().into()
impl From<Vec<u8>> for AsyncBody {
fn from(body: Vec<u8>) -> Self {
Self::from_bytes(body.into())
}
}

impl From<String> for AsyncBody {
fn from(body: String) -> Self {
body.into_bytes().into()
Self::from_bytes(body.into())
}
}

impl From<&'_ str> for AsyncBody {
fn from(body: &str) -> Self {
body.as_bytes().into()
impl From<&'static [u8]> for AsyncBody {
#[inline]
fn from(s: &'static [u8]) -> Self {
Self::from_bytes(Bytes::from_static(s))
}
}

impl From<&'static str> for AsyncBody {
#[inline]
fn from(s: &'static str) -> Self {
Self::from_bytes(Bytes::from_static(s.as_bytes()))
}
}

impl<T: Into<Self>> From<Option<T>> for AsyncBody {
fn from(body: Option<T>) -> Self {
match body {
Some(body) => body.into(),
None => Self(Inner::Empty),
}
}
}

impl std::io::Read for AsyncBody {
fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
match &mut self.0 {
Inner::Empty => Ok(0),
Inner::SyncReader(cursor) => cursor.read(buf),
Inner::AsyncReader(async_reader) => smol::block_on(async_reader.read(buf)),
None => Self::empty(),
}
}
}
Expand All @@ -100,7 +107,7 @@ impl futures::AsyncRead for AsyncBody {
match inner {
Inner::Empty => Poll::Ready(Ok(0)),
// Blocking call is over an in-memory buffer
Inner::SyncReader(cursor) => Poll::Ready(cursor.read(buf)),
Inner::Bytes(cursor) => Poll::Ready(cursor.read(buf)),
Inner::AsyncReader(async_reader) => {
AsyncRead::poll_read(async_reader.as_mut(), cx, buf)
}
Expand Down
3 changes: 2 additions & 1 deletion crates/http_client/src/http_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ use std::{
};
pub use url::Url;

#[derive(Clone)]
#[derive(Clone, Debug)]
pub struct ReadTimeout(pub Duration);

impl Default for ReadTimeout {
fn default() -> Self {
Self(Duration::from_secs(5))
Expand Down
3 changes: 3 additions & 0 deletions crates/project/src/worktree_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ impl WorktreeStore {
if abs_path.starts_with("/~") {
abs_path = abs_path[1..].to_string();
}
if abs_path.is_empty() {
abs_path = "~/".to_string();
}
let root_name = PathBuf::from(abs_path.clone())
.file_name()
.unwrap()
Expand Down
17 changes: 16 additions & 1 deletion crates/recent_projects/src/ssh_connections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,9 +515,24 @@ pub async fn open_ssh_project(

let did_open_ssh_project = cx
.update(|cx| {
workspace::open_ssh_project(window, connection_options, delegate, app_state, paths, cx)
workspace::open_ssh_project(
window,
connection_options,
delegate.clone(),
app_state,
paths,
cx,
)
})?
.await;

let did_open_ssh_project = match did_open_ssh_project {
Ok(ok) => Ok(ok),
Err(e) => {
delegate.update_error(e.to_string(), cx);
Err(e)
}
};

did_open_ssh_project
}
2 changes: 1 addition & 1 deletion crates/reqwest_client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ path = "examples/client.rs"

[dependencies]
anyhow.workspace = true
bytes = "1.0"
bytes.workspace = true
futures.workspace = true
http_client.workspace = true
serde.workspace = true
Expand Down
39 changes: 2 additions & 37 deletions crates/reqwest_client/src/reqwest_client.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{any::type_name, borrow::Cow, io::Read, mem, pin::Pin, sync::OnceLock, task::Poll};
use std::{any::type_name, mem, pin::Pin, sync::OnceLock, task::Poll};

use anyhow::anyhow;
use bytes::{BufMut, Bytes, BytesMut};
Expand Down Expand Up @@ -173,39 +173,6 @@ pub fn poll_read_buf(
Poll::Ready(Ok(n))
}

struct SyncReader {
cursor: Option<std::io::Cursor<Cow<'static, [u8]>>>,
}

impl SyncReader {
fn new(cursor: std::io::Cursor<Cow<'static, [u8]>>) -> Self {
Self {
cursor: Some(cursor),
}
}
}

impl futures::stream::Stream for SyncReader {
type Item = Result<Bytes, std::io::Error>;

fn poll_next(
mut self: std::pin::Pin<&mut Self>,
_cx: &mut std::task::Context<'_>,
) -> std::task::Poll<Option<Self::Item>> {
let Some(mut cursor) = self.cursor.take() else {
return Poll::Ready(None);
};

let mut buf = Vec::new();
match cursor.read_to_end(&mut buf) {
Ok(_) => {
return Poll::Ready(Some(Ok(Bytes::from(buf))));
}
Err(e) => return Poll::Ready(Some(Err(e))),
}
}
}

impl http_client::HttpClient for ReqwestClient {
fn proxy(&self) -> Option<&http::Uri> {
self.proxy.as_ref()
Expand Down Expand Up @@ -238,9 +205,7 @@ impl http_client::HttpClient for ReqwestClient {
}
let request = request.body(match body.0 {
http_client::Inner::Empty => reqwest::Body::default(),
http_client::Inner::SyncReader(cursor) => {
reqwest::Body::wrap_stream(SyncReader::new(cursor))
}
http_client::Inner::Bytes(cursor) => cursor.into_inner().into(),
http_client::Inner::AsyncReader(stream) => {
reqwest::Body::wrap_stream(StreamReader::new(stream))
}
Expand Down

0 comments on commit b2e844f

Please sign in to comment.