[PATCH] fetch: Print full url in header

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
---

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@xxxxxx>
	> 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@xxxxxx>
	>     Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>

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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]