-
Notifications
You must be signed in to change notification settings - Fork 189
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
Socket endpoints, in particular reading them from a String #464
base: master
Are you sure you want to change the base?
Conversation
First minor things:
|
Major things: From test examples, the essential port is parsing strings such as "host:port". Personally, I support to add such a parser to combine However, I don't want to add a new intermediate data type, I'm also the maintainer of |
Sorry, I will do this, this is just an automatic habit for me. My understanding is that newer Haskell code prefers
I'm not sure what you mean by "combine". In this PR,
Taking a look at https://hackage.haskell.org/package/network-2.6.0.0/docs/src/Network.html#PortID - it seems that this was an inappropriate abstraction, combing a hostless portnumber with a unix domain socket path. I agree this doesn't make much sense. However, a previous bad experience with an inappropriate abstraction, should not prejudice you against a better abstraction.
My motivation is to develop appropriate abstractions. With a single function |
Done in 65b9b0b. BTW, some other uses of |
Done in 1c11e49, it requires moving the aliases from Info into Types. Since PortNumber is already in Types, I think the new location is fine (and even better). |
Than you for pointing out. I have fixed them. |
Maybe in version 4 (or 3.2). I think that we will have version 4 for #364. We are planning to release 3.1.2 with the current |
If we have:
we can easily pass the result to |
@infinity0 What is your endpoint based on? RFC3986? |
The concept fits parts of the ideas in that RFC, but it originates more directly from my years of experience programming a wide variety of real-world networking programs - client/server, different types of proxies, peer-to-peer nodes, etc etc, typically with strong security requirements. I've seen many examples of this type of low-level networking code that does essentially the same thing but written slightly differently using whatever language's API that mirrors the C API, and they all support slightly different things because everyone solves their own problem in their own specific non-general way. Then you have to file a specific PR to add unix socket support, or another specific PR to add support for localhost, or another specific PR to add support for domain names, or another specific PR to support arbitrary port numbers, or another PR to support IPv6, etc etc etc. This all stems from the fact that the C API exposes the wrong abstractions (for modern usage, at least) - it exposes internal implementations, rather than thinking about the external abstractions. To put it simply, there are two sources of information in opening a network link - (1) user-supplied info i.e. the address, which can vary arbitrarily, and (2) programmer-supplied parameters, which the user cannot vary. (It is highly unusual for a user to decide whether they want to run something as TCP or UDP for example).
So if a programmer uses [*] OK, there is one minor thing missing, it doesn't support |
…n of them Useful for 3rd-party networking applications that might want to pass around service specifiers without worrying whether these are IP addresses, DNS names, or UNIX-domain socket paths. Previously, there was no data type to encapsulate these options together. In particular, getAddrInfo had to be used to resolve DNS names into a SockAddr before calling connect/bind, but it could not deal with UNIX domain sockets. The new function sockNameToAddr takes this role, transparently converting DNS names and passing through non-DNS-names unaltered, so that it can be used uniformly without worrying about the specific type of input name/address.
This comes from the main part of #428, except I've made the following changes:
I did not implement the read functions as the
Read
instance because it doesn't follow the standardRead
contract. [*]In #428 the comments were that the code belongs in a more "high-level" library, however I hope the test cases & usage examples help to convince you that it is better in this library. I can't justify creating a separate package on hackage just for these few extra functions, and they would be out-of-place in a more specialised library (e.g. a http / web / p2p library).
The main entry point is
socketFromEndpoint
, which is meant as a convenience utility, replacing the need to call combinations of 3-4 other functions - similar to howopenSocket
was added to this library in the meantime (5b09871, 2 weeks ago). In my opinion, saying this is "too high-level" is similar to sayingopenSocket
is "too high-level".readSockEndpoint
is useful for CLI parsers or other UI-related things - the user can specify an endpoint easily as a string, and you can pass it to some CLI framework likeoptparse-applicative
and have the validation & parsing done automatically, your code can then directly work with aSockEndpoint
object.[*] I have another commit, not part of this PR, that adds support for reading
SockAddr
as well, and then has separatederiving (Show, Read)
instances, which I can file afterwards if this PR is accepted.