From: "Carlo Marcelo Arenas Belón" <carenas@gmail.com>
To: gitster@pobox.com
Cc: git@vger.kernel.org, "Carlo Marcelo Arenas Belón" <carenas@gmail.com>
Subject: [RFC PATCH v10 2/1] credential-store: warn also for store and erase
Date: Thu, 30 Apr 2020 22:18:47 -0700 [thread overview]
Message-ID: <20200501051847.31803-1-carenas@gmail.com> (raw)
In-Reply-To: <20200501032152.12160-1-carenas@gmail.com>
done in a hacky way and only as a POC, but at least works, if we
are still thinking that warning should be at least comprehensive
for the first iteration.
didn't change the callback as you suggested but I think this
doesn't look that bad either.
squash for final v10 or ..?
---
Documentation/git-credential-store.txt | 7 -------
credential-store.c | 19 +++++++++----------
t/t0302-credential-store.sh | 20 +++++++++++++++++---
3 files changed, 26 insertions(+), 20 deletions(-)
diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt
index 18ba8b6984..d5841cffad 100644
--- a/Documentation/git-credential-store.txt
+++ b/Documentation/git-credential-store.txt
@@ -105,13 +105,6 @@ like spaces, line wrapping or text encoding.
An unparseable or otherwise invalid line is ignored, and a warning
message points out the problematic file and line number it appears in.
-Note that because of deficiencies of the current implementation, these
-warnings will be only generated for the entries that were processed
-while reading the credential files during a get operation, and so they
-are not comprehensive, and will require you to use an editor (or
-better a sed/awk/perl one liner) to edit the credential file and remove
-them.
-
When Git needs authentication for a particular URL context,
credential-store will consider that context a pattern to match against
each entry in the credentials file. If the protocol, hostname, and
diff --git a/credential-store.c b/credential-store.c
index c2778eff8a..236df8e7bf 100644
--- a/credential-store.c
+++ b/credential-store.c
@@ -4,8 +4,6 @@
#include "string-list.h"
#include "parse-options.h"
-#define PARSE_VERBOSE 0x01
-
static struct lock_file credential_lock;
static int valid_credential(struct credential *entry)
@@ -17,7 +15,7 @@ static int valid_credential(struct credential *entry)
static int parse_credential_file(const char *fn,
struct credential *c,
- int flags,
+ int initial_line,
void (*match_cb)(struct credential *),
void (*other_cb)(struct strbuf *))
{
@@ -25,7 +23,7 @@ static int parse_credential_file(const char *fn,
struct strbuf line = STRBUF_INIT;
struct credential entry = CREDENTIAL_INIT;
int found_credential = 0;
- int lineno = 0;
+ int lineno = initial_line;
fh = fopen(fn, "r");
if (!fh) {
@@ -40,8 +38,7 @@ static int parse_credential_file(const char *fn,
if (strchr(line.buf, '\r') ||
credential_from_url_gently(&entry, line.buf, 1) ||
!valid_credential(&entry)) {
- if (flags & PARSE_VERBOSE)
- warning(_("%s:%d: ignoring invalid credential"),
+ warning(_("%s:%d: ignoring invalid credential"),
fn, lineno);
} else if (credential_match(c, &entry)) {
found_credential = 1;
@@ -77,11 +74,14 @@ static void print_line(struct strbuf *buf)
static void rewrite_credential_file(const char *fn, struct credential *c,
struct strbuf *extra)
{
+ int initial_line = 0;
if (hold_lock_file_for_update(&credential_lock, fn, 0) < 0)
die_errno("unable to get credential storage lock");
- if (extra)
+ if (extra) {
print_line(extra);
- parse_credential_file(fn, c, 0, NULL, print_line);
+ initial_line++;
+ }
+ parse_credential_file(fn, c, initial_line, NULL, print_line);
if (commit_lock_file(&credential_lock) < 0)
die_errno("unable to write credential store");
}
@@ -158,8 +158,7 @@ static void lookup_credential(const struct string_list *fns, struct credential *
struct string_list_item *fn;
for_each_string_list_item(fn, fns)
- if (parse_credential_file(fn->string, c, PARSE_VERBOSE,
- print_entry, NULL))
+ if (parse_credential_file(fn->string, c, 0, print_entry, NULL))
return; /* Found credential */
}
diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
index 00608e365a..78010590f4 100755
--- a/t/t0302-credential-store.sh
+++ b/t/t0302-credential-store.sh
@@ -218,13 +218,27 @@ test_expect_success 'get: store file can contain empty/bogus lines' '
test_line_count = 3 stderr
'
-test_expect_success 'store: store succeeds silently in corrupted file' '
+test_expect_success 'store: succeeds in corrupted file but correctly warn' '
echo "#comment" >"$HOME/.git-credentials" &&
- check approve store <<-\EOF &&
+ test_config credential.helper store &&
+ git credential approve <<-\EOF &&
url=https://user:pass@example.com
EOF
grep "https://user:pass@example.com" "$HOME/.git-credentials" &&
- test_must_be_empty stderr
+ test_i18ngrep "git-credentials:2: ignoring invalid credential" stderr
+'
+
+test_expect_success 'erase: succeeds in corrupted file but correctly warn' '
+ cat <<-\CONFIG >"$HOME/.git-credentials" &&
+ #comment
+ https://user:pass@example.com
+ CONFIG
+ test_config credential.helper store &&
+ git credential reject <<-\EOF &&
+ url=https://example.com
+ EOF
+ ! grep "https://user:pass@example.com" "$HOME/.git-credentials" &&
+ test_i18ngrep "git-credentials:1: ignoring invalid credential" stderr
'
test_done
--
2.26.2.3.g90bd2f7136
next prev parent reply other threads:[~2020-05-01 5:19 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 ` [PATCH v3] " Carlo Marcelo Arenas Belón
2020-04-27 13:48 ` 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 ` Carlo Marcelo Arenas Belón [this message]
2020-05-01 5:35 ` [RFC PATCH v10 2/1] credential-store: warn also for store and erase 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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200501051847.31803-1-carenas@gmail.com \
--to=carenas@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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).