Re: [PATCH v3] git-credential-store: skip empty lines and comments from store
Thank you very much. That's correct and ideal in my eyes.
Regarding the comments, this is a new feature, of course. I think it's a worthless discussion about the question if the correct handling of empty lines are a bugfix or a new feature. In fact empty lines were handled correcty (ignored). So this might be considered a feature. But it wasn't documented, so it's a bugfix...
Anyway. Thank you all.
Dirk
Am 27.04.20 um 14:59 schrieb Carlo Marcelo Arenas Belón:
Show 88 quoted lines
> with the added checks for invalid URLs in credentials, any locally
> modified store files which might have empty lines or even comments
> were reported[1] failing to parse as valid credentials.
>
> using the store file in this manner wasn't intended by the original
> code and it had latent issues which the new code dutifully prevented
> but since the strings used wouldn't had been valid credentials anyway
> we could instead detect them and skip the matching logic and therefore
> formalize this "feature".
>
> trim all lines as they are being read from the store file and skip the
> ones that will be otherwise empty or that start with "#" (therefore
> assuming them to be comments)
>
> [1] https://stackoverflow.com/a/61420852/5005936
>
> Reported-by: Dirk <dirk@ed4u.de>
> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
> v3:
> * avoid using q_to_cr as suggested by Peff
> * a more verbose commit message and slightly more complete documentation
> v2:
> * use a here-doc for clarity as suggested by Eric
> * improve commit message and include documentation
>
> Documentation/git-credential-store.txt | 7 +++++++
> credential-store.c | 3 +++
> t/t0302-credential-store.sh | 15 +++++++++++++++
> 3 files changed, 25 insertions(+)
>
> diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt
> index 693dd9d9d7..48ab4b13e5 100644
> --- a/Documentation/git-credential-store.txt
> +++ b/Documentation/git-credential-store.txt
> @@ -101,6 +101,13 @@ username (if we already have one) match, then the password is returned
> to Git. See the discussion of configuration in linkgit:gitcredentials[7]
> for more information.
>
> +Note that the file used is not a configuration file and should be ideally
> +managed only through git, as any manually introduced typos will compromise
> +the validation of credentials.
> +
> +The parser will ignore any lines starting with the '#' character during
> +the processing of credentials for read, though.
> +
> GIT
> ---
> Part of the linkgit:git[1] suite
> diff --git a/credential-store.c b/credential-store.c
> index c010497cb2..b2f160890d 100644
> --- a/credential-store.c
> +++ b/credential-store.c
> @@ -24,6 +24,9 @@ static int parse_credential_file(const char *fn,
> }
>
> while (strbuf_getline_lf(&line, fh) != EOF) {
> + strbuf_trim(&line);
> + if (line.len == 0 || *line.buf == '#')
> + continue;
> credential_from_url(&entry, line.buf);
> if (entry.username && entry.password &&
> credential_match(c, &entry)) {
> diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
> index d6b54e8c65..5e6ace3a06 100755
> --- a/t/t0302-credential-store.sh
> +++ b/t/t0302-credential-store.sh
> @@ -120,4 +120,19 @@ test_expect_success 'erase: erase matching credentials from both xdg and home fi
> test_must_be_empty "$HOME/.config/git/credentials"
> '
>
> +test_expect_success 'get: allow for empty lines or comments in store file' '
> + test_write_lines "#comment" " " "" \
> + https://user:pass@example.com >"$HOME/.git-credentials" &&
> + check fill store <<-\EOF
> + protocol=https
> + host=example.com
> + --
> + protocol=https
> + host=example.com
> + username=user
> + password=pass
> + --
> + EOF
> +'
> +
> test_done