Re: [PATCH v2 3/3] sideband: add defense against packets missing a band designator
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 23, 2020, 14:44 UTC
- Message-ID
- <xmqqv9f1huic.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20201023083424.GD4012156@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 15 quoted lines
> On Mon, Oct 19, 2020 at 07:35:42PM +0000, Johannes Schindelin via GitGitGadget wrote: > >> From: Johannes Schindelin <johannes.schindelin@gmx.de> >> >> 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.