git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jon Seymour <jon.seymour@gmail.com>
To: git@vger.kernel.org
Cc: Jens.Lehmann@web.de, gitster@pobox.com, phil.hord@gmail.com,
	ramsay@ramsay1.demon.co.uk, Jon Seymour <jon.seymour@gmail.com>
Subject: [PATCH v7 6/9] submodule: fix detection of invalid submodule URL
Date: Mon, 28 May 2012 01:34:08 +1000	[thread overview]
Message-ID: <1338132851-23497-7-git-send-email-jon.seymour@gmail.com> (raw)
In-Reply-To: <1338132851-23497-1-git-send-email-jon.seymour@gmail.com>

Currently the superproject origin URL is progressively transformed
by stepping through parts of the submodule URL and removing parts
from the superproject URL for each leading ../ found in the
submodule URL. No attempt is made to check that the edited URL still
has a path part left to remove. This can result in the construction
of an absolute submodule URL where the hostname part of the URL
has been replaced by path components of the submodule URL.

For example: if the origin URL is ssh://hostname/repo and the
submodule URL is ../../subrepo, then the origin URL of the subrepo
will be calculated as ssh://subrepo.

With this change, editing is only performed on the path part
of the superproject origin URL. Any attempt by to consume the
non-path parts of the origin URL results in a failure.

As a side effect of preserving correct handling of support for
URLs of the form user@host:repo, this change also fixes handling,
by submodule init, of origin super project URLs of the form:
foo, foo/bar, ./foo and ./foo/bar.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 git-submodule.sh           | 41 ++++++++++++++++++++++++++++++++---------
 t/t7400-submodule-basic.sh | 23 ++++++++---------------
 2 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index dbbc905..2550681 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -37,23 +37,42 @@ resolve_relative_url ()
 	remoteurl=$(git config "remote.$remote.url") ||
 		remoteurl=$(pwd) # the repository is its own authoritative upstream
 	url="$1"
-	remoteurl=${remoteurl%/}
-	sep=/
+	remoteurl="${remoteurl%/}"
+
+	case "$remoteurl" in
+		*//*/*)
+			variant="${remoteurl#*//*/}"
+		;;
+		*::*)
+			variant="${remoteurl#*::}"
+		;;
+		*:*)
+			variant="${remoteurl#*:}"
+		;;
+		/*)
+			variant="${remoteurl#/}"
+		;;
+		*)
+			variant="${remoteurl}"
+		;;
+	esac
+	invariant="${remoteurl%$variant}"
+
 	while test -n "$url"
 	do
 		case "$url" in
 		../*)
 			url="${url#../}"
-			case "$remoteurl" in
+			case "$variant" in
 			*/*)
-				remoteurl="${remoteurl%/*}"
+				variant="${variant%/*}"
 				;;
-			*:*)
-				remoteurl="${remoteurl%:*}"
-				sep=:
+			.)
+				die "$(eval_gettext "cannot strip one component off url '\${invariant}'")"
 				;;
 			*)
-				die "$(eval_gettext "cannot strip one component off url '\$remoteurl'")"
+				# add a sentinel when .. matchs foo
+				variant=.
 				;;
 			esac
 			;;
@@ -64,7 +83,11 @@ resolve_relative_url ()
 			break;;
 		esac
 	done
-	echo "$remoteurl$sep${url%/}"
+	# ensure a trailing path separator
+	variant="${variant}/"
+	# strip the sentinel, if present
+	variant="${variant#./}"
+	echo "$invariant$variant${url%/}"
 }
 
 #
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 2674088..a94c5e9 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -576,7 +576,7 @@ test_expect_success '../subrepo works with helper URL- helper:://hostname/repo'
 	)
 '
 
-test_expect_failure '../../subrepo fails with URL - ssh://hostname/repo' "
+test_expect_success '../../subrepo fails with URL - ssh://hostname/repo' "
 	(
 		cd reltest &&
 		cp pristine-.git-config .git/config &&
@@ -585,12 +585,11 @@ test_expect_failure '../../subrepo fails with URL - ssh://hostname/repo' "
 		git config -f .gitmodules submodule.sub.url ../../subrepo &&
 		echo cannot strip one component off url \'ssh://hostname/\' > expected &&
 		test_must_fail git submodule init 2>actual &&
-		#actual no failure, url configured as ssh://subrepo
 		test_cmp expected actual
 	)
 "
 
-test_expect_failure '../../subrepo fails with absolute local path - /repo' "
+test_expect_success '../../subrepo fails with absolute local path - /repo' "
 	(
 		cd reltest &&
 		cp pristine-.git-config .git/config &&
@@ -603,7 +602,7 @@ test_expect_failure '../../subrepo fails with absolute local path - /repo' "
 	)
 "
 
-test_expect_failure '../../../subrepo fails with URL - ssh://hostname/repo' "
+test_expect_success '../../../subrepo fails with URL - ssh://hostname/repo' "
 	(
 		cd reltest &&
 		cp pristine-.git-config .git/config &&
@@ -612,12 +611,11 @@ test_expect_failure '../../../subrepo fails with URL - ssh://hostname/repo' "
 		git config -f .gitmodules submodule.sub.url ../../../subrepo &&
 		echo cannot strip one component off url \'ssh://hostname/\' > expected &&
 		test_must_fail git submodule init 2>actual &&
-		#actual no failure, url configured as ssh:/subrepo
 		test_cmp expected actual
 	)
 "
 
-test_expect_failure '../../../../subrepo fails with with URL - ssh://hostname/repo' "
+test_expect_success '../../../../subrepo fails with with URL - ssh://hostname/repo' "
 	(
 		cd reltest &&
 		cp pristine-.git-config .git/config &&
@@ -626,12 +624,11 @@ test_expect_failure '../../../../subrepo fails with with URL - ssh://hostname/re
 		git config -f .gitmodules submodule.sub.url ../../../../subrepo &&
 		echo cannot strip one component off url \'ssh://hostname/\' > expected &&
 		test_must_fail git submodule init 2>actual &&
-		#actual no failure, url configured as ssh:/subrepo
 		test_cmp expected actual
 	)
 "
 
-test_expect_failure '../../../../../subrepo fails with URL - ssh://hostname/repo' "
+test_expect_success '../../../../../subrepo fails with URL - ssh://hostname/repo' "
 	(
 		cd reltest &&
 		cp pristine-.git-config .git/config &&
@@ -640,7 +637,6 @@ test_expect_failure '../../../../../subrepo fails with URL - ssh://hostname/repo
 		git config -f .gitmodules submodule.sub.url ../../../../../subrepo &&
 		echo cannot strip one component off url \'ssh://hostname/\' > expected &&
 		test_must_fail git submodule init 2>actual &&
-		#actual cannot strip one component off url 'ssh'
 		test_cmp expected actual
 	)
 "
@@ -711,13 +707,12 @@ test_expect_failure 'relative path works with user@host:path/to/detour/../repo'
 	)
 '
 
-test_expect_failure '../subrepo works with relative local path - foo' '
+test_expect_success '../subrepo works with relative local path - foo' '
 	(
 		cd reltest &&
 		cp pristine-.git-config .git/config &&
 		cp pristine-.gitmodules .gitmodules &&
 		git config remote.origin.url foo &&
-		# actual: fails with an error
 		git submodule init &&
 		test "$(git config submodule.sub.url)" = subrepo
 	)
@@ -734,26 +729,24 @@ test_expect_success '../subrepo works with relative local path - foo/bar' '
 	)
 '
 
-test_expect_failure '../subrepo works with relative local path - ./foo' '
+test_expect_success '../subrepo works with relative local path - ./foo' '
 	(
 		cd reltest &&
 		cp pristine-.git-config .git/config &&
 		cp pristine-.gitmodules .gitmodules &&
 		git config remote.origin.url ./foo &&
 		git submodule init &&
-		#actual ./subrepo
 		test "$(git config submodule.sub.url)" = subrepo
 	)
 '
 
-test_expect_failure '../subrepo works with relative local path - ./foo/bar' '
+test_expect_success '../subrepo works with relative local path - ./foo/bar' '
 	(
 		cd reltest &&
 		cp pristine-.git-config .git/config &&
 		cp pristine-.gitmodules .gitmodules &&
 		git config remote.origin.url ./foo/bar &&
 		git submodule init &&
-		#actual: ./foo/subrepo
 		test "$(git config submodule.sub.url)" = foo/subrepo
 	)
 '
-- 
1.7.10.2.656.g24a6219

  parent reply	other threads:[~2012-05-27 15:36 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-27 15:34 [PATCH v7 0/9] submodule: improve robustness of path handling Jon Seymour
2012-05-27 15:34 ` [PATCH v7 1/9] submodule: additional regression tests for relative URLs Jon Seymour
2012-05-27 15:34 ` [PATCH v7 2/9] submodule: document failure to detect invalid submodule URLs Jon Seymour
2012-05-27 15:34 ` [PATCH v7 3/9] submodule: document failure to handle relative superproject origin URLs Jon Seymour
2012-05-27 15:34 ` [PATCH v7 4/9] submodule: document failure to handle improperly normalized remote " Jon Seymour
2012-05-27 15:34 ` [PATCH v7 5/9] submodule: extract normalize_path into standalone function Jon Seymour
2012-05-27 15:34 ` Jon Seymour [this message]
2012-05-28 19:01   ` [PATCH v7 6/9] submodule: fix detection of invalid submodule URL Johannes Sixt
2012-05-28 21:39     ` Jon Seymour
2012-06-03  9:51       ` Jon Seymour
2012-05-27 15:34 ` [PATCH v7 7/9] submodule: fix sync handling of relative superproject origin URLs Jon Seymour
2012-05-27 15:34 ` [PATCH v7 8/9] submodule: fix handling of denormalized " Jon Seymour
2012-05-27 22:57   ` Jon Seymour
2012-05-27 15:34 ` [PATCH v7 9/9] submodule: fix normalization to handle repeated ./ Jon Seymour
2012-05-28 20:07 ` [PATCH v7 0/9] submodule: improve robustness of path handling Jens Lehmann
2012-05-28 22:01   ` Jon Seymour
2012-05-29 19:21     ` Jens Lehmann

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=1338132851-23497-7-git-send-email-jon.seymour@gmail.com \
    --to=jon.seymour@gmail.com \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phil.hord@gmail.com \
    --cc=ramsay@ramsay1.demon.co.uk \
    /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).