Enforce safety of downcast_ref at compile time. (#1326)

* Enforce safety of `downcast_ref` at compile time.

The safety of `downcast_ref` requires that `__private_get_type_id__` not
be overriden by callers, since the returned `TypeId` is used to check if
the cast is safe. However, all trait methods in Rust are public, so
users can override `__private_get_type_id__` despite it being
`#[doc(hidden)]`.

This commit makes `__private_get_type_id__` return a type with a private
constructor, ensuring that the only possible implementation is the
default implementation. A more detailed explanation is provided in the
comments added to the file.

Note that the standard library was affected by this type of issue with
the `Error::type_id` function: see https://blog.rust-lang.org/2019/05/14/Rust-1.34.2.html#whats-in-1.34.2-stable

Co-authored-by: Yuki Okushi <[email protected]>

Author: Aaron1011

actix-http/src/error.rs

@@ -60,6 +60,12 @@ impl Error {
     }
 }
 
+/// A struct with a private constructor, for use with
+/// `__private_get_type_id__`. Its single field is private,
+/// ensuring that it can only be constructed from this module
+#[doc(hidden)]
+pub struct PrivateHelper(());
+
 /// Error that can be converted to `Response`
 pub trait ResponseError: fmt::Debug + fmt::Display {
     /// Response's status code
@@ -83,19 +89,37 @@ pub trait ResponseError: fmt::Debug + fmt::Display {
         resp.set_body(Body::from(buf))
     }
 
+    /// A helper method to get the type ID of the type
+    /// this trait is implemented on.
+    /// This method is unsafe to *implement*, since `downcast_ref` relies
+    /// on the returned `TypeId` to perform a cast.
+    ///
+    /// Unfortunately, Rust has no notion of a trait method that is
+    /// unsafe to implement (marking it as `unsafe` makes it unsafe
+    /// to *call*). As a workaround, we require this method
+    /// to return a private type along with the `TypeId`. This
+    /// private type (`PrivateHelper`) has a private constructor,
+    /// making it impossible for safe code to construct outside of
+    /// this module. This ensures that safe code cannot violate
+    /// type-safety by implementing this method.
     #[doc(hidden)]
-    fn __private_get_type_id__(&self) -> TypeId
+    fn __private_get_type_id__(&self) -> (TypeId, PrivateHelper)
     where
         Self: 'static,
     {
-        TypeId::of::<Self>()
+        (TypeId::of::<Self>(), PrivateHelper(()))
     }
 }
 
 impl dyn ResponseError + 'static {
     /// Downcasts a response error to a specific type.
     pub fn downcast_ref<T: ResponseError + 'static>(&self) -> Option<&T> {
-        if self.__private_get_type_id__() == TypeId::of::<T>() {
+        if self.__private_get_type_id__().0 == TypeId::of::<T>() {
+            // Safety: external crates cannot override the default
+            // implementation of `__private_get_type_id__`, since
+            // it requires returning a private type. We can therefore
+            // rely on the returned `TypeId`, which ensures that this
+            // case is correct.
             unsafe { Some(&*(self as *const dyn ResponseError as *const T)) }
         } else {
             None