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

updating the example comments to be usable by curl #5263

Closed
wants to merge 1 commit into from

Conversation

jkthorne
Copy link
Contributor

@jkthorne jkthorne commented Nov 9, 2017

This helps with problems mentioned in issue 5728
#4728

This helps with problems mentioned in issue 5728
crystal-lang#4728
@jhass
Copy link
Member

jhass commented Nov 9, 2017

Mh, I really wonder whether we can't bind to all addresses localhost (or any named host) has.

@jkthorne
Copy link
Contributor Author

Isn't this a problem with some local resolvers?
And this is only a documentation change.

@RX14
Copy link
Contributor

RX14 commented Nov 21, 2017

Binding to localhost should be the same as binding to 127.0.0.1 is ipv4 is enabled and ::1 if ipv6 is enabled and both if both are enabled. Or actually whatever addresses localhost resolves to.

The point is that we shouldn't have to change this documentation: it should work. If it doesn't it's a bug and it should be fixed instead of jsut changing the documentation.

@jkthorne
Copy link
Contributor Author

I like localhost better. If someone closes the issue I will close the PR.

@bcardiff
Copy link
Member

I recall issues on mac where using localhost was not working and 127.0.0.1 was the fix on different products/stacks. Maybe I am used to write whether I want to bind to 0.0.0.0 or to 127.0.0.1 so many times that seems natural to me that an ip is used instead of the localhost word.

If the scenario could be improved for localhost, I would say to create a different issue. Having better documentation for the current state is an improvement.

@RX14
Copy link
Contributor

RX14 commented Nov 21, 2017

127.0.0.1 is an ipv4 addreess. I don't think we should be encouraging solutions that only work for ipv4, not ipv6.

@jkthorne
Copy link
Contributor Author

After more searching I think the curl can be fixed but I am not sure if this is the solution. However the browser does not work for me unless I add the protocol and content-length.

Something like this.

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

@jkthorne
Copy link
Contributor Author

I think more needs to be discussed about this in the issue and what the examples should be showing.

@jkthorne jkthorne closed this Nov 21, 2017
@jkthorne jkthorne deleted the patch-1 branch July 11, 2020 04:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants