Re: [PATCH v2 09/25] upload-pack: tighten number parsing at "deepen" lines

From: Duy Nguyen <pclouds@gmail.com>
Date: Feb 15, 2016, 03:07 UTC
Message-ID: <CACsJy8DLnWpB+3+DRTq0MxAK82qe8yLVrLV29FDY=BmrWh4wWw@mail.gmail.com>
In-Reply-To: <xmqqvb64doln.fsf@gitster.mtv.corp.google.com>

On Fri, Feb 5, 2016 at 6:48 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 16 quoted lines

> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>
> Hmm, so "deepen 10-by-the-way-let-me-tell-you-something-else" was an
> acceptable input that some (third-party) version of "git fetch"
> could have used, but now we are rejecting it.
>
> That "let me tell you something else" part wouldn't have reached
> outside this code, so it is inconceivable that anybody would relied
> on that looseness as a "feature", so the only practical risk would
> be if somebody wrote a protocol driver, mumbling "on the Internet,
> the end of line is CRLF, just like SMTP does", that sends a "deepen
> 10<CR><LF>".  We used not to notice, but now we reject such a
> reimplementation of Git.

On the other hand, if a broken client sends "deepen 10f" instead of "deepen 271", we should reject and let the client be fixed instead of sending a fetch of 10 commits deep back. "10<CR>" is not that bad, but fixing it is still a good idea.

-- 
Duy