Re: [PATCH v3 7/8] fast-import: forbid escaped NUL in paths
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 10, 2024, 18:51 UTC
- Message-ID
- <xmqqwmp5rqz1.fsf@gitster.g>
- In-Reply-To
- <dc67464b6aec46c5b6ac0d9cb8549d48a35c5353.1712741871.git.thalia@archibald.dev>
Thalia Archibald <thalia@archibald.dev> writes:
Show 9 quoted lines
> 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.
Show 16 quoted lines
> 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.
Show 15 quoted lines
> } 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"