bundle2: error parts can exceed length limits and crash the server

Siddharth Agarwal sid at less-broken.com
Tue Apr 4 00:13:38 EDT 2017


On 4/3/17 20:54, Gregory Szorc wrote:
> On Mon, Apr 3, 2017 at 8:51 PM, Siddharth Agarwal <sid at less-broken.com 
> <mailto:sid at less-broken.com>> wrote:
>
>     On 4/3/17 20:48, Gregory Szorc wrote:
>
>
>         This sounds reasonable to me.
>
>         This will likely require a new bundle2 part(s) in order to
>         differentiate the behavior of the part. Alternatively, I
>         suppose there could be a bundle2 capability identifying
>         whether the peer understands error messages in payload chunks
>         and we could recycle the part name. But I'm a fan of parts
>         having strict, well-defined behavior that doesn't change over
>         time because otherwise things are too complicated. So my vote
>         is for a new part name/type.
>
>
>     So technically we could retrofit this into the existing parts by
>     following what I outlined. That's actually what I was thinking of
>     doing.
>
>     We don't need a separate bundle2 capability or anything either.
>
>     Old clients + new server = will display the truncated message,
>     still better than today. The payload will simply be ignored.
>     New client + old server = can display the message from the param
>     rather than from the payload
>
>     What do you think?
>
>
> Oh, so you'll redundantly put a copy of the error message both in a 
> param and in the payload to cover both old and new use cases? If so, 
> yeah, that'll work. There should be minimal stream overhead because 
> compression should pave over the redundancy.

Yeah, exactly.

OK, I'm going to go ahead and implement that. Thanks.


More information about the Mercurial-devel mailing list