Re: [PATCH] Avoid interpreting too-long parameter as file name

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 8, 2016, 20:46 UTC
Message-ID: <xmqq4mdj7ww4.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1454908588-22475-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 14 quoted lines

> Even if it is easier to write HEAD~2000, it is legal to write
> HEAD^^^... (repeats "^" 2000 times in total). However, such a string is
> too long to be a legal filename (and on Windows, by default even much,
> much shorter strings are still illegal because they exceed MAX_PATH).
>
> Therefore, if the check_filename() function encounters too long a
> command-line parameter, it should interpet the error code ENAMETOOLONG
> as a strong hint that this is not a file name instead of dying with an
> error message.
>
> Noticed-by: Ole Tange <ole@tange.dk>
> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

While I think it is a good thing to try solving, i.e. it would be nicer to the user if "git foo HEAD^^^^..." can be spelled without needing a "--" disambiguation, I am not sure this patch solves it at the right level. The log message is unclear if the patch author even thought about ramifications of the callers not involved in the case written in it.

For example, verify_filename() calls this function, saying "This string must name a file and otherwise I want you to die".

There is a direct call to check_filename() in builtin/checkout.c; it is unclear how it would interact with this change, either.