On 2013/03/19 14:35, Ted Unangst wrote: > On Tue, Mar 19, 2013 at 17:39, Stuart Henderson wrote: > > On 2013/03/19 18:26, Otto Moerbeek wrote: > >> On Tue, Mar 19, 2013 at 04:00:46PM +0100, Martin Pelikan wrote: > >> > >> > > wfd is stdin, so doing a shutdown on it will mostly be a noop, right? > >> > > >> > Of course you're right. I was so focused on finding the bug I didn't > >> > look above what the fd is :-( > >> > > >> > Are you okay with removing this particular shutdown(2) line? > >> > >> Yes, but it would even be better if there would be an option to get > >> the shutdown on EOF behaviour back. > >> > >> Some servers wait until they see the shutdown from the client to finish > >> their work. > >> > >> -Otto > >> > > > > OK? > > woah. Can somebody explain, using very small words, exactly what the > problem is?
Some servers terminate the connection as soon as the socket is shutdown. $ echo "89.0.0.0" | nc whois.ripe.net 43 % This is the RIPE Database query service. % The objects are in RPSL format. % % The RIPE Database is subject to Terms and Conditions. % See http://www.ripe.net/db/support/db-terms-conditions.pdf so truncating the output; there should be many more lines (as you'll see with whois -r 89.0.0.0). On the other hand, Otto reports servers waiting for the shutdown, so ideally we need to cater for both types. > From reading the ubuntu bug report, their problem comes from adding a > -q flag very similar to the proposed -N flag, and *that's* what broke. > Unmodified netcat does not have whatever bug they're talking about. OpenBSD netcat has a shutdown() which neither Hobbit's netcat nor GNU netcat has. Debian added this "quit timer" patch with a numeric value some time ago, with a default to match traditional behaviour. The argument on the ubuntu report is about default settings. The report says this - The original netcat from OpenBSD does not have a -q option if I (Soren) remember correctly and behaves similarly to netcat-traditional. - which is wrong; currently OpenBSD netcat does not behave similarly to netcat-traditional (aka Hobbit's netcat). > Why are we adding a flag that the ubuntu bug report is requesting be > reverted? > > For that matter, if this is a real problem, why are we using -N and > not -q? This seems like a wholly gratuitous difference for no benefit. I couldn't find this report and the ubuntu/debian patch when I wrote this; so picked a letter not used in the unpatched GNU netcat (which has not been touched for 8 years), Hobbit's or our netcat. (Being English I would not have come up with q for 'quit' by myself ;) Their patch looks like this, if we want to go down this route then a default of -1 to match Hobbit's and GNU netcat would make sense. From: Aron Xu <a...@debian.org> Date: Mon, 13 Feb 2012 15:16:04 +0800 Subject: quit timer --- nc.1 | 5 +++++ netcat.c | 38 +++++++++++++++++++++++++++++++++----- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/nc.1 b/nc.1 index af44976..0d92b74 100644 --- a/nc.1 +++ b/nc.1 @@ -40,6 +40,7 @@ .Op Fl O Ar length .Op Fl P Ar proxy_username .Op Fl p Ar source_port +.Op Fl q Ar seconds .Op Fl s Ar source .Op Fl T Ar toskeyword .Op Fl V Ar rtable @@ -148,6 +149,10 @@ Proxy authentication is only supported for HTTP CONNECT proxies at present. Specifies the source port .Nm should use, subject to privilege restrictions and availability. +.It Fl q Ar seconds +after EOF on stdin, wait the specified number of seconds and then quit. If +.Ar seconds +is negative, wait forever. .It Fl r Specifies that source and/or destination ports should be chosen randomly instead of sequentially within a range or in the order that the system diff --git a/netcat.c b/netcat.c index 4f4d2bf..29ecf1a 100644 --- a/netcat.c +++ b/netcat.c @@ -86,6 +86,7 @@ #include <errno.h> #include <netdb.h> #include <poll.h> +#include <signal.h> #include <stdarg.h> #include <stdio.h> #include <stdlib.h> @@ -120,6 +121,7 @@ int lflag; /* Bind to local port */ int nflag; /* Don't do name look up */ char *Pflag; /* Proxy username */ char *pflag; /* Localport flag */ +int qflag = 0; /* Quit after some secs */ int rflag; /* Random ports flag */ char *sflag; /* Source Address */ int tflag; /* Telnet Emulation */ @@ -158,6 +160,7 @@ void usage(int); static int connect_with_timeout(int fd, const struct sockaddr *sa, socklen_t salen, int ctimeout); +static void quit(); int main(int argc, char *argv[]) @@ -181,7 +184,7 @@ main(int argc, char *argv[]) sv = NULL; while ((ch = getopt(argc, argv, - "46CDdhI:i:jklnO:P:p:rSs:tT:UuV:vw:X:x:z")) != -1) { + "46CDdhI:i:jklnO:P:p:q:rSs:tT:UuV:vw:X:x:z")) != -1) { switch (ch) { case '4': family = AF_INET; @@ -235,6 +238,11 @@ main(int argc, char *argv[]) case 'p': pflag = optarg; break; + case 'q': + qflag = strtonum(optarg, INT_MIN, INT_MAX, &errstr); + if (errstr) + errx(1, "quit timer %s: %s", errstr, optarg); + break; case 'r': rflag = 1; break; @@ -924,9 +932,18 @@ readwrite(int nfd) } else if (pfd[1].revents & POLLHUP) { shutdown_wr: + /* if the user asked to exit on EOF, do it */ + if (qflag == 0) { shutdown(nfd, SHUT_WR); - pfd[1].fd = -1; - pfd[1].events = 0; + close(wfd); + } + /* if user asked to die after a while, arrange for it */ + if (qflag > 0) { + signal(SIGALRM, quit); + alarm(qflag); + } + pfd[1].fd = -1; + pfd[1].events = 0; } } } @@ -1164,6 +1181,7 @@ help(void) \t-O length TCP send buffer length\n\ \t-P proxyuser\tUsername for proxy authentication\n\ \t-p port\t Specify local port for remote connects\n\ + \t-q secs\t quit after EOF on stdin and delay of secs\n\ \t-r Randomize remote ports\n\ \t-S Enable the TCP MD5 signature option\n\ \t-s addr\t Local source address\n\ @@ -1186,9 +1204,19 @@ usage(int ret) { fprintf(stderr, "usage: nc [-46CDdhjklnrStUuvz] [-I length] [-i interval] [-O length]\n" - "\t [-P proxy_username] [-p source_port] [-s source] [-T toskeyword]\n" - "\t [-V rtable] [-w timeout] [-X proxy_protocol]\n" + "\t [-P proxy_username] [-p source_port] [-q seconds] [-s source]\n" + "\t [-T toskeyword] [-V rtable] [-w timeout] [-X proxy_protocol]\n" "\t [-x proxy_address[:port]] [destination] [port]\n"); if (ret) exit(1); } + +/* + * quit() + * handler for a "-q" timeout (exit 0 instead of 1) + */ +static void quit() +{ + /* XXX: should explicitly close fds here */ + exit(0); +} ............. Is their XXX comment valid? it doesn't seem really necessary with an explicit exit().
