Add HTTP::Request#remote_address

[asterite]

This PR adds a remote_address property to HTTP::Request.

Some details:

• the property is of type String?. It can be nil if the address can't be determined, but this usually won't happen. It's also nilable because for HTTP::Client it doesn't make sense. It's String and not a structured object like IPAddress because a middleware might want to overwrite this property, for example using the X-Forwarded-By or X-Real-IP headers. Also, this field is apparently commonly used for logging so it being a String is probably fine and the most flexible option.
• I'm not super convinced about using responds_to?(:remote_address) to determine this, I'd maybe like a module that defines this as an abstract method, but we can change the implementation later.
• I know I didn't document the local_address and remote_address getters of OpenSSL::SSL::Socket, but only because the underlying IO is not necessarily a IPSocket, so the type of those getters isn't IPAddress. Maybe we can assume the underlying IO is IPSocket, otherwise raise in this case. But this is something that we can improve later.
• If you have a better idea for the doc comments of HTTP::Request#remote_address, let me know

The idea here is to define an API that's usable and we won't modify later on. I think adding HTTP::Request#remote_address : String? is enough for this. We can redefine the implementation details I mention above in subsequent PRs.

Fixes #453
Related to #5784
Supersedes #7302

[asterite]

Also, we should probably add some way to parse a network address that looks like "ip:port" into {String, Int32}?. Right now we have Socket::IPAddress.parse(URI) but you have to pass something like "tcp://ip:port". But here we only care about the "ip:port" part (would work for IPv6 too) and getting those values as IP (string) and port (int).

But again, this can be done later on.

[straight-shoota]

Looks good.
Specs could be improved, though. Best case would be to merge #7197 for using run_server´. At least wait_for { server.listening? }` should be added here. But let's decide on what to do with #7197 first.

[bcardiff]

I second @straight-shoota input. The implementation is GTG. So after the spec is improved based on the other PR this is good to be merged

[asterite]

Rebased against master. I noticed spec/std/http/spec_helper didn't include the general spec/std/spec_helper which contains datapath so I added that too.

[asterite]

Also thank you @omarroth for the original code, which I used almost as is. It's just that this needed a bit more forward pushing from us core team members and having it on our side makes it easier to rebase, edit, etc.

[omarroth]

No worries, thanks @asterite!

[rdp]

I noticed today that determining remote_address "up front" for request makes this benchmark:

https://gist.github.com/jgaskins/ba74aaca0ce45f714caaa7571703beba [1]

2.5% slower.(12500 -> 12225 req/s) single threaded, and about 5% slower multi-threaded. Maybe there's a way to avoid the slowdown?

[1] https://forum.crystal-lang.org/t/multithreaded-crystal-initial-thoughts/1089

[straight-shoota]

What exactly are you comparing? Crystal Vs Go? Different Crystal versions?

[rdp]

Crystal versus crystal with or without this patch applied:

diff --git a/src/http/request.cr b/src/http/request.cr
index f942c33f3..ade289eb1 100644
--- a/src/http/request.cr
+++ b/src/http/request.cr
@@ -110,7 +110,7 @@ class HTTP::Request
       request = new line.method, line.resource, headers, body, line.http_version, internal: nil
 
       if io.responds_to?(:remote_address)
-        request.remote_address = io.remote_address.try &.to_s
+        #request.remote_address = io.remote_address.try &.to_s
       end
 
       return request

First up any "small" HTTP server, here's one: https://gist.github.com/rdp/b136f6a998919fe0eae39dc7990d0ba0
then load test it with wrk: $ wrk http://localhost:9000
Thanks!