From 64a0d4dce54199c2c3752d48e91a5118bb898b91 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Thu, 17 Feb 2022 15:12:00 -0500
Subject: [PATCH] dirclient: Remove HttpStatus error variant

Getting a non-200 status is no longer a failure condition; it's just
a different kind of answer.

Closes #349.
---
 crates/tor-dirclient/src/err.rs    |  5 -----
 crates/tor-dirclient/src/lib.rs    |  9 +++++++--
 crates/tor-dirmgr/src/bootstrap.rs | 13 +++++++++++--
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/crates/tor-dirclient/src/err.rs b/crates/tor-dirclient/src/err.rs
index 06e4c7cca..a2cd20998 100644
--- a/crates/tor-dirclient/src/err.rs
+++ b/crates/tor-dirclient/src/err.rs
@@ -18,10 +18,6 @@ pub enum Error {
     #[error("truncated HTTP headers")]
     TruncatedHeaders,
 
-    /// Got an HTTP status other than 200
-    #[error("unexpected HTTP status {0:?}")]
-    HttpStatus(Option<u16>),
-
     /// Received a response that was longer than we expected.
     #[error("response too long; gave up after {0} bytes")]
     ResponseTooLong(usize),
@@ -94,7 +90,6 @@ impl HasKind for Error {
         match self {
             E::DirTimeout => EK::TorNetworkTimeout,
             E::TruncatedHeaders => EK::TorProtocolViolation,
-            E::HttpStatus(_) => EK::RemoteRefused,
             E::ResponseTooLong(_) => EK::TorProtocolViolation,
             E::Utf8Encoding(_) => EK::TorProtocolViolation,
             // TODO: it would be good to get more information out of the IoError
diff --git a/crates/tor-dirclient/src/lib.rs b/crates/tor-dirclient/src/lib.rs
index 62460a068..61af8ae31 100644
--- a/crates/tor-dirclient/src/lib.rs
+++ b/crates/tor-dirclient/src/lib.rs
@@ -179,7 +179,12 @@ where
     // TODO: should there be a separate timeout here?
     let header = read_headers(&mut buffered).await?;
     if header.status != Some(200) {
-        return Err(Error::HttpStatus(header.status));
+        return Ok(DirResponse::new(
+            header.status.unwrap_or(0),
+            None,
+            vec![],
+            source,
+        ));
     }
 
     let mut decoder = get_decoder(buffered, header.encoding.as_deref())?;
@@ -726,7 +731,7 @@ mod test {
         let response_text = b"HTTP/1.0 418 I'm a teapot\r\n\r\n";
         let (response, _request) = run_download_test(req, response_text);
 
-        assert!(matches!(response, Err(Error::HttpStatus(Some(418)))));
+        assert_eq!(response.unwrap().status_code(), 418);
     }
 
     #[test]
diff --git a/crates/tor-dirmgr/src/bootstrap.rs b/crates/tor-dirmgr/src/bootstrap.rs
index 68faf3d0f..3f5657cfb 100644
--- a/crates/tor-dirmgr/src/bootstrap.rs
+++ b/crates/tor-dirmgr/src/bootstrap.rs
@@ -94,9 +94,18 @@ async fn fetch_multiple<R: Runtime>(
 
     let mut useful_responses = Vec::new();
     for r in responses {
+        // TODO: on some error cases we might want to stop using this source.
         match r {
-            Ok(x) => useful_responses.push(x),
-            // TODO: in this case we might want to stop using this source.
+            Ok((request, response)) => {
+                if response.status_code() == 200 {
+                    useful_responses.push((request, response));
+                } else {
+                    trace!(
+                        "cache declined request; reported status {:?}",
+                        response.status_code()
+                    );
+                }
+            }
             Err(e) => warn!("error while downloading: {:?}", e),
         }
     }