fpco / streaming-commons

Common lower-level functions needed by various streaming data libraries
MIT License
36 stars 41 forks source link

bindPort{TCP,Gen,GenEx} only accepts numeric hostnames #46

Closed TravisCardwell closed 6 years ago

TravisCardwell commented 6 years ago

A colleague recently discovered that he is unable to use Warp with hostnames (such as "localhost"); it only works with IP addresses.

  1. runSettings calls bindPortTCP (source)
  2. bindPortTCP calls bindPortGen (source)
  3. bindPortGen calls bindPortGenEx (source)
  4. bindPortGenEx sets the NS.AI_NUMERICHOST hint (source), which limits hosts to numeric addresses

Is this a desired limitation? If so, perhaps it should be documented, as none of the above functions have documentation that mentions it. If not, would removing the NS.AI_NUMERICHOST hint be an acceptable fix, or would that have undesired consequences for other uses of the function?

Note that the limitation can be easily worked around by resolving hostnames before configuring the Warp settings.

For anybody who runs across this issue and searches for it, here is the error message:

Network.Socket.getAddrInfo (called with
preferred socket type/protocol: AddrInfo
  { addrFlags = [AI_PASSIVE,AI_NUMERICSERV,AI_NUMERICHOST]
  , addrFamily = AF_UNSPEC
  , addrSocketType = Stream
  , addrProtocol = 0
  , addrAddress = <assumed to be undefined>
  , addrCanonName = <assumed to be undefined>
  },
host name: Just "localhost",
service name: Just "8000"):
does not exist (Name or service not known)
snoyberg commented 6 years ago

That set of hints has been floating through my network code for a very long time, and I can't say I know what negative impacts could come from changing them. The earliest commit I can find with them is:

https://github.com/yesodweb/wai/commit/02c1396c86e3fceb48cbe7df58cb631c804e24d4

@kazu-yamamoto Do you have any thoughts here?

kazu-yamamoto commented 6 years ago

I would suggest addrFlags = [AI_PASSIVE,AI_ADDRCONFIG].

snoyberg commented 6 years ago

I'm fine with such a change. Travis: you want to try this out and, if it works for you, send a PR with the update?

On Wed, Jan 31, 2018 at 9:40 AM, Kazu Yamamoto notifications@github.com wrote:

I would suggest addrFlags = [AI_PASSIVE,AI_ADDRCONFIG].

  • Delete AI_NUMERICHOST to allow hostnames
  • Delete AI_NUMERICSERV to allow portnames
  • Add AI_ADDRCONFIG to ensure to have suitable IP addresses

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/fpco/streaming-commons/issues/46#issuecomment-361849449, or mute the thread https://github.com/notifications/unsubscribe-auth/AADBB2VxJyFuvSRH_OaZyuyFrERkuoWzks5tQBkGgaJpZM4RyU__ .

TravisCardwell commented 6 years ago

Kazu's suggestion sounds good to me. I tested against my use cases, and it works for me. I have submitted the changes as PR #47.

snoyberg commented 6 years ago

Addressed by #47, and uploaded as 0.1.19, thanks!