-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add connection metadata to HTTP::Server::Response #5870
Add connection metadata to HTTP::Server::Response #5870
Conversation
Adds HTTP::Server::Response#local_address and #remote_address
I'm not sure anymore about whether this is a good solution. It's not ideal, but at least relatively easy compared to a bigger refactor (see #5784 (comment)). |
Yeah, maybe having separate |
Hi all, Note that i am not interested in the (real) client ip (i know this is more complicated) but in the remote socket source ip and yes, i know the workarounds with adding an HTTP-Header through a reverse proxy. BTW: how is this patch applied in the most canonical way (best practice) as long as it isn't merged? Thanks for your reply & best regards, |
I don't think this should be merged soon. @asterite's suggestion to separate You shouldn't apply this patch directly to your local std lib copy, but rather add a monkey patch in your application code: class HTTP::Server::Response
# Returns the local address of the network socket used by this connection.
#
# This can be useful if the server listens on multiple sockets.
def local_address : Socket::Address?
if (socket = @io).responds_to? :local_address
socket.local_address
end
end
# Returns the remote address of the network socket used by this connection.
#
# This is not necessarily the address of the original requestor but the last
# proxy (e.g. a load balancer).
def remote_address : Socket::Address?
if (socket = @io).responds_to? :remote_address
socket.remote_address
end
end
end |
You probably won't need any of the changes in the first commit, they'll be in 0.25.0 (see #5869). |
Great, thanks a lot! BTW: I am impressed by the quick response I received here! |
I'll close this. Let's discuss a more general solution regarding |
This PR adds methods
#local_address
and#remote_address
address toHTTP::Server::Response
.I'm not entirely sure whether the method's return type should be nilable. It's not very typical to have a Response not wrapping a socket. This is mostly used as mockup for specs. Maybe we can avoid forcing usually unnecessary nil-checks in actual code. This could be done either by raising (though there should probably also be a non-raising option) or returning some default address if the IO does not provide one.
Closes #5784
Based on #5869