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

Fix handling of None and "" as hostnames in trio.socket #277

Closed
njsmith opened this Issue Aug 11, 2017 · 4 comments

Comments

Projects
None yet
1 participant
@njsmith
Member

njsmith commented Aug 11, 2017

It looks like the Python stdlib has a weird behavior:

  • socket.getaddrinfo(None, 0) resolves to the wildcard address(es)
  • sock.bind((None, 0)) is an error
  • socket.getaddrinfo("", 0) is an error
  • sock.bind(("", 0)) binds to the wildcard address

Trio is currently not quite consistent. Trio's getaddrinfo matches the stdlib, but for bind we raise an error with both versions, because we use getaddrinfo to validate the user's input, and then if it passes we hand the user's input directly to the stdlib bind.

We should at the least support one way of calling bind, and perhaps we should teach both versions to support both behaviors.

@njsmith

This comment has been minimized.

Member

njsmith commented Aug 14, 2017

So I guess we probably shouldn't special-case "" in our getaddrinfo implementation, since it technically is a name string you could look up, and it has well-defined behavior in some cases. In particular, on Windows, it returns a list of all currently-configured local interfaces. (Why this is useful, I have no idea. But it's specified to do so.)

So I think we should:

  • Support None everywhere
  • Special case "" in our sockaddr resolver routines (but not getaddrinfo) by replacing it with None.

(Fun fact: on Linux getaddrinfo("*", ...) also resolves to the wildcard addresses, exactly like passing None. But this isn't portable, and None is.)

@njsmith

This comment has been minimized.

Member

njsmith commented Aug 22, 2017

There's another special case in the sockaddr resolution stuff: "<broadcast>"

@njsmith

This comment has been minimized.

Member

njsmith commented Dec 20, 2017

It looks like basically to fix this we just need to add an if address[0] == "": address[0] = None check to _resolve_local_address.

I think "<broadcast>" is not worth worrying about; it's just a hard-coded shorthand for "255.255.255.255" or INADDR_BROADCAST, and it doesn't really do anything useful these days anyway: https://stackoverflow.com/questions/683624/udp-broadcast-on-all-interfaces. Or we could do if address[0] == "<broadcast>": address[0] = "255.255.255.255".

@njsmith

This comment has been minimized.

Member

njsmith commented Dec 21, 2017

Eh, handling <broadcast> was trivial so I went ahead and did it in #383

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment