Use Pin<Box<S>> in BodyStream and SizedStream

Fixes actix#1321

A better fix would be to change `MessageBody` to take a `Pin<&mut
Self>`, rather than a `Pin<&mut Self>`. This will avoid requiring the
use of `Box` for all consumers by allowing the caller to determine how
to pin the `MessageBody` implementation (e.g. via stack pinning).

However, doing so is a breaking change that will affect every user of
`MessageBody`. By pinning the inner stream ourselves, we can fix the
undefined behavior without breaking the API.

I've included @sebzim4500's reproduction case as a new test case.
However, due to the nature of undefined behavior, this could pass (and
not segfault) even if underlying issue were to regress.

Unfortunately, until rust-lang/unsafe-code-guidelines#148 is resolved,
it's not even possible to write a Miri test that will pass when the bug
is fixed.

Author: Aaron1011

actix-http/src/body.rs

@@ -360,10 +360,8 @@ impl MessageBody for String {
 
 /// Type represent streaming body.
 /// Response does not contain `content-length` header and appropriate transfer encoding is used.
-#[pin_project]
 pub struct BodyStream<S, E> {
-    #[pin]
-    stream: S,
+    stream: Pin<Box<S>>,
     _t: PhantomData<E>,
 }
 
@@ -374,7 +372,7 @@ where
 {
     pub fn new(stream: S) -> Self {
         BodyStream {
-            stream,
+            stream: Box::pin(stream),
             _t: PhantomData,
         }
     }
@@ -390,29 +388,27 @@ where
     }
 
     fn poll_next(&mut self, cx: &mut Context<'_>) -> Poll<Option<Result<Bytes, Error>>> {
-        unsafe { Pin::new_unchecked(self) }
-            .project()
+        self
             .stream
+            .as_mut()
             .poll_next(cx)
             .map(|res| res.map(|res| res.map_err(std::convert::Into::into)))
     }
 }
 
 /// Type represent streaming body. This body implementation should be used
 /// if total size of stream is known. Data get sent as is without using transfer encoding.
-#[pin_project]
 pub struct SizedStream<S> {
     size: u64,
-    #[pin]
-    stream: S,
+    stream: Pin<Box<S>>,
 }
 
 impl<S> SizedStream<S>
 where
     S: Stream<Item = Result<Bytes, Error>>,
 {
     pub fn new(size: u64, stream: S) -> Self {
-        SizedStream { size, stream }
+        SizedStream { size, stream: Box::pin(stream) }
     }
 }
 
@@ -425,10 +421,7 @@ where
     }
 
     fn poll_next(&mut self, cx: &mut Context<'_>) -> Poll<Option<Result<Bytes, Error>>> {
-        unsafe { Pin::new_unchecked(self) }
-            .project()
-            .stream
-            .poll_next(cx)
+        self.stream.as_mut().poll_next(cx)
     }
 }
 

tests/test_weird_poll.rs

@@ -0,0 +1,26 @@
+// Regression test for #/1321
+
+use futures::task::{noop_waker, Context};
+use futures::stream::once;
+use actix_http::body::{MessageBody, BodyStream};
+use bytes::Bytes;
+
+#[test]
+fn weird_poll() {
+    let (sender, receiver) = futures::channel::oneshot::channel();
+    let mut body_stream = Ok(BodyStream::new(once(async {
+        let x = Box::new(0);
+        let y = &x;
+        receiver.await.unwrap();
+        let _z = **y;
+        Ok::<_, ()>(Bytes::new())
+    })));
+
+    let waker = noop_waker();
+    let mut context = Context::from_waker(&waker);
+
+    let _ = body_stream.as_mut().unwrap().poll_next(&mut context);
+    sender.send(()).unwrap();
+    let _ = std::mem::replace(&mut body_stream, Err([0; 32])).unwrap().poll_next(&mut context);
+}
+