> diff --git a/fetch-pack.c b/fetch-pack.c
> index a9604f35a3e..62ea90541c5 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -1653,8 +1653,12 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
> receive_wanted_refs(&reader, sought, nr_sought);
>
> /* get the pack(s) */
> + if (git_env_bool("GIT_TRACE_REDACT", 1))
> + reader.options |= PACKET_READ_REDACT_URI_PATH;
> if (process_section_header(&reader, "packfile-uris", 1))
> receive_packfile_uris(&reader, &packfile_uris);
> + reader.options &= ~PACKET_READ_REDACT_URI_PATH;
Probably worth commenting why you're resetting the flag (avoid the
relatively expensive URI check when we don't need it).
> diff --git a/pkt-line.c b/pkt-line.c
> index 2dc8ac274bd..5a69ddc2e77 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -370,6 +370,31 @@ int packet_length(const char lenbuf_hex[4])
> return (val < 0) ? val : (val << 8) | hex2chr(lenbuf_hex + 2);
> }
>
> +static char *find_packfile_uri_path(const char *buffer)
> +{
> + const char *URI_MARK = "://";
> + char *path;
> + int len;
> +
> + /* First char is sideband mark */
> + buffer += 1;
> +
> + len = strspn(buffer, "0123456789abcdefABCDEF");
> + if (len != (int)the_hash_algo->hexsz || buffer[len] != ' ')
> + return NULL; /* required "<hash>SP" not seen */
Optional: As I said in my reply (just sent out), checking for both SHA-1
and SHA-256 lengths is reasonable too.
[1] https://lore.kernel.org/git/20211108224335.569596-1-jonathantanmy@xxxxxxxxxx/
> diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
> index d527cf6c49f..f01af2f2ed3 100755
> --- a/t/t5702-protocol-v2.sh
> +++ b/t/t5702-protocol-v2.sh
> @@ -1107,6 +1107,57 @@ test_expect_success 'packfile-uri with transfer.fsckobjects fails when .gitmodul
> test_i18ngrep "disallowed submodule name" err
> '
>
> +test_expect_success 'packfile-uri path redacted in trace' '
> + P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
> + rm -rf "$P" http_child log &&
> +
> + git init "$P" &&
> + git -C "$P" config "uploadpack.allowsidebandall" "true" &&
> +
> + echo my-blob >"$P/my-blob" &&
> + git -C "$P" add my-blob &&
> + git -C "$P" commit -m x &&
> +
> + git -C "$P" hash-object my-blob >objh &&
> + git -C "$P" pack-objects "$HTTPD_DOCUMENT_ROOT_PATH/mypack" <objh >packh &&
> + git -C "$P" config --add \
> + "uploadpack.blobpackfileuri" \
> + "$(cat objh) $(cat packh) $HTTPD_URL/dumb/mypack-$(cat packh).pack" &&
> +
> + GIT_TRACE=1 GIT_TRACE_PACKET="$(pwd)/log" GIT_TEST_SIDEBAND_ALL=1 \
No need for GIT_TRACE=1 since you're not checking stdout. Also I don't
think GIT_TEST_SIDEBAND_ALL=1 is needed - we should check that it works
even without a test variable (and I've checked and it seems to work).
[snip]
> +test_expect_success 'packfile-uri path not redacted in trace when GIT_TRACE_REDACT=0' '
> + P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
> + rm -rf "$P" http_child log &&
> +
> + git init "$P" &&
> + git -C "$P" config "uploadpack.allowsidebandall" "true" &&
> +
> + echo my-blob >"$P/my-blob" &&
> + git -C "$P" add my-blob &&
> + git -C "$P" commit -m x &&
> +
> + git -C "$P" hash-object my-blob >objh &&
> + git -C "$P" pack-objects "$HTTPD_DOCUMENT_ROOT_PATH/mypack" <objh >packh &&
> + git -C "$P" config --add \
> + "uploadpack.blobpackfileuri" \
> + "$(cat objh) $(cat packh) $HTTPD_URL/dumb/mypack-$(cat packh).pack" &&
> +
> + GIT_TRACE=1 GIT_TRACE_PACKET="$(pwd)/log" GIT_TEST_SIDEBAND_ALL=1 \
Same comment here.
