Cleanup: message reporting for HTTP redirect handling

The addition of "tooManyRedirects" in commit 7ac1bfc ("Do
authentication re-tries on HTTP POST") was an error I didn't
catch after rebasing that change. That message had been renamed
in the earlier commit e17bfc9 ("Add support to follow HTTP
redirects") to "redirectLimitExceeded".

Also make sure we always use the TransportException(URIish, ...)
constructor; it'll prefix the message given with the sanitized URI.
Change messages to remove the explicit mention of that URI inside the
message. Adapt tests that check the expected exception message text.

For the info logging of redirects, remove a potentially present
password component in the URI to avoid leaking it into the log.

Change-Id: I517112404757a9a947e92aaace743c6541dce6aa
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

4 files changed, 28 insertions, 25 deletions

diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java
index 1b0c6949a9..51b79903c6 100644
--- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java
+++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java
@@ -595,9 +595,9 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 			t.fetch(NullProgressMonitor.INSTANCE, mirror(master));
 			fail("Should have failed (too many redirects)");
 		} catch (TransportException e) {
-			String expectedMessageBegin = MessageFormat.format(
-					JGitText.get().redirectLimitExceeded, remoteUri, "3",
-					remoteUri.replace("/4/", "/1/") + '/', "");
+			String expectedMessageBegin = remoteUri.toString() + ": "
+					+ MessageFormat.format(JGitText.get().redirectLimitExceeded,
+							"3", remoteUri.replace("/4/", "/1/") + '/', "");
 			String message = e.getMessage();
 			if (message.length() > expectedMessageBegin.length()) {
 				message = message.substring(0, expectedMessageBegin.length());
@@ -616,7 +616,7 @@ public class SmartClientSmartServerTest extends HttpTestCase {
 			t.fetch(NullProgressMonitor.INSTANCE, mirror(master));
 			fail("Should have failed (redirect loop)");
 		} catch (TransportException e) {
-			assertTrue(e.getMessage().contains("redirected more than"));
+			assertTrue(e.getMessage().contains("Redirected more than"));
 		}
 	}
 
diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index c532b328db..8d3931493c 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -365,7 +365,7 @@ invalidPathContainsSeparator=Invalid path (contains separator ''{0}''): {1}
 invalidPathPeriodAtEndWindows=Invalid path (period at end is ignored by Windows): {0}
 invalidPathSpaceAtEndWindows=Invalid path (space at end is ignored by Windows): {0}
 invalidPathReservedOnWindows=Invalid path (''{0}'' is reserved on Windows): {1}
-invalidRedirectLocation=Invalid redirect location {1} -> {2}
+invalidRedirectLocation=Invalid redirect location {0} -> {1}
 invalidReflogRevision=Invalid reflog revision: {0}
 invalidRefName=Invalid ref name: {0}
 invalidReftableBlock=Invalid reftable block
@@ -535,11 +535,11 @@ receivePackObjectTooLarge2=Object too large ({0} bytes), rejecting the pack. Max
 receivePackInvalidLimit=Illegal limit parameter value {0}
 receivePackTooLarge=Pack exceeds the limit of {0} bytes, rejecting the pack
 receivingObjects=Receiving objects
-redirectBlocked=URI ''{0}'' redirection blocked: redirect {1} -> {2} not allowed
+redirectBlocked=Redirection blocked: redirect {0} -> {1} not allowed
 redirectHttp=URI ''{0}'': following HTTP redirect #{1}  {2} -> {3}
-redirectLimitExceeded=URI ''{0}'' redirected more than {1} times; aborted at {2} -> {3}
-redirectLocationMissing=Invalid redirect of ''{0}'': no redirect location for {1}
-redirectsOff=Cannot redirect ''{0}'': http.followRedirects is false (HTTP status {1})
+redirectLimitExceeded=Redirected more than {0} times; aborted at {1} -> {2}
+redirectLocationMissing=Invalid redirect: no redirect location for {0}
+redirectsOff=Cannot redirect because http.followRedirects is false (HTTP status {0})
 refAlreadyExists=already exists
 refAlreadyExists1=Ref {0} already exists
 reflogEntryNotFound=Entry {0} not found  in reflog for ''{1}''
@@ -636,7 +636,6 @@ timeIsUncertain=Time is uncertain
 timerAlreadyTerminated=Timer already terminated
 tooManyCommands=Too many commands
 tooManyIncludeRecursions=Too many recursions; circular includes in config file(s)?
-tooManyRedirects=Too many redirects; stopped after {0} redirects at ''{1}''
 topologicalSortRequired=Topological sort required.
 transactionAborted=transaction aborted
 transportExceptionBadRef=Empty ref: {0}: {1}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index 5acf058764..07666eb934 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -696,7 +696,6 @@ public class JGitText extends TranslationBundle {
 	/***/ public String timerAlreadyTerminated;
 	/***/ public String tooManyCommands;
 	/***/ public String tooManyIncludeRecursions;
-	/***/ public String tooManyRedirects;
 	/***/ public String topologicalSortRequired;
 	/***/ public String transportExceptionBadRef;
 	/***/ public String transportExceptionEmptyRef;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java
index 1647200e65..26612120c1 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/TransportHttp.java
@@ -604,7 +604,8 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
 
 				case HttpConnection.HTTP_FORBIDDEN:
 					throw new TransportException(uri, MessageFormat.format(
-							JGitText.get().serviceNotPermitted, service));
+							JGitText.get().serviceNotPermitted, baseUrl,
+							service));
 
 				case HttpConnection.HTTP_MOVED_PERM:
 				case HttpConnection.HTTP_MOVED_TEMP:
@@ -614,9 +615,10 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
 					// and in general should occur only on POST requests. But it
 					// doesn't hurt to accept it here as a redirect.
 					if (http.followRedirects == HttpRedirectMode.FALSE) {
-						throw new TransportException(MessageFormat.format(
-								JGitText.get().redirectsOff, uri,
-								Integer.valueOf(status)));
+						throw new TransportException(uri,
+								MessageFormat.format(
+										JGitText.get().redirectsOff,
+										Integer.valueOf(status)));
 					}
 					URIish newUri = redirect(conn.getHeaderField(HDR_LOCATION),
 							Constants.INFO_REFS, redirects++);
@@ -655,31 +657,34 @@ public class TransportHttp extends HttpTransport implements WalkTransport,
 	private URIish redirect(String location, String checkFor, int redirects)
 			throws TransportException {
 		if (location == null || location.isEmpty()) {
-			throw new TransportException(MessageFormat.format(
-					JGitText.get().redirectLocationMissing, uri, baseUrl));
+			throw new TransportException(uri,
+					MessageFormat.format(JGitText.get().redirectLocationMissing,
+							baseUrl));
 		}
 		if (redirects >= http.maxRedirects) {
-			throw new TransportException(MessageFormat.format(
-					JGitText.get().redirectLimitExceeded, uri,
+			throw new TransportException(uri,
+					MessageFormat.format(JGitText.get().redirectLimitExceeded,
 					Integer.valueOf(http.maxRedirects), baseUrl, location));
 		}
 		try {
 			if (!isValidRedirect(baseUrl, location, checkFor)) {
-				throw new TransportException(
+				throw new TransportException(uri,
 						MessageFormat.format(JGitText.get().redirectBlocked,
-								uri, baseUrl, location));
+								baseUrl, location));
 			}
 			location = location.substring(0, location.indexOf(checkFor));
 			URIish result = new URIish(location);
 			if (LOG.isInfoEnabled()) {
-				LOG.info(MessageFormat.format(JGitText.get().redirectHttp, uri,
+				LOG.info(MessageFormat.format(JGitText.get().redirectHttp,
+						uri.setPass(null),
 						Integer.valueOf(redirects), baseUrl, result));
 			}
 			return result;
 		} catch (URISyntaxException e) {
-			throw new TransportException(MessageFormat.format(
-					JGitText.get().invalidRedirectLocation,
-					uri, baseUrl, location), e);
+			throw new TransportException(uri,
+					MessageFormat.format(JGitText.get().invalidRedirectLocation,
+							baseUrl, location),
+					e);
 		}
 	}