Sorry for slow feedback. This is a lot to ponder, and a bit complicated by
A) I'm not a regular maintainer, just trying to help out and expedite PR reviews a bit since regular maintainer seems to be lacking time.
B) Natural complexity of melding TCP and Unix in a meaningful yet transparent way.
C) Several overlapping PR:s open currently

One immediate reflection regarding PeerAddr though; For a Unix peer, isn't the socket always unnamed? Only the server_addr() can even be linked to a path? This means that remote_addr() should probably return Option<&TcpAddr>, where None means client connected over Unix. Alternatively, an option-like enum RemoteAddr { Tcp(SocketAddr), Unix } could be devised for clarity, but I think it would be good to highlight the anonymous nature of Unix clients.

Hi Ulrik,

Thanks for taking the time to look over the patch.

I believe for a UnixStream, if the socket is bound to a file rather than anonymous, then peer_addr() will return a path to that file.
It's unclear if we want to treat an error fetching the peer address differently from a socket that has no peer address.

Either way, the current design of having remote_addr() return &SocketAddr is unfortunate, since it requires an unwrap call at

I think if we want to have Request represent either a Unix or TCP connection, we need to have an enum type to unify them, similar to the PeerAddr struct in this pull request. I'm curious if you see any alternatives.

Since adding Unix socket support requires changing the return type anyways, I think converting remote_addr() to return io::Result<PeerAddr> would make the most sense.

I believe for a UnixStream, if the socket is bound to a file rather than anonymous, then peer_addr() will return a path to that file.

From what I can tell, no. From the servers perspective (which is the case for tiny_http::Request), the client will be unnamed. https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=c8349309bedf30e242bbcbe9c6aeb4a5

I'm not against the PeerAddr-model as such. It's explicit = great, but I argue that that for Request::remote_addr() and a unix-socket, in practice it's always going to be an "unnamed" address. My gut drawback r.e. PeerAddr vs. Option is that the option-alternative give us a lot of important traits for free (Eq, Ord, Hash, Debug ...). I could fully endorse custom struct, if it
A) Does not hint at named "peer" on the type-level
B) Implements the Traits that SocketAddr does, so the casual app does not have to think too much about the unix special-case.

One, much more invasive way, to implement this could be to specialize the entire TinyHttp-instance with a sockettype. I think that needs a bit more thought, though.

I believe you're right. I was confusing local address and peer address.

This isn't a particularly useful use case, but a C client can bind the client socket to some path before calling connect(), no? Then peer_addr() wouldn't be (unnamed). I don't think it can bind to the same path it connects to though, since the server will have already bound to it.

I agree with you that for TCP, PeerAddr is a misnomer, and calling it SocketAddr would be more generic. We should also derive all the traits we can if we use an enum.

Using a type parameter for socket type would also work. I haven't deeply considered it, but I feel that would make it harder to write generic code that worked easily for both a Unix and TCP server.

Any news on this? I'm interested in hosting on Unix sockets.

This isn't a particularly useful use case, but a C client can bind the client socket to some path before calling connect(), no? Then peer_addr() wouldn't be (unnamed). I don't think it can bind to the same path it connects to though, since the server will have already bound to it.

This is possible. bind and connect are not mutually exclusive (though Rust does not provide a way to do it). I'm not sure how useful it is, but hey.

I can possibly take up the patches, but is this something that the maintainers will be willing to merge?

@ColonelThirtyTwo absolutely, it doesn't seem like there's any good reason not to include this, however we'll need to resolve the questions Rawler and fletcherw raise above with how we adjust the external API surface to handle these anonymous peer addresses.

If you want to open another PR with a rebased branch we can continue to discuss the changes there, I can close this one redirecting it over.

I feel like this has been overtaken by events, given that #224 is merged and released.