From: Junio C Hamano Date: Fri, 23 Oct 2020 14:44:27 GMT Subject: Re: [PATCH v2 3/3] sideband: add defense against packets missing a band designator Message-ID: In-Reply-To: <20201023083424.GD4012156@coredump.intra.peff.net> Jeff King writes: > On Mon, Oct 19, 2020 at 07:35:42PM +0000, Johannes Schindelin via GitGitGadget wrote: > >> From: Johannes Schindelin >> >> While there is currently no instance of code producing this type of >> packet, if the `demultiplex_sideband()` would receive a packet whose >> payload is not only empty but even misses the band designator, it would >> mistake it for a flush packet. >> >> Let's defend against such a bug in the future. > > That seems reasonable, but I'm not sure if these ought to be BUG()s. > Isn't it an indication that the other side sent us bogus input? That > likely is a bug on the other end, but I think it should be a die(), just > as we would produce for any other malformed protocol input. Thanks for spotting. I also think this was a good change, but at this point in the code we found a problem in the data the other side created (i.e. we diagnosed a bug on the other side), which is a usual input error, so it should not be a BUG(). Would this be something we can warn and ignore if the connection is still active, I wonder, though.