puniverse / comsat

Fibers and actors for web development
docs.paralleluniverse.co/comsat
Other
598 stars 103 forks source link

Get remote address from HttpRequest #50

Closed roded closed 8 years ago

roded commented 8 years ago

Hi, Can the remote address be exposed by HttpRequest? I'm guessing this is just a matter of implementing the wrappers' methods. Thanks

circlespainter commented 8 years ago

Hi, yes it is a very useful addition indeed. It didn't make for 0.6.0 but it'll be part of the next release.

I'll let you know as soon as there's a snapshot available including it.

roded commented 8 years ago

Much appreciated.

roded commented 8 years ago

This is now blocking for me. Can I assist with this in anyway?

Edit: Erm, nevermind. Not blocking but needed at some point.

circlespainter commented 8 years ago

Right now we have some higher priorities but we plan to have a look soon at it. Without commitment, it could be as soon as next week.

roded commented 8 years ago

No worries, I jumped the gun there.. Thanks.

circlespainter commented 8 years ago

A new 0.7.0-SNAPSHOT should now be available on SonaType with a new String HttpRequest::getSourceAddress() WebActors API implemented in all backends (servlet through getRemoteAddr, Undertow through getSourceAddress and Netty through the channel's getRemoteAddress) but without proxy-related header inspection (because this might not be what the caller wants). Let me know if this works for you and if you have any suggestions.

roded commented 8 years ago

Much obliged. Works fine (tried it for Undertow). As for suggestions, as the client code using HttpRequest has no knowledge of the underlying InetSocketAddress (when working w/ Netty or Undertow), maybe it would be better to expose 3 methods instead of the one: getAddress()/getHost()/getPort(). Or rather, use InetSocketAddress's getHostAddress() instead of toString() which has a special formatting: "host/ip:port". For your consideration.

circlespainter commented 8 years ago

No problem, the API was lacking that information indeed.

Yes I was pondering the same but I had missed getRemoteHost / getRemotePort in the servlet API so I ended up exposing the unparsed address. Thanks for the suggestion, I'll think again about it.

circlespainter commented 8 years ago

Exposing String getSourceHost() (using either InetSocketAddress::getHostString() or HttpServletRequest::getRemoteHost()) and int getSourcePort() (using either InetSocketAddress::getHostPort() or HttpServletRequest::getRemotePort()). Undertow eposes the InetSocketAddress and even in Netty-based Web Actors the SocketAddress willl always actually be InetSocketAddress (although there's a check).

roded commented 8 years ago

Cool. Is there a plan to install the latest snapshot on sonatype anytime soon?

pron commented 8 years ago

We'll do it tomorrow.

circlespainter commented 8 years ago

A new snapshot should be available on SonaType.