On Mon, Nov 27, 2017 at 12:37:16AM +0100, Alexander Bluhm wrote:
> On Sun, Nov 26, 2017 at 09:58:48PM +0100, Klemens Nanni wrote:
> > On Sun, Nov 26, 2017 at 09:17:11PM +0100, Alexander Bluhm wrote:
> > > On Sun, Nov 26, 2017 at 03:52:33PM +0100, Klemens Nanni wrote:
> > > > nc as client keeps sending after the server shutdown the socket:
> > > 
> > > Yes, that is a half closed connection.  Server has shutdown, but
> > > client is still sending.  This is a TCP feature made accessible by
> > > the nc tool.
> > Of course this is a valid scenario and I thought about my patch breaking
> > use cases where the server closes it's write end but continues/expects
> > to read more data from the client.
> 
> You patch affects server and client.  You should also think about
> when the server calls shutdown write.  There is a race when netinbufpos
> != 0 and some data gets lost.  I think the behavior you wish could
> be implemented by removing the "lflag &&" at the check above your
> patch.
Removing the lflag condition was actually my first iteration but
checking fd's seemed more reasonable; I stand corrected.

> > The reason I chose this behaviour is that nc(1)'s
> > 
> >     -N      shutdown(2) the network socket after EOF on the input.
> >             Some servers require this to finish their work.
> > 
> > does not mention how it shuts down the socket in particular, e.g.
> > whether just SHUT_RD/SHUT_WR or SHUT_RDWR. As a reader I'd assume it to
> > shut both ends down and end the connection.
> 
> It shuts down write.  When we read EOF from stdin we want to send
> this over the TCP connection by saying we will not send anymore.
Sure, that was clear to be from the beginning. But going over it again
after your feedback makes it reasonable that we only SHUT_RD after EOF
since that's the very same direction/analog signal.

> > > > tedu@ introduced this with r1.125 (three years ago), r1.124 works as
> > > > expected just like telnet does for that matter:
> > > 
> > > Who is expecting that 1.124 is the correct behavior?
> > Well, I (naively) did.
> 
> netcat is not telnet.  netcat should (and I think it does) allow
> to run half closed connections as TCP does.
> 
> > > And why was this changed?  Why are the reasons for r1.125 no longer
> > > valid?
> > That's a question I didn't ask but rather assumed it to be an undesired
> > change.
> > 
> > Was it an intentional change in behaviour?
> 
> I don't know that.  But there was some discussion about the shutdown
> behavior and commits going in and out a few years ago.  Currently
> netcat works for me as expected and I am not aware of any use cases
> that do not work.  So I am reluctant to change anything without a
> real problem.
> 
> > > Which use case does your change fix?
> > The one where I expect netcat clients to exit upon netcat servers
> > shutting down the socket (as telnet does) due to my interpretation of
> > -N's description.
> 
> I think you misunderstood it.  shutdown(2) means half closed TCP,
> not connection close and exit.
Understanding shutdown(2) and its implications was never a problem, my
expectations for netcat's behaviour were simply wrong/misleading.

> > If the current behaviour is indeed intended, I'd like to clarify -N's
> > description at least so that users can safely expect the server to
> > continue to read.
> 
> Like this?  Does this make clear what is going on?
> 
> bluhm
> 
> Index: usr.bin/nc/nc.1
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.bin/nc/nc.1,v
> retrieving revision 1.87
> diff -u -p -r1.87 nc.1
> --- usr.bin/nc/nc.1   15 Jul 2017 18:11:47 -0000      1.87
> +++ usr.bin/nc/nc.1   26 Nov 2017 22:39:04 -0000
> @@ -183,7 +183,7 @@ Ask the kernel to drop incoming packets 
>  .Ar minttl .
>  .It Fl N
>  .Xr shutdown 2
> -the network socket after EOF on the input.
> +writing to the network socket after EOF on the input.
>  Some servers require this to finish their work.
>  .It Fl n
>  Do not do any DNS or service lookups on any specified addresses,
> 
Almost exactly what I had, so yes. It's clear to me without the diff
*now* but definitely shows improvement.

Reply via email to