From: Tom Miller <jackerran@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, Tom Miller <jackerran@gmail.com>
Subject: [PATCH] fetch: Print full url in header
Date: Wed, 8 Jan 2014 20:39:18 -0600 [thread overview]
Message-ID: <1389235158-21902-1-git-send-email-jackerran@gmail.com> (raw)
Do not remove "/" and ".git" from the end of the header url when
fetching. This affects the output of "fetch" and "fetch --prune"
making the header url more consistent with "remote --verbose".
Add tests to verify that "fetch" and "fetch --prune" do not strip the
trailing characters from the header url.
Output before this patch:
$ git fetch remote-with-dot-git-and-slash
From https://github.com/git/git
a155a5f..5512ac5 maint -> upstream/maint
Output after this patch:
$ git fetch remote-with-dot-git-and-slash
From https://github.com/git/git.git/
a155a5f..5512ac5 maint -> upstream/maint
Signed-off-by: Tom Miller <jackerran@gmail.com>
---
This patch should be based on the tip of "next" because it is dependent
on the code from "tm/fetch-prune".
Initially I thought I would stop anonymizing the header url as well.
Then I ran across this commit.
> commit 47abd85ba06ed7209d1caa3e5ac7cc6b232bece4
> Author: Andreas Ericsson <ae@op5.se>
> Date: Fri Apr 17 10:20:11 2009 +0200
>
> fetch: Strip usernames from url's before storing them
>
> When pulling from a remote, the full URL including username
> is by default added to the commit message. Since it adds
> very little value but could be used by malicious people to
> glean valid usernames (with matching hostnames), we're far
> better off just stripping the username before storing the
> remote URL locally.
>
> Note that this patch has no lasting visible effect when
> "git pull" does not create a merge commit. It simply
> alters what gets written to .git/FETCH_HEAD, which is used
> by "git merge" to automagically create its messages.
>
> Signed-off-by: Andreas Ericsson <ae@op5.se>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
After reading this and trying different things with the code. I believe
it would make sense to continue to anonymize the url for output to the
terminal. I found if someone is using HTTP basic authentication and has
the username and password in the url. Then, one could unexpectedly
compromise their credentials during a fetch. I do not believe that
anyone should store their credentials in this way, but it is better safe
than sorry.
I also chose to continue to strip the trailing characters for the
FETCH_HEAD file. I wanted the input of the mailing list to see if we
should also stop striping the trailing characters off of the url written
to FETCH_HEAD? If so, I'll do it as a seperate patch.
builtin/fetch.c | 15 +++------------
t/t5510-fetch.sh | 29 ++++++++++++++++++++++++-----
2 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 025bc3e..01df749 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -664,8 +664,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
*what ? what : "HEAD");
if (note.len) {
if (verbosity >= 0 && !shown_url) {
- fprintf(stderr, _("From %.*s\n"),
- url_len, url);
+ fprintf(stderr, _("From %s\n"), url);
shown_url = 1;
}
if (verbosity >= 0)
@@ -723,7 +722,7 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)
static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
const char *raw_url)
{
- int url_len, i, result = 0;
+ int result = 0;
struct ref *ref, *stale_refs = get_stale_heads(refs, ref_count, ref_map);
char *url;
const char *dangling_msg = dry_run
@@ -735,19 +734,11 @@ static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
else
url = xstrdup("foreign");
- url_len = strlen(url);
- for (i = url_len - 1; url[i] == '/' && 0 <= i; i--)
- ;
-
- url_len = i + 1;
- if (4 < i && !strncmp(".git", url + i - 3, 4))
- url_len = i - 3;
-
for (ref = stale_refs; ref; ref = ref->next) {
if (!dry_run)
result |= delete_ref(ref->name, NULL, 0);
if (verbosity >= 0 && !shown_url) {
- fprintf(stderr, _("From %.*s\n"), url_len, url);
+ fprintf(stderr, _("From %s\n"), url);
shown_url = 1;
}
if (verbosity >= 0) {
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 12674ac..882bfa1 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -614,15 +614,34 @@ test_expect_success 'all boundary commits are excluded' '
test_bundle_object_count .git/objects/pack/pack-${pack##pack }.pack 3
'
-test_expect_success 'fetch --prune prints the remotes url' '
+test_expect_success 'fetch prints the remotes full url' '
+ git clone . fetch-url-repo.git &&
+ git clone --mirror fetch-url-repo.git/ fetch-full-url &&
+ (
+ cd fetch-url-repo.git &&
+ git branch hello
+ ) &&
+ (
+ cd fetch-full-url &&
+ git fetch origin 2>&1 | head -n1 >../actual
+ ) &&
+ echo "From ${D}/fetch-url-repo.git/" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'fetch --prune prints the remotes full url' '
git branch goodbye &&
- git clone . only-prunes &&
- git branch -D goodbye &&
+ git clone --mirror . fetch-prune-url-repo.git &&
+ git clone fetch-prune-url-repo.git/ fetch-prune-full-url &&
+ (
+ cd fetch-prune-url-repo.git &&
+ git branch -D goodbye
+ ) &&
(
- cd only-prunes &&
+ cd fetch-prune-full-url &&
git fetch --prune origin 2>&1 | head -n1 >../actual
) &&
- echo "From ${D}/." >expect &&
+ echo "From ${D}/fetch-prune-url-repo.git/" >expect &&
test_cmp expect actual
'
--
1.8.5.2.374.ga7e4b90
next reply other threads:[~2014-01-09 2:40 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-09 2:39 Tom Miller [this message]
2014-01-09 20:07 ` [PATCH] fetch: Print full url in header Junio C Hamano
2014-01-10 3:03 ` Tom Miller
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=1389235158-21902-1-git-send-email-jackerran@gmail.com \
--to=jackerran@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.
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 NNTP newsgroup(s).