From 74f66ef747e1be6a6e9eff232172667ef03e26bf Mon Sep 17 00:00:00 2001
From: Adam Wick <awick@uhsure.com>
Date: Sun, 21 Nov 2021 21:08:29 -0800
Subject: [PATCH] Add a separate trait for converting errors into server
 responses.

---
 src/messages/server_response.rs | 13 ++++++++++---
 src/network/generic.rs          | 17 +++++++++++++++--
 src/network/standard.rs         |  9 ++++++---
 src/network/testing.rs          |  6 +++---
 4 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/src/messages/server_response.rs b/src/messages/server_response.rs
index f5b8274..611957e 100644
--- a/src/messages/server_response.rs
+++ b/src/messages/server_response.rs
@@ -1,4 +1,5 @@
 use crate::errors::{DeserializationError, SerializationError};
+use crate::network::generic::IntoErrorResponse;
 use crate::network::SOCKSv5Address;
 use crate::serialize::read_amt;
 use crate::standard_roundtrip;
@@ -37,6 +38,12 @@ pub enum ServerResponseStatus {
     AddressTypeNotSupported,
 }
 
+impl IntoErrorResponse for ServerResponseStatus {
+    fn into_response(&self) -> ServerResponseStatus {
+        self.clone()
+    }
+}
+
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct ServerResponse {
     pub status: ServerResponseStatus,
@@ -45,9 +52,9 @@ pub struct ServerResponse {
 }
 
 impl ServerResponse {
-    pub fn error<E: Into<ServerResponseStatus>>(resp: E) -> ServerResponse {
+    pub fn error<E: IntoErrorResponse>(resp: &E) -> ServerResponse {
         ServerResponse {
-            status: resp.into(),
+            status: resp.into_response(),
             bound_address: SOCKSv5Address::IP4(Ipv4Addr::new(0, 0, 0, 0)),
             bound_port: 0,
         }
@@ -189,7 +196,7 @@ fn check_bad_command() {
 #[test]
 fn short_write_fails_right() {
     let mut buffer = [0u8; 2];
-    let cmd = ServerResponse::error(ServerResponseStatus::AddressTypeNotSupported);
+    let cmd = ServerResponse::error(&ServerResponseStatus::AddressTypeNotSupported);
     let mut cursor = Cursor::new(&mut buffer as &mut [u8]);
     let result = task::block_on(cmd.write(&mut cursor));
     match result {
diff --git a/src/network/generic.rs b/src/network/generic.rs
index ff89d4b..0a82759 100644
--- a/src/network/generic.rs
+++ b/src/network/generic.rs
@@ -4,7 +4,7 @@ use crate::network::datagram::GenericDatagramSocket;
 use crate::network::listener::GenericListener;
 use crate::network::stream::GenericStream;
 use async_trait::async_trait;
-use std::fmt::Display;
+use std::fmt::{Debug, Display};
 
 #[async_trait]
 pub trait Networklike {
@@ -12,7 +12,7 @@ pub trait Networklike {
     /// for using only one; if you have a use case for separating your errors,
     /// please shoot the author(s) and email to split this into multiple types, one
     /// for each trait function.
-    type Error: Display + Into<ServerResponseStatus> + Send;
+    type Error: Debug + Display + IntoErrorResponse + Send;
 
     /// Connect to the given address and port, over this kind of network. The
     /// underlying stream should behave somewhat like a TCP stream ... which
@@ -46,3 +46,16 @@ pub trait Networklike {
         port: u16,
     ) -> Result<GenericDatagramSocket<Self::Error>, Self::Error>;
 }
+
+/// This trait is a hack; sorry about that. The thing is, we want to be able to
+/// convert Errors from the `Networklike` trait into a `ServerResponseStatus`,
+/// but want to do so on references to the error object rather than the actual
+/// object. This is for the paired reason that (a) we want to be able to use
+/// the errors in multiple places -- for example, to return a value to the client
+/// and then also to whoever called the function -- and (b) some common errors
+/// (I'm looking at you, `io::Error`) aren't `Clone`. So ... hence this overly-
+/// specific trait.
+pub trait IntoErrorResponse {
+    #[allow(clippy::wrong_self_convention)]
+    fn into_response(&self) -> ServerResponseStatus;
+}
diff --git a/src/network/standard.rs b/src/network/standard.rs
index 91b4db7..e55f8e1 100644
--- a/src/network/standard.rs
+++ b/src/network/standard.rs
@@ -13,6 +13,9 @@ use async_trait::async_trait;
 use futures::AsyncWriteExt;
 use log::error;
 
+use super::generic::IntoErrorResponse;
+
+#[derive(Clone)]
 pub struct Builtin {}
 
 impl Builtin {
@@ -226,9 +229,9 @@ fn check_sanity() {
     });
 }
 
-impl From<io::Error> for ServerResponseStatus {
-    fn from(e: io::Error) -> ServerResponseStatus {
-        match e.kind() {
+impl IntoErrorResponse for io::Error {
+    fn into_response(&self) -> ServerResponseStatus {
+        match self.kind() {
             io::ErrorKind::ConnectionRefused => ServerResponseStatus::ConnectionRefused,
             io::ErrorKind::NotFound => ServerResponseStatus::HostUnreachable,
             _ => ServerResponseStatus::GeneralFailure,
diff --git a/src/network/testing.rs b/src/network/testing.rs
index 3f5ad1f..44310fd 100644
--- a/src/network/testing.rs
+++ b/src/network/testing.rs
@@ -6,7 +6,7 @@ use crate::network::address::{HasLocalAddress, SOCKSv5Address};
 #[cfg(test)]
 use crate::network::datagram::Datagramlike;
 use crate::network::datagram::GenericDatagramSocket;
-use crate::network::generic::Networklike;
+use crate::network::generic::{IntoErrorResponse, Networklike};
 use crate::network::listener::{GenericListener, Listenerlike};
 use crate::network::stream::GenericStream;
 use crate::network::testing::datagram::TestDatagram;
@@ -82,8 +82,8 @@ impl fmt::Display for TestStackError {
     }
 }
 
-impl From<TestStackError> for ServerResponseStatus {
-    fn from(_: TestStackError) -> Self {
+impl IntoErrorResponse for TestStackError {
+    fn into_response(&self) -> ServerResponseStatus {
         ServerResponseStatus::GeneralFailure
     }
 }