feat(server): make Http compatible with TcpServer

[seanmonstar]

This implements From<Message> for Request and Into<Message> for Response, allowing an Http instance to be used with a TcpServer
from tokio-proto.

Closes #1036

BREAKING CHANGE: This makes Request.remote_addr an
Option<SocketAddr>, instead of SocketAddr.

---

@alexcrichton This would make hyper's Http compatible with TcpServer. Requiring a secondary trait to be able to get a SocketAddr felt like more of a pain, so instead I've settled on allowing the address to be an Option. Does it seem like TcpServer could gain the ability to collect the SocketAddr later, such as for 0.2, and we can make use of it then? If so, I can file a tracking bug.

[alexcrichton]

Currently we don't have super concrete plans to expose SocketAddr itself specifically, but the thinking was that this would be a bound on the I/O objects that hyper operates with. Right now it's Io but the theory is that if Hyper wanted to extract a remote address it'd instead be Io + MyTrait.

This'd be a great trait for specialization to avoid imposing extra requirements, but unfortunately that's not there just yet :(.

[mattwoodyard]

Compile with this PR, still produces an error. It looks to me like the BindServer should pick work via ServerProto. Clearly, the compiler disagrees with me. Should hyper need a BindServer impl? If so why does the ServerProto impl no fulfill the bounds? Is there a way to make the compiler spit out more type inference detail?
|
213 | let tcp = TcpServer::new(Http::new(), addr);
| ^^^^^^^^^^^^^^ the trait tokio_proto::BindServer<_, tokio_core::net::TcpStream> is not implemented for hyper::server::Http
|

[seanmonstar]

@mattwoodyard It looks like the generalization of tokio's TcpServer hasn't been released on crates.io yet. If I change the the version of tokio-proto to a git dependency, my example compiles using TcpServer.

@alexcrichton hm, so I can create a RemoteAddr trait or similar, and then change the bounds to T: Io + RemoteAddr + 'static. I can implement this easily for TcpStream, but it breaks down quickly. I cannot implement the trait for TlsStream without depending on it, and then anyone who wants to make use tokio-tls also cannot implement the trait, since it is then a foreign type and foreign trait. The user then needs to create a wrapper, and then implement Read + Write + Io + RemoteAddr for it, and groan.

It might be possible to alleviate that if I include an impl<T: Io + AsRef<TcpStream>> RemoteAddr for T, which could work for TcpStream and TlsStream<TcpStream>, if rust-native-tls were to implement AsRef<T> (it doesn't currently, but does have get_ref() methods). However, that seems rather fragile. Also seems like besides hyper's desire for a RemoteAddr trait, various other protocols will also want to get that.

[alexcrichton]

@seanmonstar yeah it's true the situation isn't great no matter how you slice it right now. That's where I think that specialization will be best here because you really do want to work with all Io, it's just that optionally sometimes there's an extra piece of information to learn either through AsRef, a custom trait, or such.

In the meantime what do you think about with being conservative and removing the method from Request? It can always be added back later, and I was under the assumption that you typically don't want the TCP level peer because you've got some proxy like nginx in front of you anyway.

[mattwoodyard]

I have no idea what I had done, but a couple rounds of git pull; and cargo update got it building. Works now, with ssl too. Thanks.