Skip to content

TCPServer.new takes a nil host and a String port - #3274

Open
hasannadeem wants to merge 1 commit into
ruby:masterfrom
hasannadeem:tcp-server-new-port
Open

hasannadeem wants to merge 1 commit into
ruby:masterfrom
hasannadeem:tcp-server-new-port

Conversation

@hasannadeem

Copy link
Copy Markdown

Fixes #1671.

TCPServer.new([hostname,] port) passes both arguments to getaddrinfo, so the host can be nil and the port can be a String, either a number or a service name:

TCPServer.new(0)                    # OK with the current signature
TCPServer.new("0")                  # rejected: String port
TCPServer.new("127.0.0.1", "8080")  # rejected: String port
TCPServer.new(nil, 0)               # rejected: nil host

All four work in Ruby, but the signature was (?String host, Integer port) -> untyped. This changes it to (?String? host, Integer | String port) -> void, and adds TCPServerSingletonTest#test_new for each of these calls. With the old signature the test fails at the first String port ("Call trace does not match one of method definitions"). With the new one it passes, and so do the other socket tests and Net_HTTP_test.rb. rbs validate and rubocop pass too (Ruby 3.4.4).

I kept to the common types and left out some rarer ones Ruby also accepts: an Integer IPv4 address as the host, to_str objects, and a nil port. A to_int port is rejected by Ruby (no implicit conversion of Object into String), so ::int would be wrong for the port.

The rest of the socket signatures, such as TCPSocket#initialize and Socket.tcp_server_loop, also type ports as Integer only. I haven't changed those here, to keep this to the method in the issue.

TCPServer.new([hostname,] port) passes both arguments to getaddrinfo,
so the host can be nil and the port can be a String, either a number
or a service name. The signature only allowed a String host and an
Integer port, so calls such as TCPServer.new(nil, 0) and
TCPServer.new("127.0.0.1", "8080") didn't type-check.

Allow String? for the host and Integer | String for the port, return
void from initialize, and add TCPServerSingletonTest#test_new.

Fixes ruby#1671
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.

TCPServer.new type signature incorrect

1 participant