feature(socket.tcp): enhance the logic of parameter verification in

[rainingmaster]

In ngx.socket.tcp:connect, if host is a TCP socket, the third parameter could be nil, it is to some third part module to write their code like this:

function _M.connect(host, port, opt)
    sock = ngx.socket.tcp()
    sock:connect(host, port, opt)
end

But if the host is a unix-domain socket, the second parameter could not be a nil, we have to write the code like this:

function _M.connect(path, opt)
    sock = ngx.socket.tcp()
    if opt ~= nil then
        sock:connect("unix:" .. path, opt)
    else
       sock:connect("unix:" .. path)
   end
end

Just like here: https://github.com/openresty/lua-resty-redis/pull/200/files#diff-adb515948c05651051c83fd6fb3ce7ceR143-R149. I think it is a comment use way, so update the second parameter and allow it be nil.

[agentzh]

@rainingmaster Please use feature: prefix in the commit log titles. Thank you!

[rainingmaster]

Hi @agentzh, I have update the code, make the port optional and accept nil, could you help me review again?

[mergify]

This pull request is now in conflict :(

[agentzh]

I don't like this behavior. This may lead to hard-to-debug mistakes. When the host name part is an INET address, the port cannot be optional.

[rainingmaster]

I see, I agree with this point. So we should allow like this:

sock:connect("unix:/path") -- work
sock:connect("unix:/path", nil) -- work
sock:connect("unix:/path", opt) -- work

sock:connect("127.0.0.1", 80) -- work
sock:connect("127.0.0.1", 80, nil) -- work
sock:connect("127.0.0.1", 80, opt) -- work

But like this will be raise an error:

sock:connect("127.0.0.1", nil) -- error

In this version, the behaviour should be like above: #1735 (comment), but we have to judge the domain is an unix domain or a ip.

[agentzh]

@rainingmaster My original comment you quoted was for the unix domain socket case, not for the INET case.

[rainingmaster]

@agentzh hihi, my quoted should be mean not unix domain socket? so it should mean INET case.

[rainingmaster]

@agentzh @doujiang24 @spacewander Hi, I have raise a new version for this concern, now it will require that the second parameter must be passed in if the host is not unix domain.

Another point to note is that the unix domain has always supported usage like this:

connect("unix/path", nil, opts)

IMO, I think this usage should be retained, because users can easily use the format of connect(host, port, opts) to pass parameters when implementing their own lib, without the need for more if.