All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Carlo Marcelo Arenas Belón" <carenas@gmail.com>
To: git@vger.kernel.org
Cc: dirk@ed4u.de, sunshine@sunshineco.com, peff@peff.net,
	"Carlo Marcelo Arenas Belón" <carenas@gmail.com>
Subject: [PATCH v3] git-credential-store: skip empty lines and comments from store
Date: Mon, 27 Apr 2020 05:59:15 -0700	[thread overview]
Message-ID: <20200427125915.88667-1-carenas@gmail.com> (raw)
In-Reply-To: <20200427084235.60798-1-carenas@gmail.com>

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
-- 
2.26.2.569.g1d74ac4d14


  parent reply	other threads:[~2020-04-27 12:59 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-26 23:47 [PATCH] git-credential-store: skip empty lines and comments from store Carlo Marcelo Arenas Belón
2020-04-27  0:19 ` Eric Sunshine
2020-04-27  0:46   ` Carlo Marcelo Arenas Belón
2020-04-27  8:42 ` [PATCH v2] " Carlo Marcelo Arenas Belón
2020-04-27 11:52   ` Jeff King
2020-04-27 12:25     ` Carlo Marcelo Arenas Belón
2020-04-27 14:43       ` Eric Sunshine
2020-04-27 17:47     ` Junio C Hamano
2020-04-27 19:09       ` Jeff King
2020-04-27 12:59   ` Carlo Marcelo Arenas Belón [this message]
2020-04-27 13:48     ` [PATCH v3] " Philip Oakley
2020-04-28  1:49       ` Carlo Marcelo Arenas Belón
2020-04-29 10:09         ` Philip Oakley
2020-04-27 15:39     ` Dirk
2020-04-27 18:09     ` Junio C Hamano
2020-04-27 19:18       ` Jeff King
2020-04-27 20:43         ` Junio C Hamano
2020-04-27 21:10           ` Jeff King
2020-04-28  1:37             ` Carlo Marcelo Arenas Belón
2020-04-27 23:49           ` Carlo Marcelo Arenas Belón
2020-04-28  5:25           ` Jonathan Nieder
2020-04-28  5:41             ` Jeff King
2020-04-28  7:18               ` Carlo Marcelo Arenas Belón
2020-04-28  8:16                 ` Jeff King
2020-04-28 11:25                   ` Carlo Marcelo Arenas Belón
2020-04-28 10:58             ` Stefan Tauner
2020-04-28 16:03             ` Junio C Hamano
2020-04-28 21:14               ` Carlo Marcelo Arenas Belón
2020-04-28 21:17                 ` Junio C Hamano
2020-04-28 10:48     ` [PATCH v4 0/4] credential-store: prevent fatal errors Carlo Marcelo Arenas Belón
2020-04-28 10:52       ` [PATCH v4 1/4] credential-store: document the file format a bit more Carlo Marcelo Arenas Belón
2020-04-28 10:52         ` [PATCH v4 2/4] git-credential-store: skip empty lines and comments from store Carlo Marcelo Arenas Belón
2020-04-28 16:09           ` Eric Sunshine
2020-04-28 16:42             ` Carlo Marcelo Arenas Belón
2020-04-28 10:52         ` [PATCH v4 3/4] git-credential-store: fix (WIP) Carlo Marcelo Arenas Belón
2020-04-28 16:11           ` Eric Sunshine
2020-04-28 17:14             ` Carlo Marcelo Arenas Belón
2020-04-28 10:52         ` [PATCH v4 4/4] credential-store: make sure there is no regression with missing scheme Carlo Marcelo Arenas Belón
2020-04-28 16:06         ` [PATCH v4 1/4] credential-store: document the file format a bit more Eric Sunshine
2020-04-28 18:18           ` Junio C Hamano
2020-04-28 18:15         ` Junio C Hamano
2020-04-29  0:33       ` [PATCH v5] credential-store: warn instead of fatal for bogus lines from store Carlo Marcelo Arenas Belón
2020-04-29  4:36         ` Junio C Hamano
2020-04-29  7:31           ` Carlo Marcelo Arenas Belón
2020-04-29 16:46             ` Junio C Hamano
2020-04-29 20:35         ` [RFC PATCH v6 0/2] credential-store: prevent fatal errors Carlo Marcelo Arenas Belón
2020-04-29 20:35           ` [RFC PATCH v6 1/2] credential-store: warn instead of fatal for bogus lines from store Carlo Marcelo Arenas Belón
2020-04-29 21:05             ` Junio C Hamano
2020-04-29 21:17               ` Junio C Hamano
2020-04-29 20:35           ` [RFC PATCH v6 2/2] credential-store: warn for any incomplete credentials instead of using Carlo Marcelo Arenas Belón
2020-04-29 21:12             ` Junio C Hamano
2020-04-29 21:49               ` [RFC PATCH v6 2/2] credential-store: warn for any incomplete credentials instead of usingy Carlo Marcelo Arenas Belón
2020-04-29 22:04                 ` Junio C Hamano
2020-04-29 23:23           ` [PATCH v6] credential-store: warn instead of fatal for bogus lines from store Carlo Marcelo Arenas Belón
2020-04-29 23:47             ` Junio C Hamano
2020-04-29 23:57               ` Junio C Hamano
2020-04-30  1:00               ` Carlo Marcelo Arenas Belón
2020-04-30  1:19             ` [PATCH v7] " Carlo Marcelo Arenas Belón
2020-04-30  9:29               ` [PATCH v8] " Carlo Marcelo Arenas Belón
2020-04-30 16:06               ` [PATCH v9] " Carlo Marcelo Arenas Belón
2020-04-30 20:21                 ` Junio C Hamano
2020-04-30 21:14                   ` Junio C Hamano
2020-05-01  0:30                   ` Carlo Marcelo Arenas Belón
2020-05-01  1:40                     ` Junio C Hamano
2020-05-01  2:24                       ` Carlo Arenas
2020-05-01  5:27                         ` Junio C Hamano
2020-05-01 13:57                           ` Carlo Marcelo Arenas Belón
2020-05-01 18:59                             ` Junio C Hamano
2020-05-01  3:21                 ` [RFC PATCH v10] credential-store: warn/ignore for bogus lines from store file Carlo Marcelo Arenas Belón
2020-05-01  5:18                   ` [RFC PATCH v10 2/1] credential-store: warn also for store and erase Carlo Marcelo Arenas Belón
2020-05-01  5:35                     ` Junio C Hamano
2020-05-02 18:16                 ` [PATCH v10] credential-store: ignore bogus lines from store file Carlo Marcelo Arenas Belón
2020-05-02 20:47                   ` Junio C Hamano
2020-05-02 21:23                     ` Carlo Marcelo Arenas Belón
2020-05-02 21:53                     ` Carlo Marcelo Arenas Belón
2020-05-03  0:44                       ` Junio C Hamano
2020-05-03 10:06                     ` Jeff King
2020-05-02 21:05                   ` Carlo Marcelo Arenas Belón
2020-05-02 22:34                   ` [PATCH v11] " Carlo Marcelo Arenas Belón

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20200427125915.88667-1-carenas@gmail.com \
    --to=carenas@gmail.com \
    --cc=dirk@ed4u.de \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.