Handle HTTPS errors more nicely

This commit adds an error case for when the HTTP headers fail to parse, and changes the "server returned 400" case with a "server returned something other than 200" case, which is what it was actually doing anyway.

Some xtests that speak to httpbin have been added.

dns-transport/src/error.rs

@@ -2,8 +2,11 @@
 #[derive(Debug)]
 pub enum Error {
 
-    /// There was a problem with the network sending the request or receiving
-    /// a response asynchorously.
+    /// The data in the response did not parse correctly from the DNS wire
+    /// protocol format.
+    WireError(dns::WireError),
+
+    /// There was a problem with the network making a TCP or UDP request.
     NetworkError(std::io::Error),
 
     /// There was a problem making a TLS request.
@@ -14,13 +17,14 @@ pub enum Error {
     #[cfg(feature="tls")]
     TlsHandshakeError(native_tls::HandshakeError<std::net::TcpStream>),
 
-    /// The data in the response did not parse correctly from the DNS wire
-    /// protocol format.
-    WireError(dns::WireError),
+    /// There was a problem decoding the response HTTP headers or body.
+    #[cfg(feature="https")]
+    HttpError(httparse::Error),
 
-    /// The server specifically indicated that the request we sent it was
-    /// malformed.
-    BadRequest,
+    /// The HTTP response code was something other than 200 OK, along with the
+    /// response code text, if present.
+    #[cfg(feature="https")]
+    WrongHttpStatus(u16, Option<String>),
 }
 
 
@@ -33,21 +37,28 @@ impl From<dns::WireError> for Error {
 }
 
 impl From<std::io::Error> for Error {
-    fn from(inner: std::io::Error) -> Error {
+    fn from(inner: std::io::Error) -> Self {
         Self::NetworkError(inner)
     }
 }
 
 #[cfg(feature="tls")]
 impl From<native_tls::Error> for Error {
-    fn from(inner: native_tls::Error) -> Error {
+    fn from(inner: native_tls::Error) -> Self {
         Self::TlsError(inner)
     }
 }
 
 #[cfg(feature="tls")]
 impl From<native_tls::HandshakeError<std::net::TcpStream>> for Error {
-    fn from(inner: native_tls::HandshakeError<std::net::TcpStream>) -> Error {
+    fn from(inner: native_tls::HandshakeError<std::net::TcpStream>) -> Self {
         Self::TlsHandshakeError(inner)
     }
 }
+
+#[cfg(feature="https")]
+impl From<httparse::Error> for Error {
+    fn from(inner: httparse::Error) -> Self {
+        Self::HttpError(inner)
+    }
+}

dns-transport/src/https.rs

@@ -80,11 +80,12 @@ impl Transport for HttpsTransport {
 
         let mut headers = [httparse::EMPTY_HEADER; 16];
         let mut response = httparse::Response::new(&mut headers);
-        let index: usize = response.parse(&buf).unwrap().unwrap();
+        let index: usize = response.parse(&buf)?.unwrap();
         let body = &buf[index .. read_len];
 
         if response.code != Some(200) {
-            return Err(Error::BadRequest);
+            let reason = response.reason.map(|e| e.to_string());
+            return Err(Error::WrongHttpStatus(response.code.unwrap(), reason));
         }
 
         for header in response.headers {

src/output.rs

@@ -506,25 +506,30 @@ pub fn print_error_code(rcode: ErrorCode) {
 /// to the user so they can debug what went wrong.
 fn erroneous_phase(error: &TransportError) -> &'static str {
 	match error {
+		TransportError::WireError(_)          => "protocol",
 		TransportError::NetworkError(_)       => "network",
         #[cfg(feature="tls")]
 		TransportError::TlsError(_)           |
 		TransportError::TlsHandshakeError(_)  => "tls",
-		TransportError::BadRequest            => "http-status",
-		TransportError::WireError(_)          => "protocol",
+        #[cfg(feature="https")]
+		TransportError::HttpError(_)          |
+		TransportError::WrongHttpStatus(_,_)  => "http",
 	}
 }
 
 /// Formats an error into its human-readable message.
 fn error_message(error: TransportError) -> String {
 	match error {
+		TransportError::WireError(e)          => wire_error_message(e),
 		TransportError::NetworkError(e)       => e.to_string(),
         #[cfg(feature="tls")]
 		TransportError::TlsError(e)           => e.to_string(),
         #[cfg(feature="tls")]
 		TransportError::TlsHandshakeError(e)  => e.to_string(),
-		TransportError::BadRequest            => "Nameserver returned HTTP 400 Bad Request".into(),
-		TransportError::WireError(e)          => wire_error_message(e),
+        #[cfg(feature="https")]
+		TransportError::HttpError(e)          => e.to_string(),
+        #[cfg(feature="https")]
+		TransportError::WrongHttpStatus(t,r)  => format!("Nameserver returned HTTP {} ({})", t, r.unwrap_or_else(|| "No reason".into()))
 	}
 }
 

xtests/bins.toml

@@ -0,0 +1,15 @@
+[[cmd]]
+name = "Using a DNS-over-HTTPS server that returns status 500"
+shell = "dog --https @https://eu.httpbin.org/status/500 lookup.dog"
+stdout = { empty = true }
+stderr = { string = "Error [http]: Nameserver returned HTTP 500 (INTERNAL SERVER ERROR)" }
+status = 1
+tags = [ 'live', 'httpbin', 'https' ]
+
+[[cmd]]
+name = "Using a DNS-over-HTTPS server that returns no content"
+shell = "dog --https @https://eu.httpbin.org/status/200 lookup.dog"
+stdout = { empty = true }
+stderr = { string = "Error [protocol]: Malformed packet: insufficient data" }
+status = 1
+tags = [ 'live', 'httpbin', 'https' ]