Re: [PATCH v3 7/8] fast-import: forbid escaped NUL in paths

[Junio C Hamano <gitster@pobox.com>]

Thalia Archibald <thalia@archibald.dev> writes:

> NUL cannot appear in paths. Even disregarding filesystem path
> limitations, the tree object format delimits with NUL, so such a path
> cannot be encoded by Git.
>
> When a quoted path is unquoted, it could possibly contain NUL from
> "\000". Forbid it so it isn't truncated.
>
> fast-import still has other issues with NUL, but those will be addressed
> later.

Later meaning outside the series, as 8/8 is not about that?  Not a
complaint, and if the way I interpreted is correct, then there is no
need to update the above statement.  Just double-checking.

> Signed-off-by: Thalia Archibald <thalia@archibald.dev>
> ---
>  Documentation/git-fast-import.txt | 1 +
>  builtin/fast-import.c             | 2 ++
>  t/t9300-fast-import.sh            | 1 +
>  3 files changed, 4 insertions(+)
>
> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
> index db53b50268..edda30f90c 100644
> --- a/Documentation/git-fast-import.txt
> +++ b/Documentation/git-fast-import.txt
> @@ -660,6 +660,7 @@ and must be in canonical form. That is it must not:
>  
>  The root of the tree can be represented by an empty string as `<path>`.
>  
> +`<path>` cannot contain NUL, either literally or escaped as `\000`.

OK.

> +		if (strlen(sb->buf) != sb->len)
> +			die("NUL in %s: %s", field, command_buf.buf);

Nice.  !memchr(sb->buf, ch, sb->len) would be more general solution
if we were looking for ch that is not NUL, but for checking NUL,
what you wrote is the most natural to read.

>  	} else {
>  		if (include_spaces)
>  			*endp = p + strlen(p);
> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
> index 5cde8f8d01..1e68426852 100755
> --- a/t/t9300-fast-import.sh
> +++ b/t/t9300-fast-import.sh
> @@ -3300,6 +3300,7 @@ test_path_base_fail () {
>  	local change="$1" prefix="$2" field="$3" suffix="$4"
>  	test_path_fail "$change" 'unclosed " in '"$field"          "$prefix" '"hello.c'    "$suffix" "Invalid $field"
>  	test_path_fail "$change" "invalid escape in quoted $field" "$prefix" '"hello\xff"' "$suffix" "Invalid $field"
> +	test_path_fail "$change" "escaped NUL in quoted $field"    "$prefix" '"hello\000"' "$suffix" "NUL in $field"
>  }
>  test_path_eol_quoted_fail () {
>  	local change="$1" prefix="$2" field="$3"