Skip to content

Commit

Permalink
[clone] abort early on HTTP status errors
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Aug 23, 2020
1 parent 791c05e commit e829c0a
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 6 deletions.
39 changes: 36 additions & 3 deletions git-transport/src/client/http/curl.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use crate::client::http;
use curl::easy::Easy2;
use git_features::pipe;
use std::io::{Read, Write};
use std::{
io,
io::{Read, Write},
sync::mpsc::{sync_channel, Receiver, SyncSender, TrySendError},
thread,
};
Expand All @@ -13,6 +13,25 @@ struct Handler {
send_header: Option<pipe::Writer>,
send_data: Option<pipe::Writer>,
receive_body: Option<pipe::Reader>,
checked_status: bool,
}

impl Handler {
fn reset(&mut self) {
self.checked_status = false;
}
fn parse_status(data: &[u8]) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
let code = data
.split(|b| *b == b' ')
.nth(1)
.ok_or_else(|| "Expected HTTP/<VERSION> STATUS")?;
let code = std::str::from_utf8(code)?;
let status: usize = code.parse()?;
if status < 200 || status > 299 {
return Err(format!("Received HTTP status {}", status).into());
}
Ok(())
}
}

impl curl::easy::Handler for Handler {
Expand All @@ -31,9 +50,21 @@ impl curl::easy::Handler for Handler {
}

fn header(&mut self, data: &[u8]) -> bool {
// TODO: check for HTTP status!
match self.send_header.as_mut() {
Some(writer) => writer.write_all(data).is_ok(),
Some(writer) => {
if self.checked_status {
writer.write_all(data).is_ok()
} else {
self.checked_status = true;
match Handler::parse_status(data) {
Ok(()) => true,
Err(err) => {
writer.channel.send(Err(io::Error::new(io::ErrorKind::Other, err))).ok();
false
}
}
}
}
None => false,
}
}
Expand Down Expand Up @@ -164,6 +195,7 @@ fn new_remote_curl() -> (

if let Err(err) = handle.perform() {
let handler = handle.get_mut();
handler.reset();
let err = Err(io::Error::new(io::ErrorKind::Other, err));
handler.receive_body.take();
match (handler.send_header.take(), handler.send_data.take()) {
Expand All @@ -183,6 +215,7 @@ fn new_remote_curl() -> (
};
} else {
let handler = handle.get_mut();
handler.reset();
handler.receive_body.take();
handler.send_header.take();
handler.send_data.take();
Expand Down
6 changes: 4 additions & 2 deletions git-transport/tests/client/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,18 @@ fn serve_and_connect(
}

#[test]
#[ignore]
fn http_error_results_in_observable_error() -> crate::Result {
let (_server, mut client) = serve_and_connect("http-404.response", "path/not-important", Protocol::V1)?;
use std::error::Error;
assert_eq!(
client
.set_service(Service::UploadPack)
.err()
.expect("non-200 status causes error")
.source()
.expect("source")
.to_string(),
"something about the status"
"Received HTTP status 404"
);
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
* [x] pipe to link writers with readers using bytes
* [x] piped iterator
* [x] HTTP trait for simple gets and post implemented for Curl
* [ ] propagate HTTP status code
* [x] propagate HTTP status code
* [x] non-OK is propagated
* [ ] test for auto-reset on ReadWithProgress drop
* [ ] timeout configuration
Expand Down

0 comments on commit e829c0a

Please sign in to comment.