On Tue, Oct 26 2021, Ivan Frade via GitGitGadget wrote:
> From: Ivan Frade <ifrade@xxxxxxxxxx>
>
> http-fetch prints the URL after failing to fetch it. This can be
> confusing to users (they cannot really do anything with it) but even
> worse, they can share by accident a sensitive URL (e.g. with
> credentials) while looking for help.
>
> Redact the URL unless the GIT_TRACE_REDACT variable is set to false. This
> mimics the redaction of other sensitive information in git, like the
> Authorization header in HTTP.
>
> Signed-off-by: Ivan Frade <ifrade@xxxxxxxxxx>
> ---
> http-fetch.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/http-fetch.c b/http-fetch.c
> index fa642462a9e..bbe09a6ad9f 100644
> --- a/http-fetch.c
> +++ b/http-fetch.c
> @@ -4,6 +4,7 @@
> #include "http.h"
> #include "walker.h"
> #include "strvec.h"
> +#include "urlmatch.h"
>
> static const char http_fetch_usage[] = "git http-fetch "
> "[-c] [-t] [-a] [-v] [--recover] [-w ref] [--stdin | --packfile=hash | commit-id] url";
> @@ -63,8 +64,18 @@ static void fetch_single_packfile(struct object_id *packfile_hash,
> if (start_active_slot(preq->slot)) {
> run_active_slot(preq->slot);
> if (results.curl_result != CURLE_OK) {
> - die("Unable to get pack file %s\n%s", preq->url,
> - curl_errorstr);
> + struct url_info url;
> + char *nurl = url_normalize(preq->url, &url);
> + if (!git_env_bool("GIT_TRACE_REDACT", 1) || !nurl) {
> + die("Unable to get pack file %s\n%s", preq->url,
> + curl_errorstr);
small nit: arrange if's from "if (cheap || expensive)", i.e. no need for
getenv() if !nurl, but maybe compilers are smart enough for that...
nit: die() messages should start with lower-case (in CodingGuidelines), and I think it's better to quote both, so:
die("unable to get pack '%s': '%s'", ...)
Or maybe without the second '%s', as in 3e8084f1884 (http: check
CURLE_SSL_PINNEDPUBKEYNOTMATCH when emitting errors, 2021-09-24) (which
I authored, but just copy/pasted the convention in the surrounding
code)>
> + } else {
> + char *schema = xstrndup(url.url, url.scheme_len);
> + char *host = xstrndup(&url.url[url.host_off], url.host_len);
> + die("failed to get '%s' url from '%s' "
> + "(full URL redacted due to GIT_TRACE_REDACT setting)\n%s",
> + schema, host, curl_errorstr);
Hrm, I haven't tested, but aren't both of those xstrndup's redundant to
using %*s instead of %s for the printf format? I.e.:
die("failed to get '%*s'[...]", url.schema_len, url.url, )
