Re: [PATCH v2 09/25] upload-pack: tighten number parsing at "deepen" lines
On Fri, Feb 5, 2016 at 6:48 AM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> Signed-off-by: Nguyễn Thái Ngọc Duy >> --- > > 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". 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" is not that bad, but fixing it is still a good idea. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 09/25] upload-pack: tighten number parsing at "deepen" lines
Nguyễn Thái Ngọc Duy writes:
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
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". We used not to notice, but now we reject such a
reimplementation of Git.
> upload-pack.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/upload-pack.c b/upload-pack.c
> index 257ad48..9f14933 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -641,9 +641,9 @@ static void receive_needs(void)
> continue;
> }
> if (skip_prefix(line, "deepen ", &arg)) {
> - char *end;
> + char *end = NULL;
> depth = strtol(arg, &end, 0);
> - if (end == arg || depth <= 0)
> + if (!end || *end || depth <= 0)
> die("Invalid deepen: %s", line);
> continue;
> }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
