[zeromq-dev] Binding to TCP port 0
Pierre Ynard
I want to bind a zeromq socket to an available TCP port assigned by the
OS. However the standard way of doing this, binding to port 0, fails
with EINVAL. Indeed, there is a check in the code saying that 0 is not a
valid port. Is there any reason for this? Is there another way of doing
this?
Also it would be useful to have a way to fetch the port number that was
assigned, although personally I can live without it.
Regards,
--
Pierre Ynard
"Une âme dans un corps, c'est comme un dessin sur une feuille de papier."
_______________________________________________
zeromq-dev mailing list
zerom...@lists.zeromq.org
http://lists.zeromq.org/mailman/listinfo/zeromq-dev
Pieter Hintjens
> I want to bind a zeromq socket to an available TCP port assigned by the
> OS. However the standard way of doing this, binding to port 0, fails
> with EINVAL. Indeed, there is a check in the code saying that 0 is not a
> valid port. Is there any reason for this? Is there another way of doing
> this?
If you're using the high level C binding this works. It's not a core
libzmq functionality. Some other bindings may also provide it.
You can check the CZMQ code that does this:
https://github.com/zeromq/czmq/blob/master/src/zsocket.c
-Pieter
Pierre Ynard
> libzmq functionality. Some other bindings may also provide it.
>
> You can check the CZMQ code that does this:
> https://github.com/zeromq/czmq/blob/master/src/zsocket.c
Thanks, but I don't really want to write an ugly for loop to (badly)
reinvent a feature of the socket API. Also this code ignores the OS
settings for local port assignation.
Why is it disabled in libzmq?
--
Pierre Ynard
"Une âme dans un corps, c'est comme un dessin sur une feuille de papier."
Pieter Hintjens
> Thanks, but I don't really want to write an ugly for loop to (badly)
> reinvent a feature of the socket API. Also this code ignores the OS
> settings for local port assignation.
>
> Why is it disabled in libzmq?
No good reason I can see. If you want it, make the change and send a
pull request to libzmq.
-Pieter
Martin Lucina
On Wed, 25 Jan 2012 18:22:55 +0100
Pierre Ynard <link...@yahoo.fr> wrote:
> > If you're using the high level C binding this works. It's not a core
> > libzmq functionality. Some other bindings may also provide it.
> >
> > You can check the CZMQ code that does this:
> > https://github.com/zeromq/czmq/blob/master/src/zsocket.c
>
> Thanks, but I don't really want to write an ugly for loop to (badly)
> reinvent a feature of the socket API. Also this code ignores the OS
> settings for local port assignation.
+1, I have proposed this functionality before, but we could not figure
out a way to make the API work. Its obvious that many people need
this, and have implemented it with the same hack as czmq.
> Why is it disabled in libzmq?
It is disabled because in ZeroMQ, by design, there is no way to work
with individual underlying connections of a socket. Hence, there is no
API equivalent to e.g. getsockname() which you could use to get the port
back from the OS.
If you can think of a way to make this work, without exposing the
details of the individual connections to the application, and
without a backward-incompatible API change, please propose it (or
even better, make a patch).
-mato
--
Martin Lucina <mar...@lucina.net>
Pieter Hintjens
> It is disabled because in ZeroMQ, by design, there is no way to work
> with individual underlying connections of a socket. Hence, there is no
> API equivalent to e.g. getsockname() which you could use to get the port
> back from the OS.
The functionality regards binding not connections, this isn't the concern.
> If you can think of a way to make this work, without exposing the
> details of the individual connections to the application, and
> without a backward-incompatible API change, please propose it (or
> even better, make a patch).
Current working code would not break by returning the port number via
the zmq_bind API.
The other solution, if this is too ugly (which is debatable) is via a
zmq_getsockopt call. That was your suggestion previously, iirc. Both
are worth exploring.
I don't recall any valid technical concerns with this change proposal,
it came down to opinion.
-Pieter
Martin Lucina
Pieter Hintjens <p...@imatix.com> wrote:
> On Wed, Jan 25, 2012 at 6:19 PM, Martin Lucina <mar...@lucina.net> wrote:
>
> > It is disabled because in ZeroMQ, by design, there is no way to work
> > with individual underlying connections of a socket. Hence, there is no
> > API equivalent to e.g. getsockname() which you could use to get the port
> > back from the OS.
>
> The functionality regards binding not connections, this isn't the concern.
s/connections/endpoints/.
> > If you can think of a way to make this work, without exposing the
> > details of the individual connections to the application, and
> > without a backward-incompatible API change, please propose it (or
> > even better, make a patch).
>
> Current working code would not break by returning the port number via
> the zmq_bind API.
How? zmq_bind() does not have an out parameter. You mean abusing the
(int) return value of zmq_bind() to return a port number? That's
horrible and not all transports have such a thing as a port number,
so it's not generic either.
> The other solution, if this is too ugly (which is debatable) is via a
> zmq_getsockopt call. That was your suggestion previously, iirc. Both
> are worth exploring.
>
> I don't recall any valid technical concerns with this change proposal,
> it came down to opinion.
The major technical problem which everyone (including me) forgets every
time this comes up is that you can bind a single ZeroMQ socket to
multiple endpoints. Cf. what I said above, some of the bound endpoints
may not even have such a thing as a "port number".
To create an API for this you basically end up with some way to
enumerate the bound endpoints and get them to the application. And at
that point we're coming dangerously close to letting people enumerate
connected endpoints, etc.
-mato
--
Martin Lucina <mar...@lucina.net>
Pieter Hintjens
> How? zmq_bind() does not have an out parameter. You mean abusing the
> (int) return value of zmq_bind() to return a port number? That's
> horrible and not all transports have such a thing as a port number,
> so it's not generic either.
Yes, it's horrid, I didn't claim it was neat :-) But it is technically
compatible since it will not break current working code.
> The major technical problem which everyone (including me) forgets every
> time this comes up is that you can bind a single ZeroMQ socket to
> multiple endpoints. Cf. what I said above, some of the bound endpoints
> may not even have such a thing as a "port number".
Speak for yourself, I did not forget about this :) Binds, as I said
deliberately, are synchronous. That means you can bind and then
immediately ask for the port number.
Further, 95% of applications bind exactly one time. And further to
that, it is an extreme edge case (one we can ignore IMO) to bind twice
to system-provided ports.
> To create an API for this you basically end up with some way to
> enumerate the bound endpoints and get them to the application. And at
> that point we're coming dangerously close to letting people enumerate
> connected endpoints, etc.
Please read Martin's excellent posting about solving the right
problems only and not the full range of theoretical problems. It is
amazing how even experienced engineers can focus so intensely on
solving, perfectly, the wrong problem.
No-one is asking for an API that lets you bind 10 times to port zero
and get all ten results. 100% of use cases here are for binding once
to port zero and getting exactly one result back.
It is bad design process to exaggerate the user's requirements into
absurd extremity and then state there is no simple solution. It's far
wiser to understand the actual problem people are trying to solve, and
solve that simply and minimally.
The elegant and minimal solution is of course to return the system
assigned port via a zmq_getsockopt and I'd make that patch except it's
evening here in Dallas and there's a bar with barely-clad waitresses
that has a cold beer waiting for me.
Cheers
Pieter
john skaller
On 26/01/2012, at 12:28 PM, Pieter Hintjens wrote:
> The elegant and minimal solution is of course to return the system
> assigned port via a zmq_getsockopt and I'd make that patch except it's
> evening here in Dallas and there's a bar with barely-clad waitresses
> that has a cold beer waiting for me.
Later in the nigh there'll be a beer with bearly clad waitresses
surfing in it
--
john skaller
ska...@users.sourceforge.net
Martin Lucina
Pieter Hintjens <p...@imatix.com> wrote:
> No-one is asking for an API that lets you bind 10 times to port zero
> and get all ten results. 100% of use cases here are for binding once
> to port zero and getting exactly one result back.
Indeed. In fact, the OP was specifically asking for a use case where he
could bind an unspecified number of times to port zero, and get zero
results back. :-)
> It is bad design process to exaggerate the user's requirements into
> absurd extremity and then state there is no simple solution. It's far
> wiser to understand the actual problem people are trying to solve, and
> solve that simply and minimally.
Given that an API will stick around for a long time, it is good API
design process to extrapolate current and possible future use cases
and arrive at the simplest possible API covering all these use cases.
--
Martin Lucina <mar...@lucina.net>
Pieter Hintjens
> Indeed. In fact, the OP was specifically asking for a use case where he
> could bind an unspecified number of times to port zero, and get zero
> results back. :-)
Pierre may clarify whether "unspecified" is 1, or N > 1.
> Given that an API will stick around for a long time, it is good API
> design process to extrapolate current and possible future use cases
> and arrive at the simplest possible API covering all these use cases.
That road leads to AMQP/1.0. Solve the core majority problem
minimally, period. It is terrible API design (indeed, design in
general) to comingle abstract philosophically potential problems with
real ones and end up with incredibly complex solutions of which people
only use 10%.
Even if (some) users claim to require N, which is unproven here and
IMO inaccurate, it would be necessary to measure the cost of solving 1
vs. the cost of solving N, and measure the value to all users of that
solution. Often solving N is simpler than solving 1. In this case,
it's not. Solving N would satisfy 0.01% of users and make a solution
horrible to 99.9% of users. Quod Erat Demonstrandum.
If anyone on this list needs to bind one socket multiple times to port
0, please do raise a hand.
Not to mention the "binds are synchronous" statement that you seem to
be ignoring in the pursuit of complexity.
-Pieter
Gleb Peregud
> If anyone on this list needs to bind one socket multiple times to port
> 0, please do raise a hand.
I don't, but I have an idea. Not sure if this was already proposed,
but what about allowing to "upgrade" TCP socket to ZeroMQ socket? I.e.
allow someone to bind a socket in a way he want and then "import" it
into ZeroMQ socket as one of the connections
Ian Barber
> It is bad design process to exaggerate the user's requirements into> absurd extremity and then state there is no simple solution. It's farGiven that an API will stick around for a long time, it is good API
> wiser to understand the actual problem people are trying to solve, and
> solve that simply and minimally.
design process to extrapolate current and possible future use cases
and arrive at the simplest possible API covering all these use cases.
Is it possible that this API should simply be on a different calls, a zmq_bind_assign or similar, to avoid this clash? This could then work with all transports:
tcp/pgm: bind to random port
ipc: bind to random free name in system tmp
inproc: bind to random free name
That can either have a return parameter which gives you the bound address, or it can be passed back via a second argument or similar.
Ian
Pierre Ynard
> > he could bind an unspecified number of times to port zero, and get
> > zero results back. :-)
>
> Pierre may clarify whether "unspecified" is 1, or N > 1.
In my case I would only need to bind the socket to one port.
Regards,
--
Pierre Ynard
"Une âme dans un corps, c'est comme un dessin sur une feuille de papier."
Pieter Hintjens
> Is it possible that this API should simply be on a different calls, a
> zmq_bind_assign or similar, to avoid this clash? This could then work with
> all transports:
>
> tcp/pgm: bind to random port
> ipc: bind to random free name in system tmp
> inproc: bind to random free name
That would work, but so would a less surprising zmq_bind plus a
getsockopt with return value depending on transport type.
-Pieter
AJ Lewis
> On Thu, Jan 26, 2012 at 2:17 AM, Ian Barber <ian.b...@gmail.com> wrote:
>
> > Is it possible that this API should simply be on a different calls, a
> > zmq_bind_assign or similar, to avoid this clash? This could then work with
> > all transports:
> >
> > tcp/pgm: bind to random port
> > ipc: bind to random free name in system tmp
> > inproc: bind to random free name
>
> That would work, but so would a less surprising zmq_bind plus a
> getsockopt with return value depending on transport type.
Forgive me if I'm being dense, but couldn't the N case work if you made
sure to do the getsockopt() call after each zmq_bind()?
zmq_bind(foo, "tcp://XXXX:0")
zmq_getsockopt(foo, GET_LAST_BOUND_PORT, &port1, sizeof(port1))
zmq_bind(foo, "tcp://YYYY:0")
zmq_getsockopt(foo, GET_LAST_BOUND_PORT, &port2, sizeof(port2))
--
AJ Lewis
Software Engineer
Quantum Corporation
Work: 651 688-4346
----------------------------------------------------------------------
The information contained in this transmission may be confidential. Any disclosure, copying, or further distribution of confidential information is not permitted unless such privilege is explicitly granted in writing by Quantum. Quantum reserves the right to have electronic communications, including email and attachments, sent across its networks filtered through anti virus and spam software programs and retain such messages in order to comply with applicable data security and retention requirements. Quantum is not responsible for the proper and complete transmission of the substance of this communication or for any delay in its receipt.
Martin Lucina
> On Thu, Jan 26, 2012 at 07:53:06AM -0600, Pieter Hintjens wrote:
> > On Thu, Jan 26, 2012 at 2:17 AM, Ian Barber <ian.b...@gmail.com> wrote:
> >
> > > Is it possible that this API should simply be on a different calls, a
> > > zmq_bind_assign or similar, to avoid this clash? This could then work with
> > > all transports:
> > >
> > > tcp/pgm: bind to random port
> > > ipc: bind to random free name in system tmp
> > > inproc: bind to random free name
> >
> > That would work, but so would a less surprising zmq_bind plus a
> > getsockopt with return value depending on transport type.
>
> Forgive me if I'm being dense, but couldn't the N case work if you made
> sure to do the getsockopt() call after each zmq_bind()?
>
> zmq_bind(foo, "tcp://XXXX:0")
> zmq_getsockopt(foo, GET_LAST_BOUND_PORT, &port1, sizeof(port1))
> zmq_bind(foo, "tcp://YYYY:0")
> zmq_getsockopt(foo, GET_LAST_BOUND_PORT, &port2, sizeof(port2))
Yup, that would work. Sorry for missing the point about zmq_bind() being
synchronous.
Ian also made a great point that this can be extended to "bind to unnamed
endpoint" (for want of a better name) for all transports. So, what I think
we want is that the getsockopt API should return a string (so as to be
usable for multiple transports, plus returning a
different-data-type-per-transport is a PITA).
It'd also be nice to define a consistent way to specify this "unnamed
endpoint" for all transports that might want to provide such functionality.
":0" happens to work for the tcp:// case, but does not really make sense
for inproc:// or ipc://.
This leaves us with something like this proposal:
zmq_bind(foo, "tcp://XXXX:*"); // "tcp://*:*" if you want INADDR_ANY
char endpoint [ZMQ_ENDPOINT_MAX];
zmq_getsockopt(foo, ZMQ_GET_ENDPOINT, endpoint, sizeof endpoint);
=> endpoint is filled as "tcp://XXXX:12345".
zmq_bind(foo, "ipc://*");
char endpoint [ZMQ_ENDPOINT_MAX];
zmq_getsockopt(foo, ZMQ_GET_ENDPOINT, endpoint, sizeof endpoint);
=> endpoint is filled as "ipc:///tmp/Xyz358hfA7".
inproc:// semantics would be identical to ipc:// (w/o the /tmp/ prefix
obviously).
The use of "*" seems fairly uncontroversial -- note that this means an
ipc:// endpoint cannot therefore contain "*" which is an (albeit niche)
backward-incompatible change.
Thoughts? I'll ping Martin Sustrik tomorrow to see if he thinks there's any
reason why this wouldn't work; I believe he's ignoring this thread as TL;DR
:-)
-mato
AJ Lewis
I think the "*" is fine, and it makes sense to me to extend it to all
endpoint types. I'm a bit concerned about having the full
${transport}://${path/port} string passed back. That means that the app
needs to parse that string out to get what it needs, when what it really
wants is just the ${path/port} depending on the transport. Is there a
reason to include more than the part of the transport string that was
wildcarded?
> Thoughts? I'll ping Martin Sustrik tomorrow to see if he thinks
> there's any reason why this wouldn't work; I believe he's ignoring
> this thread as TL;DR :-)
Thanks,
--
AJ Lewis
Software Engineer
Quantum Corporation
Work: 651 688-4346
----------------------------------------------------------------------
The information contained in this transmission may be confidential. Any disclosure, copying, or further distribution of confidential information is not permitted unless such privilege is explicitly granted in writing by Quantum. Quantum reserves the right to have electronic communications, including email and attachments, sent across its networks filtered through anti virus and spam software programs and retain such messages in order to comply with applicable data security and retention requirements. Quantum is not responsible for the proper and complete transmission of the substance of this communication or for any delay in its receipt.
Robert Kern
> On Thu, Jan 26, 2012 at 03:57:37PM +0100, Martin Lucina wrote:
>> This leaves us with something like this proposal:
>>
>> zmq_bind(foo, "tcp://XXXX:*"); // "tcp://*:*" if you want
>> INADDR_ANY char endpoint [ZMQ_ENDPOINT_MAX]; zmq_getsockopt(foo,
>> ZMQ_GET_ENDPOINT, endpoint, sizeof endpoint);
>>
>> => endpoint is filled as "tcp://XXXX:12345".
>>
>> zmq_bind(foo, "ipc://*"); char endpoint [ZMQ_ENDPOINT_MAX];
>> zmq_getsockopt(foo, ZMQ_GET_ENDPOINT, endpoint, sizeof endpoint);
>>
>> => endpoint is filled as "ipc:///tmp/Xyz358hfA7".
>>
>> inproc:// semantics would be identical to ipc:// (w/o the /tmp/ prefix
>> obviously).
>>
>> The use of "*" seems fairly uncontroversial -- note that this means an
>> ipc:// endpoint cannot therefore contain "*" which is an (albeit
>> niche) backward-incompatible change.
>
> I think the "*" is fine, and it makes sense to me to extend it to all
> endpoint types. I'm a bit concerned about having the full
> ${transport}://${path/port} string passed back. That means that the app
> needs to parse that string out to get what it needs, when what it really
> wants is just the ${path/port} depending on the transport. Is there a
> reason to include more than the part of the transport string that was
> wildcarded?
The immediate use case I can think of is to make an ad hoc channel for
communicating with a particular client and communicating the new endpoint to the
client via an already established channel. While that my code (and possibly the
client) already "knows" the transport and the hostname and just needs to know
the port, what it really wants to communicate is the endpoint that the client
should connect to. If I just get the port number, then I still need to
reconstruct the whole endpoint string in order to connect to the new bound
socket. I think this is universal. Every time you bind a socket, you want
something connect to it, and that something needs the full endpoint address in
order to connect.
--
Robert Kern
"I have come to believe that the whole world is an enigma, a harmless enigma
that is made terrible by our own mad attempt to interpret it as though it had
an underlying truth."
-- Umberto Eco
Pieter Hintjens
> Forgive me if I'm being dense, but couldn't the N case work if you made
> sure to do the getsockopt() call after each zmq_bind()?
>
> zmq_bind(foo, "tcp://XXXX:0")
> zmq_getsockopt(foo, GET_LAST_BOUND_PORT, &port1, sizeof(port1))
> zmq_bind(foo, "tcp://YYYY:0")
> zmq_getsockopt(foo, GET_LAST_BOUND_PORT, &port2, sizeof(port2))
You're not being dense and this is exactly what I meant by "bind is
synchronous", unlike a connect. The code example makes it easier to
understand. Thanks!
-Pieter
Pieter Hintjens
> zmq_bind(foo, "tcp://XXXX:*"); // "tcp://*:*" if you want INADDR_ANY
> zmq_bind(foo, "ipc://*");
> The use of "*" seems fairly uncontroversial -- note that this means an
> ipc:// endpoint cannot therefore contain "*" which is an (albeit niche)
> backward-incompatible change.
This is also what we did for CZMQ. Consistent and unsurprising.
+1
-Pieter
Ian Barber
zmq_bind(foo, "tcp://XXXX:*"); // "tcp://*:*" if you want INADDR_ANY
char endpoint [ZMQ_ENDPOINT_MAX];
zmq_getsockopt(foo, ZMQ_GET_ENDPOINT, endpoint, sizeof endpoint);
=> endpoint is filled as "tcp://XXXX:12345".
zmq_bind(foo, "ipc://*");
char endpoint [ZMQ_ENDPOINT_MAX];
zmq_getsockopt(foo, ZMQ_GET_ENDPOINT, endpoint, sizeof endpoint);
=> endpoint is filled as "ipc:///tmp/Xyz358hfA7".
Nice, +1 from me!
Ian
AJ Lewis
Hrm...what I was thinking of was the initial establishment of a
connection. For TCP for example, this could be via a portmapper or
zeroconf, in which case all you care about is the port number. I could
see the full string being useful in the pre-established connection case,
or for inproc, where the binding thread creates the worker threads and
simply passes the inproc string on.
I guess having that be the standard and either having a helper function
in zmq proper, or in the language bindings, to do the parsing out of the
wildcard part probably makes the most sense.
Regards,
--
AJ Lewis
Software Engineer
Quantum Corporation
Work: 651 688-4346
----------------------------------------------------------------------
The information contained in this transmission may be confidential. Any disclosure, copying, or further distribution of confidential information is not permitted unless such privilege is explicitly granted in writing by Quantum. Quantum reserves the right to have electronic communications, including email and attachments, sent across its networks filtered through anti virus and spam software programs and retain such messages in order to comply with applicable data security and retention requirements. Quantum is not responsible for the proper and complete transmission of the substance of this communication or for any delay in its receipt.
Pieter Hintjens
> This leaves us with something like this proposal:
> => endpoint is filled as "tcp://XXXX:12345".
Missed this. It won't work. Bind interfaces are not the same as
connect addresses. The caller needs to know the port number, it can
then reconstruct a full address and broadcast that to peers, e.g. via
a central broker or out-of-band UDP (we do this in several
applications using CZMQ). Returning the bound interface + port just
creates extra parsing work for applications.
Further, you need to support "*" in the bind address, just treat ":*"
at the end as special. Otherwise you break the very common and
necessary use case of "bind to all interfaces".
-Pieter
gonzalo diethelm
Perhaps you can have a getsockopt that returns just the "port" bound, and another that returns the full endpoint.
--
Gonzalo Diethelm
DCV Chile
Pieter Hintjens
> Perhaps you can have a getsockopt that returns just the "port" bound, and another that returns the full endpoint.
Yay for over-design. :-/
How about a minimal plausible solution for binding to dynamic TCP
ports, getting that working, and improving it over time?
-Pieter
Martin Sustrik
> zmq_bind(foo, "tcp://XXXX:*"); // "tcp://*:*" if you want INADDR_ANY
> char endpoint [ZMQ_ENDPOINT_MAX];
> zmq_getsockopt(foo, ZMQ_GET_ENDPOINT, endpoint, sizeof endpoint);
>
> => endpoint is filled as "tcp://XXXX:12345".
Note that out of the two wildcards, only one is resolved in synchronous
way -- the port. The interface is resolved asynchronously when actual
connection is made from the peer.
Martin
Ian Barber
zmq_bind(foo, "tcp://XXXX:*"); // "tcp://*:*" if you want INADDR_ANY
char endpoint [ZMQ_ENDPOINT_MAX];
zmq_getsockopt(foo, ZMQ_GET_ENDPOINT, endpoint, sizeof endpoint);
Hi all,
I have knocked up a relatively simple patch to implement this on IPC and TCP transports, would appreciate comments as I'm pretty sure there's some dodgy stuff in there! I've made a pull req, though mainly for comment (please don't pull it chuck/mikko/pieter!): https://github.com/zeromq/libzmq/pull/238
The TCP is handled by binding to port 0 then using getsockname to retrieve the details in tcp_listener. This has an added get_address function which is called from socket_base to populate a field in options. That field is ZMQ_LAST_ENDPOINT just to make very explicit that it will get overwritten with subsequent socket calls. The IPC uses tempnam, and otherwise the same mechanism. At the moment is only looks for the wildcards on bind, connect looked a bit trickier, but I'm sure it would just be finding the right place. Inproc should be fairly straightforward, but I don't think there's as much of a use case there.
#include "zmq.h" #include <stdio.h> int main (void) { size_t len = 255; char endpoint[len]; void *context = zmq_init(1); void *sock = zmq_socket(context, ZMQ_REQ); // Test 1: test with specified host portion zmq_bind(sock, "tcp://127.0.0.1:*"); zmq_getsockopt(sock, ZMQ_LAST_ENDPOINT, &endpoint, &len); printf("%d - %s\n", (int)len, endpoint); // Test 2: Test with IPC len = 255; // reset the length zmq_bind(sock, "ipc://*"); zmq_getsockopt(sock, ZMQ_LAST_ENDPOINT, &endpoint, &len); printf("%d - %s\n", (int)len, endpoint); zmq_close(sock); zmq_term(context); return 0; }Ian
Pierre Ynard
> I have knocked up a relatively simple patch to implement this on IPC
> and TCP transports, would appreciate comments as I'm pretty sure
> there's some dodgy stuff in there! I've made a pull req, though
> mainly for comment (please don't pull it chuck/mikko/pieter!):
> https://github.com/zeromq/libzmq/pull/238
I like your patch, and I'm looking forward to try it!
> At the moment is only looks for the wildcards on bind, connect looked
> a bit trickier, but I'm sure it would just be finding the right place.
> Inproc should be fairly straightforward, but I don't think there's as
> much of a use case there.
I'm confused, I don't really understand the meaning of connecting to a
wildcard.
A few comments follow:
> + // Allow wildcard file
> + if(*addr_ == '*') {
> + addr_ = tempnam(NULL, NULL);
> + }
Hmm this brings a race condition, between the moment when the
availability of the file name is checked, and the moment the socket is
created. I don't really know how to prevent this :/
> + // Last socket endpoint URI
> + unsigned char last_endpoint [256];
> + size_t last_endpoint_size;
Wouldn't it be better to use ZMQ_ENDPOINT_MAX here?
> + uint16_t port;
> + if (port_str[0] == '*') {
> + // Resolve wildcard to 0 to allow autoselection of port
> + port = 0;
> + } else {
> + // Parse the port number (0 is not a valid port).
> + port = (uint16_t) atoi (port_str.c_str());
I think it would be better and simpler to accept both * and 0
> + rc = getsockname (s, &sa, &sl);
> + if (rc == 0) {
> + char host[INET6_ADDRSTRLEN];
> + int port;
> +
> + if ( sa.sa_family == AF_INET ) {
> + inet_ntop(AF_INET, &(((struct sockaddr_in *)&sa)->sin_addr), host, INET6_ADDRSTRLEN);
> + port = ntohs( ((struct sockaddr_in *)&sa)->sin_port);
> + } else {
> + inet_ntop(AF_INET6, &(((struct sockaddr_in6 *)&sa)->sin6_addr), host, INET6_ADDRSTRLEN);
> + port = ntohs( ((struct sockaddr_in6 *)&sa)->sin6_port);
> + }
> +
> + // Store the address for retrieval by users using wildcards
> + bound_addr_len = sprintf(bound_addr, "tcp://%s:%d", host, port);
You probably want to enclose the address within [ ] if it's IPv6.
Also, is it really necessary to use a bound_addr buffer? It seems to me
that in get_address() you could directly call getsockname() on the
socket file descriptor, and build the address string directly in the
supplied buffer. Normally this could work with IPC sockets too.
Regards,
--
Pierre Ynard
"Une âme dans un corps, c'est comme un dessin sur une feuille de papier."
Ian Barber
I like your patch, and I'm looking forward to try it!
Thanks!
I'm confused, I don't really understand the meaning of connecting to a
wildcard.
It just means "fill in the name for me".
A few comments follow:
> + // Allow wildcard file
> + if(*addr_ == '*') {
> + addr_ = tempnam(NULL, NULL);
> + }
Hmm this brings a race condition, between the moment when the
availability of the file name is checked, and the moment the socket is
created. I don't really know how to prevent this :/
Yep, that was my thought that it is possible - thought given this is somewhat edgecase functionality it may be acceptable to have a chance of a race - all that will happen is the socket will fail to bind, so a retry will almost certainly work.
> + // Last socket endpoint URI
> + unsigned char last_endpoint [256];
> + size_t last_endpoint_size;
Wouldn't it be better to use ZMQ_ENDPOINT_MAX here?
Yes.
> + // Parse the port number (0 is not a valid port).
> + port = (uint16_t) atoi (port_str.c_str());
I think it would be better and simpler to accept both * and 0
Agreed.
> + // Store the address for retrieval by users using wildcards
> + bound_addr_len = sprintf(bound_addr, "tcp://%s:%d", host, port);
You probably want to enclose the address within [ ] if it's IPv6.
Will do.
Also, is it really necessary to use a bound_addr buffer? It seems to me
that in get_address() you could directly call getsockname() on the
socket file descriptor, and build the address string directly in the
supplied buffer. Normally this could work with IPC sockets too.
Ah, nice idea, can't think of a reason not too. I'll take a look and update the patch, thanks for your feedback!
Ian
Pieter Hintjens
>> I'm confused, I don't really understand the meaning of connecting to a
>> wildcard.
I actually made this work in VTX, with UDP, it's neat. The semantics
are "connect to any host that has an open port", and works by
broadcasting a connect request and getting back an OK reply. Cute, but
only works on UDP.
-Pieter
Martin Lucina
Thanks for the patch!
You asked me to review it, so here goes...
This is based on https://github.com/zeromq/libzmq/pull/238.diff as of
today; there doesn't seem to be a better way to uniquely identify a Github
pull request's *content* since it can change all the time :-(
> diff --git a/src/ipc_listener.cpp b/src/ipc_listener.cpp
> index 07a7dff..6797344 100644
> --- a/src/ipc_listener.cpp
> +++ b/src/ipc_listener.cpp
> @@ -95,8 +95,31 @@ void zmq::ipc_listener_t::in_event ()
> send_attach (session, engine, false);
> }
>
> +int zmq::ipc_listener_t::get_address (unsigned char *addr, size_t *len)
Coding style: Please use addr_ and len_ for function parameter names.
> +{
> + struct sockaddr_un sun;
> + int rc;
> +
> + // Get the details of the IPC socket
> + socklen_t sl = sizeof(sockaddr_un);
> + rc = getsockname (s, (sockaddr *)&sun, &sl);
> + if (rc != 0) {
> + return rc;
> + }
> + // Store the address for retrieval by users using wildcards
> + *len = sprintf((char *)addr, "ipc://%s", sun.sun_path);
> +
> + return 0;
> +}
> +
The use of sprintf() is a security hole if the user allocated not enough
space at *addr. Please use snprintf() to ensure a maximum of len bytes
(including the string terminator) are written to *addr.
> int zmq::ipc_listener_t::set_address (const char *addr_)
> {
> +
> + // Allow wildcard file
> + if(*addr_ == '*') {
> + addr_ = tempnam(NULL, NULL);
> + }
> +
> // Get rid of the file associated with the UNIX domain socket that
> // may have been left behind by the previous run of the application.
> ::unlink (addr_);
> @@ -124,7 +147,7 @@ int zmq::ipc_listener_t::set_address (const char *addr_)
> rc = listen (s, options.backlog);
> if (rc != 0)
> return -1;
> -
> +
> return 0;
> }
The use of tempnam() is a potential security hole, although we don't make
any guarantees about untrusted local users. I'm not sure if there is an
equivalent of mkstemp() for UNIX domain sockets, can't remember off the top
of my head.
> diff --git a/src/ipc_listener.hpp b/src/ipc_listener.hpp
> index e1f4817..57e04ef 100644
> --- a/src/ipc_listener.hpp
> +++ b/src/ipc_listener.hpp
> @@ -48,6 +48,9 @@
>
> // Set address to listen on.
> int set_address (const char *addr_);
> +
> + // Get the bound address for use with wildcards
> + int get_address(unsigned char *addr, size_t *len);
Coding style: Use addr_ and len_.
>
> private:
>
> diff --git a/src/options.cpp b/src/options.cpp
> index 4db1a6c..c8790a8 100644
> --- a/src/options.cpp
> +++ b/src/options.cpp
> @@ -30,6 +30,7 @@
> rcvhwm (1000),
> affinity (0),
> identity_size (0),
> + last_endpoint_size(0),
> rate (100),
> recovery_ivl (10000),
> multicast_hops (1),
> @@ -213,7 +214,6 @@ int zmq::options_t::setsockopt (int option_, const void *optval_,
> ipv4only = val;
> return 0;
> }
> -
> }
>
> errno = EINVAL;
> @@ -385,7 +385,15 @@ int zmq::options_t::getsockopt (int option_, void *optval_, size_t *optvallen_)
> *((int*) optval_) = ipv4only;
> *optvallen_ = sizeof (int);
> return 0;
> -
> +
> + case ZMQ_LAST_ENDPOINT:
> + if (*optvallen_ < last_endpoint_size) {
> + errno = EINVAL;
> + return -1;
> + }
> + memcpy (optval_, last_endpoint, last_endpoint_size);
> + *optvallen_ = last_endpoint_size;
> + return 0;
> }
Security hole: You want to copy last_endpoint_size bytes, or optvallen_
bytes, whichever is lesser, and return the actual # of bytes copied in
*optvallen_.
>
> errno = EINVAL;
> diff --git a/src/options.hpp b/src/options.hpp
> index bfc9dc7..7feea95 100644
> --- a/src/options.hpp
> +++ b/src/options.hpp
> @@ -46,6 +46,10 @@
> // Socket identity
> unsigned char identity_size;
> unsigned char identity [256];
> +
> + // Last socket endpoint URI
> + unsigned char last_endpoint [256];
> + size_t last_endpoint_size;
I guess you wanted to use the ZMQ_ENDPOINT_MAX define here rather than
hardcoding 256?
>
> // Maximum tranfer rate [kb/s]. Default 100kb/s.
> int rate;
> diff --git a/src/socket_base.cpp b/src/socket_base.cpp
> index 3761b46..74c807f 100644
> --- a/src/socket_base.cpp
> +++ b/src/socket_base.cpp
> @@ -161,6 +161,7 @@ int zmq::socket_base_t::parse_uri (const char *uri_,
> }
> protocol_ = uri.substr (0, pos);
> address_ = uri.substr (pos + 3);
> +
> if (protocol_.empty () || address_.empty ()) {
> errno = EINVAL;
> return -1;
> @@ -340,6 +341,8 @@ int zmq::socket_base_t::bind (const char *addr_)
> delete listener;
> return -1;
> }
> +
> + rc = listener->get_address (options.last_endpoint, &(options.last_endpoint_size));
> launch_child (listener);
> return 0;
> }
> @@ -354,6 +357,7 @@ int zmq::socket_base_t::bind (const char *addr_)
> delete listener;
> return -1;
> }
> + rc = listener->get_address (options.last_endpoint, &(options.last_endpoint_size));
> launch_child (listener);
> return 0;
> }
> diff --git a/src/tcp_address.cpp b/src/tcp_address.cpp
> index de6e0ad..9fe6083 100644
> --- a/src/tcp_address.cpp
> +++ b/src/tcp_address.cpp
> @@ -387,11 +387,18 @@ int zmq::tcp_address_t::resolve (const char *name_, bool local_, bool ipv4only_)
> addr_str [addr_str.size () - 1] == ']')
> addr_str = addr_str.substr (1, addr_str.size () - 2);
>
> - // Parse the port number (0 is not a valid port).
> - uint16_t port = (uint16_t) atoi (port_str.c_str());
> - if (port == 0) {
> - errno = EINVAL;
> - return -1;
> + uint16_t port;
> + // Allow 0 specifically, to detect invalid port error in atoi if not
> + if (port_str[0] == '*' || port_str[0] == '0') {
> + // Resolve wildcard to 0 to allow autoselection of port
> + port = 0;
> + } else {
> + // Parse the port number (0 is not a valid port).
> + port = (uint16_t) atoi (port_str.c_str());
> + if (port == 0) {
> + errno = EINVAL;
> + return -1;
> + }
> }
I'm not sure I understand what the business with "Allow 0 specifically" is
about?
>
> // Resolve the IP address.
> diff --git a/src/tcp_listener.cpp b/src/tcp_listener.cpp
> index 0b7a90d..191e05c 100644
> --- a/src/tcp_listener.cpp
> +++ b/src/tcp_listener.cpp
> @@ -119,6 +119,37 @@ void zmq::tcp_listener_t::close ()
> s = retired_fd;
> }
>
> +int zmq::tcp_listener_t::get_address (unsigned char *addr, size_t *len)
Coding style: Please use addr_ and len_.
> +{
> + struct sockaddr sa;
> + char host[INET6_ADDRSTRLEN];
> + int port, rc;
> +
> + // Get the details of the TCP socket
> + socklen_t sl = sizeof(sockaddr);
> + rc = getsockname (s, &sa, &sl);
> + if (rc != 0) {
> + return rc;
> + }
> +
> + // Split the retrieval between IPv4 and v6 addresses
> + if ( sa.sa_family == AF_INET ) {
> + inet_ntop(AF_INET, &(((struct sockaddr_in *)&sa)->sin_addr), host, INET6_ADDRSTRLEN);
> + port = ntohs( ((struct sockaddr_in *)&sa)->sin_port);
> +
> + // Store the address for retrieval by users using wildcards
> + *len = sprintf((char *)addr, "tcp://%s:%d", host, port);
> + } else {
> + inet_ntop(AF_INET6, &(((struct sockaddr_in6 *)&sa)->sin6_addr), host, INET6_ADDRSTRLEN);
> + port = ntohs( ((struct sockaddr_in6 *)&sa)->sin6_port);
> +
> + // Store the address for retrieval by users using wildcards
> + *len = sprintf((char *)*addr, "tcp://[%s]:%d", host, port);
> + }
> +
> + return 0;
> +}
Security hole: Please use snprintf(). (See above)
> +
> int zmq::tcp_listener_t::set_address (const char *addr_)
> {
> // Convert the textual address into address structure.
> @@ -168,6 +199,7 @@ int zmq::tcp_listener_t::set_address (const char *addr_)
> errno_assert (rc == 0);
> #endif
>
> +
> // Bind the socket to the network interface and port.
> rc = bind (s, address.addr (), address.addrlen ());
> #ifdef ZMQ_HAVE_WINDOWS
> diff --git a/src/tcp_listener.hpp b/src/tcp_listener.hpp
> index c2116b3..fc6ebd1 100644
> --- a/src/tcp_listener.hpp
> +++ b/src/tcp_listener.hpp
> @@ -44,6 +44,9 @@
>
> // Set address to listen on.
> int set_address (const char *addr_);
> +
> + // Get the bound address for use with wildcard
> + int get_address(unsigned char *addr, size_t *len);
Coding style: Please use addr_ and len_.
One more quick comment about this API: It actually won't work for the most
common TCP case (based on what people are asking for), which is:
bind("tcp://*:*")
The problem is that getsockname() will return INADDR_ANY or IN6ADDR_ANY for
such a socket, and such an address is not something you can *connect* to.
If the ZMQ_LAST_ENDPOINT API is to be theoretically sound, what guarantees
are we providing to the caller, exactly? If the guarantee is "Return an
endpoint I can connect to", then you have a problem with INADDR_ANY.
I don't see how this can be solved. Any ideas? Off the top of my head
trawling through all local interface IP addresses and returning a list of
endpoints but that rapidly becomes pretty horrible.
<philosophy>
We (Martin Sustrik and myself) have always tried to design the ZeroMQ APIs
to behave consistently and give explicit guarantees which can be applied to
all transports/patterns where possible.
In my opinion this is a major win for libzmq, and is an often misunderstood
aspect of the library. You don't notice it because it "just works". Many
times, the reason something has *not* been implemented is that we'd rather
have no implementation than a theoretically unsound one.
</philosophy>
Cheers,
Ian Barber
Hi Ian!
Thanks for the patch!
You asked me to review it, so here goes...
Thanks Mato, I'll update the patch with your suggestions.
I guess you wanted to use the ZMQ_ENDPOINT_MAX define here rather than
hardcoding 256?
Yep, will do.
I'm not sure I understand what the business with "Allow 0 specifically" is
about?
As far as I could see atoi might return zero if the port is invalid, so rather than working but potentially confusing code on passing an system assigned port, the patch looks specifically for * or 0 and checks that, so the other cases will cause an EINVAL.
I don't see how this can be solved. Any ideas? Off the top of my head
trawling through all local interface IP addresses and returning a list of
endpoints but that rapidly becomes pretty horrible.
Yep, I think this is an issue that doesn't have a pleasant solution. I think the functionality is useful without resolving the interface in that case - the OP I think wanted just the port for example. I think that the functionality is useful even with this restriction, and the documentation can reflect this case.
In my opinion this is a major win for libzmq, and is an often misunderstood
aspect of the library. You don't notice it because it "just works". Many
times, the reason something has *not* been implemented is that we'd rather
have no implementation than a theoretically unsound one.
Ack, I think this is a good principle, and I can see the questions it brings to this case.
Ian
AJ Lewis
> The use of sprintf() is a security hole if the user allocated not
> enough space at *addr. Please use snprintf() to ensure a maximum of
> len bytes (including the string terminator) are written to *addr.
Be aware that the Windows version (and some UNIX versions) behave
differently than the GCC snprintf. For example, on Windows, if the
number of bytes required to store the data exceeds count, then count
bytes are stored, a negative values is returned, and the string is *not*
NULL terminated! Quite annoying.
--
AJ Lewis
Software Engineer
Quantum Corporation
Work: 651 688-4346
----------------------------------------------------------------------
The information contained in this transmission may be confidential. Any disclosure, copying, or further distribution of confidential information is not permitted unless such privilege is explicitly granted in writing by Quantum. Quantum reserves the right to have electronic communications, including email and attachments, sent across its networks filtered through anti virus and spam software programs and retain such messages in order to comply with applicable data security and retention requirements. Quantum is not responsible for the proper and complete transmission of the substance of this communication or for any delay in its receipt.
Ian Barber
Be aware that the Windows version (and some UNIX versions) behave
differently than the GCC snprintf. For example, on Windows, if the
number of bytes required to store the data exceeds count, then count
bytes are stored, a negative values is returned, and the string is *not*
NULL terminated! Quite annoying.
Thanks for that - in discussions with Mikko it seemed it might be easier to use a std::string internally, and only turn to a c string when it is requested, so I'll put that into the patch when I get a sec.
Ian