Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Jeff King <>
To: Thomas Desveaux <>
Cc: "Carlo Marcelo Arenas Belón" <>,
Subject: Re: Git Bug Report: git credential 'url' parsing clear other fields
Date: Mon, 29 Apr 2024 01:25:40 -0400	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Fri, Apr 26, 2024 at 09:27:03PM +0200, Thomas Desveaux wrote:

> input:
> ```
> username=user
> password=s3cret
> url=
> ```
> This next input works as intended.
> input:
> ```
> url=
> username=user
> password=s3cret
> ```
> [...]
> `username` and `password` are correctly parsed and stored in the struct.
> `url` parsing then clear the fields.

Thanks for a clear report and example.

However, what you're seeing is the intended behavior. When the "url"
feature was added, it came with these docs in git-credential.txt:

  When this special attribute is read by `git credential`, the value is
  parsed as a URL and treated as if its constituent parts were read
  (e.g., `url=` would behave as if `protocol=https`
  and `` had been provided). This can help callers avoid
  parsing URLs themselves.  Note that any components which are missing
  from the URL (e.g., there is no username in the example above) will be
  set to empty; if you want to provide a URL and override some
  attributes, provide the URL attribute first, followed by any

That last "Note that..." part was lost in 1aed817f99 (credential:
document protocol updates, 2020-05-06). Between digging around in the
thread[1] and my recollection, I think the point was that we might drop
the "override" behavior as a defense-in-depth against injection attacks
(like "path=evil/\"). I.e., make it an error to specify
"host=" more than once. But we didn't actually make that code change

However, even if we did so, I think we'd keep the "clear" half of the
behavior (we parsed a URL with no password in it, so the result is a
credential with no password in it). In which case your first example
still wouldn't behave as you expected.

So I think 1aed817f99 may have gone a bit too far and caused the
documentation to be misleading. It should probably keep the "any
components which are missing will be set to empty" part.



      reply	other threads:[~2024-04-29  5:25 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-26 19:27 Git Bug Report: git credential 'url' parsing clear other fields Thomas Desveaux
2024-04-29  5:25 ` Jeff King [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).