Skip to content
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

HTTP::Client and HTTP::Server on Unix Socket #2735

Closed
sdogruyol opened this issue Jun 3, 2016 · 16 comments
Closed

HTTP::Client and HTTP::Server on Unix Socket #2735

sdogruyol opened this issue Jun 3, 2016 · 16 comments

Comments

@sdogruyol
Copy link
Member

Currently HTTP::Server only listens on TCP.

It'd be really great to listen on a Unix socket like Unicorn, Thin, Puma e.g

An API like this

server = HTTP::Server.new("/tmp/server.sock") do |context|
  context.response.content_type = "text/plain"
  context.response.print "Hello world!"
end

server.listen
@sdogruyol
Copy link
Member Author

sdogruyol commented Jun 3, 2016

Preferably i'd like to tackle this issue but seems like https://github.com/crystal-lang/crystal/blob/master/src/socket/unix_server.cr needs to be improved.

I've already managed to make bind https://github.com/crystal-lang/crystal/blob/master/src/http/server/server.cr#L135-L137 to create UNIXServer but then handle_client needs to be updated.

Any tips and leads are welcome 😄

@asterite
Copy link
Member

asterite commented Jun 3, 2016

My only comment is that I wouldn't accept a string for this, in HTTP::Server constructor, as it's confusing with a host String parameter. Maybe use a required named argument, or a UnixServer instance.

@sdogruyol
Copy link
Member Author

@asterite how about?

HTTP::Server.new(socket: "/tmp/server.sock") {|context|}

@jhass
Copy link
Member

jhass commented Jun 3, 2016

How about dropping both host and port and requiring an URI instead?

HTTP::Server.new
HTTP::Server.new("localhost")
HTTP::Server.new("localhost:443")
HTTP::Server.new("[::1]")
HTTP::Server.new("[::1]:443")
HTTP::Server.new("127.0.0.1:443")
HTTP::Server.new("0.0.0.0:443")
HTTP::Server.new("tcp:localhost:443")
HTTP::Server.new("tcp:[::]")
HTTP::Server.new("unix:server.sock") # relative
HTTP::Server.new("unix:/run/server.sock") # absolute

@sdogruyol
Copy link
Member Author

@jhass it sounds like a good idea but i guess it's not common to create a server that way (or is it?) 🤔

@jhass
Copy link
Member

jhass commented Jun 3, 2016

I've seen it in various places, one popular example is unicorn or puma.

@ysbaddaden
Copy link
Contributor

In puma you add listeners to a single server:

server = Puma::Server.new
server.add_tcp_listener(host, port)
server.add_ssl_listener(host, port, ctx)
server.add_unix_listener(path)

@ozra
Copy link
Contributor

ozra commented Jun 4, 2016

Having multiple listeners, both unix-socket and tcp, ssl seems like something one very well might want to do for a service (if not using nanomsg, ZMQ, etc.)!

@sdogruyol
Copy link
Member Author

If there's a final decision for the API, i'd like to tackle this //cc @asterite

@RX14
Copy link
Contributor

RX14 commented Apr 12, 2017

I'm in favour of the HTTP::Server.new(*, socket : String) API for this. It's simple to use, and a minimal API change.

@elorest
Copy link
Contributor

elorest commented May 12, 2017

Is there any active work on this issue?

@mverzilli
Copy link

No, there isn't. I think it got stuck at the design phase with so many variants and no clear winner way forward. I'm with @jhass on this. Sounds like adding more listeners would be a topic for a separate issue, wouldn't it?

@straight-shoota
Copy link
Member

I think it'd be great to accept a single argument with an URI (either as String or URI). This way it's easy to let binding to a port/host or unix socket be configured by an external config setting (command line argument, config file etc.). It does not require an application to interpret such a config value to call different constructors. This is quite useful as it makes life easy and encourages to use a standardized format instead of individual implementations.
This does not change the API, since every signature accepting a String currently also requires an integer for the port - and the port is required in any case.
There could also be a way to directly assign a unix socket (as is the case for port/host) via a named parameter.

@straight-shoota
Copy link
Member

straight-shoota commented Jun 26, 2017

To get this moved on, I would propose the following API (with simplified bodies):

# existing overloads for TCP server with port/port+host:
def self.new(host : String, port : Int32)
  TCPServer.new(host, port)
end
def self.new(port : Int32)
  new("127.0.0.1", port)
end
# new socket overload:
def self.new(*, socket : String)
  UNIXServer.new(socket)
end
# URI overloads:
def self.new(uri : URI)
  if port = uri.port
    if host = uri.host
      new(host, port)
    else
      new(port)
    end
  elsif path = uri.path
    new(socket: path)
  else
    raise "URI requires either port or path"
  end
end
def self.new(uri : String)
  new(URI.parse(uri))
end

However, with 4 variants for each (handlers : Array(HTTP::Handler), &handler : Context ->, &handler : Context ->, handlers : Array(HTTP::Handler), handler : HTTP::Handler | HTTP::Handler::Proc) this would result in a total of 20 overloads for HTTP::Server.new. 🤔 The code could probably be neatly organized using macros, but I fear the API doc might become overwhelming...

A different approach would be to deprecate the current API and only assign handlers in the initializer (i.e. only 4 total overloads) and move the binding target to the listen/bind methods. The current argument reuse_port for these methods only makes sense in the context of a TCP server.
This would also be a step towards the idea of adding multiple listeners to one HTTP server.

@bcardiff
Copy link
Member

Note: this is almost covered by #9543

@bcardiff
Copy link
Member

bcardiff commented Sep 1, 2023

Coming back to this issue, I think we can close it. Both server and client are supported. The API can still be improved on the client but still, is working.

require "http/server"

server = HTTP::Server.new do |context|
  context.response.content_type = "text/plain"
  context.response.print "Hello world!"
end

server.bind_unix "/tmp/my-socket.sock"
puts "Listening"
server.listen
require "http/client"

client = HTTP::Client.new(UNIXSocket.new("/tmp/my-socket.sock"))

response_body = client.get("/test").body
pp! response_body

client.close

Testing the server via curl can be done via

curl --unix-socket /tmp/my-socket.sock http:/test

@bcardiff bcardiff closed this as completed Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests