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

tcpserver documentation instructions can cause confusion. #4728

Open
rdp opened this issue Jul 19, 2017 · 11 comments
Open

tcpserver documentation instructions can cause confusion. #4728

rdp opened this issue Jul 19, 2017 · 11 comments
Labels
good first issue This is an issue suited for newcomers to become aquianted with working on the codebase. help wanted This issue is generally accepted and needs someone to pick it up kind:docs topic:stdlib:networking

Comments

@rdp
Copy link
Contributor

rdp commented Jul 19, 2017

Hello.

The docs for https://crystal-lang.org/api/0.23.0/TCPServer.html have this example:

require "socket"

server = TCPServer.new("localhost", 1234)
loop do
  server.accept do |client|
    message = client.gets
    client << message # echo the message back
  end
end

In OS X if I run this, and fire up my "client of choice" and attempt to connect to localhost:1234 using ipv4, I get an unexpected "connection refused", here's how to repro:

$curl --ipv4 http://localhost:1234
curl: (7) Failed to connect to localhost port 1234: Connection refused

It appears that it's binding to the ipv6 version of localhost, and not the ipv4 version, by default, which is what causes my confusion. The ideal would be to bind to both. If that's tricky, then perhaps change the example to this:

server = TCPServer.new("127.0.0.1", 1234)

just so that people like me, who see localhost and think "127.0.0.1" don't get confused by the example.

Thanks!

@rdp rdp changed the title tcpserver documentation instructions can be misleading. tcpserver documentation instructions can cause confusion. Jul 19, 2017
jkthorne added a commit to jkthorne/crystal that referenced this issue Nov 9, 2017
This helps with problems mentioned in issue 5728
crystal-lang#4728
@ysbaddaden
Copy link
Contributor

This is a mere example. You can't expect your resolver to behave the sale in scenarios where you specify ipv4 specifically, but don't specify it otherwise.

What happens if you remove --ipv4? Does it work? Then the example correct. We could use ::1 which should bind to both ipv4 and ipv6 but then if you don't have an ipv6 it will fail.

IMHO the example is just fine.

@jkthorne
Copy link
Contributor

I don't know if the curl is as much of a problem. I just tried this and the browsers on macOS do not work. That seems like a problem.

@jkthorne
Copy link
Contributor

After more research I think this does not work in the browser because you need to add the protocol and content-length.

require "socket"

server = TCPServer.new("localhost", 1234)
loop do
  server.accept do |client|
    message = client.gets
    client << "HTTP/1.1 200\n"
    client << "Content-Length: #{message.as(String).size}\n\n"
    client << message # echo the message back
  end
end

@straight-shoota
Copy link
Member

straight-shoota commented Nov 21, 2017

This is an example of a simple TCPServer without any application protocol. So it will certainly not work with an HTTP protocol, neither with curl nor a browser, because it returns an invalid HTTP response. (actually, curl doesn't complain and just prints the received contents without interpreting it as an HTTP response)
But of course it should be possible to connect to the socket, this seems to be already failing for curl on MacOS. But I don't think this is an error on Crystal's side.

This tcp server can be tested for example with netcat: echo hello world | netcat localhost 1234
Maybe something like this could be added to the example. But I think people who want to build a TCP server should be familiar with this.

@jkthorne
Copy link
Contributor

I was not considering that when I ran across this. I think this issue might be able to be closed. I am not sure since @rdp has not responded.

@rdp
Copy link
Contributor Author

rdp commented Dec 9, 2017

I believe the problem I ran into was "first up your crystal server, using the example, add some HTTP header responses, now in a console, since it's localhost, try to do $ curl http://127.0.0.1:port"
except that doesn't work, since I'm using 127.0.0.1 in that case, not "localhost" which is ipv4 only, causing confusion. Perhaps what "should" happen is that if you bind to localhost, it should bind to "all localhosts" (i.e. both ipv4 and ipv6 localhost), which would be a separate issue and this could be closed...thoughts?

@rdp
Copy link
Contributor Author

rdp commented Nov 18, 2019

I'd think binding to "localhost" would bind to both the ipv4 and ipv6 versions of localhost (ruby seems to have this behavior, but oddly OS X but not linux, weird). Crystal's TCPServer.new(8000) constructor in OS X doesn't bind to IPv4, though the docs say it binds to "all" interfaces?

In linux crystal's TCPServer.new(8000) binds to both ipv6 and ipv4, which is how I'd expect it to work.

netcat on OS X (from homebrew) with crystal server example gives netcat -v localhost 8000 localhost [127.0.0.1] 8000 (irdmi): Connection refused FWIW...

Cheers!

@straight-shoota
Copy link
Member

straight-shoota commented Nov 19, 2019

localhost is a hostname and resolves to whatever is configured on your system. Crystal doesn't have any influence on that. Technically we could add an internal override for this special case, but I don't think that makes sense.

But it would probably be helpful to replace localhost by the loopback address "127.0.0.1" directly, so the example should work regardless of hostname resolution. Alternatively, we could also use ::1 as suggested above by @ysbaddaden as it should bind to both IPv6 and IPv4 loopbacks.
AFAIK all operating systems Crystal runs on have had IPv6 support for a long time. And the loopback address shouldn't require any hardware support. So unless your executing the example on a 20 year old OS, this should not really be an issue.

@rdp
Copy link
Contributor Author

rdp commented Nov 20, 2019

Yeah 127.0.0.1 would clear things up. I think preferably if you bind to "localhost" it should either bind to "ipv4 over ipv6" (if both are available) or bind to both, to increase compatibility with older programs (like some old versions of netcat). The host-less option TCPServer.new(1234) does seem to bind to both, so that's working well. So another option would be to change it to hostless.

Is that worth filing a separate issue for "ipv4 default instead of ipv6", thoughts?
Thanks.

@jhass jhass added good first issue This is an issue suited for newcomers to become aquianted with working on the codebase. kind:docs labels Jan 27, 2020
@straight-shoota straight-shoota added the help wanted This issue is generally accepted and needs someone to pick it up label Jul 2, 2021
@simon1tan1
Copy link

Just want to note that this affects Linux as well (Ubuntu). netstat shows:
tcp6 0 0 ::1:1234 :::* LISTEN 75472/./tcpserver off (0.00/0/0)

Apparently, ipv6 is the default for "localhost" so Crystal is not wrong here.
https://unix.stackexchange.com/questions/530976/why-does-localhost-resolve-to-1-but-not-127-0-0-1

Let's just add a note to the docs and close this issue.

simon1tan1 added a commit to simon1tan1/crystal that referenced this issue Jul 14, 2022
"localhost" defaults to ipv6. resolve crystal-lang#4728
@BlobCodes
Copy link
Contributor

But it would probably be helpful to replace localhost by the loopback address "127.0.0.1" directly, so the example should work regardless of hostname resolution. Alternatively, we could also use ::1 as suggested above by @ ysbaddaden as it should bind to both IPv6 and IPv4 loopbacks.

Binding ::1 does not bind the IPv4 loopback 127.0.0.1 (and the other way around), so that isn't a good idea. If you'd use a program primarily using IPv6, you would get a Connection refused error.

If you want to bind all addresses, there's 0.0.0.0 (all IPv4 addresses) and :: (all addresses).
But the docs already include this:

Options:

  • host local interface to bind on, or :: to bind on all local interfaces.

Apparently, ipv6 is the default for "localhost" so Crystal is not wrong here.
https://unix.stackexchange.com/questions/530976/why-does-localhost-resolve-to-1-but-not-127-0-0-1

Let's just add a note to the docs and close this issue.

This is not the case for all platforms. MSDN docs for getaddrinfo:

If the pNodeName parameter points to a string equal to "localhost", all loopback addresses on the local computer are returned.

Because of this, I think this solution would be bad.


I think the best option would be to either use the host-less option TCPServer.new(1234) or use the IPv6 unspecified address TCPServer.new("::", 1234). That should be enough to close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue This is an issue suited for newcomers to become aquianted with working on the codebase. help wanted This issue is generally accepted and needs someone to pick it up kind:docs topic:stdlib:networking
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants