Make Repository.normalizeBranchName less strict

This operation was added recently with the goal to provide some
way to auto-correct invalid user input, or to provide a correction
suggestion to the user -- EGit uses it now that way. But the initial
implementation was very restrictive; it removed all non-ASCII
characters and even slashes.

Understandably end users were not happy with that. Git has no such
restriction to ASCII-only; nor does JGit. Branch names should be
meaningful to the end user, and if a user-supplied branch name is
invalid for technical reasons, a "normalized" name should still
be meaningful to the user.

Rewrite to attempt a minimal fix such that the result will pass
isValidRefName.

* Replace all Unicode whitespace by underscore.
* Replace troublesome special characters by dash.
* Collapse sequences of underscores, dots, and dashes.
* Remove underscores, dots, and dashes following slashes, and
  collapse sequences of slashes.
* Strip leading and trailing sequences of slashes, dots, dashes,
  and underscores.
* Avoid the ".lock" extension.
* Avoid the Windows reserved device names.
* If input name is null return an empty String so callers don't need to
check for null.

This still allows branch names with single slashes as separators
between components, avoids some pitfalls that isValidRefName() tests
for, and leaves other character untouched and thus allows non-ASCII
branch names.

Also move the function from the bottom of the file up to where
isValidRefName is implemented.

Bug: 512508
Change-Id: Ia0576d9b2489162208c05e51c6d54e9f0c88c3a7
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

1 files changed, 61 insertions, 82 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ValidRefNameTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ValidRefNameTest.java
index f069b65f66..d431a8910b 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ValidRefNameTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ValidRefNameTest.java
@@ -44,6 +44,7 @@
 package org.eclipse.jgit.lib;
 
 import static org.eclipse.jgit.junit.Assert.assertEquals;
+import static org.junit.Assert.assertEquals;
 
 import org.eclipse.jgit.junit.MockSystemReader;
 import org.eclipse.jgit.util.SystemReader;
@@ -94,6 +95,25 @@ public class ValidRefNameTest {
 		}
 	}
 
+	private static void assertNormalized(final String name,
+			final String expected) {
+		SystemReader instance = SystemReader.getInstance();
+		try {
+			setUnixSystemReader();
+			String normalized = Repository.normalizeBranchName(name);
+			assertEquals("Normalization of " + name, expected, normalized);
+			assertEquals("\"" + normalized + "\"", true,
+					Repository.isValidRefName(Constants.R_HEADS + normalized));
+			setWindowsSystemReader();
+			normalized = Repository.normalizeBranchName(name);
+			assertEquals("Normalization of " + name, expected, normalized);
+			assertEquals("\"" + normalized + "\"", true,
+					Repository.isValidRefName(Constants.R_HEADS + normalized));
+		} finally {
+			SystemReader.setInstance(instance);
+		}
+	}
+
 	@Test
 	public void testEmptyString() {
 		assertValid(false, "");
@@ -250,97 +270,56 @@ public class ValidRefNameTest {
 
 	@Test
 	public void testNormalizeBranchName() {
-
+		assertEquals(true, Repository.normalizeBranchName(null) == "");
 		assertEquals(true, Repository.normalizeBranchName("").equals(""));
+		assertNormalized("Bug 12345::::Hello World", "Bug_12345-Hello_World");
+		assertNormalized("Bug 12345 :::: Hello World", "Bug_12345_Hello_World");
+		assertNormalized("Bug 12345 :::: Hello::: World",
+				"Bug_12345_Hello-World");
+		assertNormalized(":::Bug 12345 - Hello World", "Bug_12345_Hello_World");
+		assertNormalized("---Bug 12345 - Hello World", "Bug_12345_Hello_World");
+		assertNormalized("Bug 12345 ---- Hello --- World",
+				"Bug_12345_Hello_World");
+		assertNormalized("Bug 12345 - Hello World!", "Bug_12345_Hello_World!");
+		assertNormalized("Bug 12345 : Hello World!", "Bug_12345_Hello_World!");
+		assertNormalized("Bug 12345 _ Hello World!", "Bug_12345_Hello_World!");
+		assertNormalized("Bug 12345   -       Hello World!",
+				"Bug_12345_Hello_World!");
+		assertNormalized(" Bug 12345   -   Hello World! ",
+				"Bug_12345_Hello_World!");
+		assertNormalized(" Bug 12345   -   Hello World!   ",
+				"Bug_12345_Hello_World!");
+		assertNormalized("Bug 12345   -   Hello______ World!",
+				"Bug_12345_Hello_World!");
+		assertNormalized("_Bug 12345 - Hello World!", "Bug_12345_Hello_World!");
+	}
 
-		assertEquals(true,
-				Repository.normalizeBranchName("__@#$@#$@$____   _")
-						.equals(""));
-
-		assertEquals(true,
-				Repository.normalizeBranchName("~`!@#$%^&*()_+}]{[|\\\";?>.<,/")
-						.equals(""));
-
-		assertEquals(true,
-				Repository.normalizeBranchName("Bug 12345 :::: Hello World")
-						.equals("Bug_12345-Hello_World"));
-
-		assertEquals(true,
-				Repository.normalizeBranchName("Bug 12345 :::: Hello::: World")
-						.equals("Bug_12345-Hello-_World"));
-
-		assertEquals(true,
-				Repository.normalizeBranchName(":::Bug 12345 - Hello World")
-						.equals("Bug_12345-Hello_World"));
-
-		assertEquals(true,
-				Repository.normalizeBranchName("---Bug 12345 - Hello World")
-						.equals("Bug_12345-Hello_World"));
-
-		assertEquals(true,
-				Repository.normalizeBranchName("Bug 12345 ---- Hello --- World")
-						.equals("Bug_12345-Hello-World"));
-
-		assertEquals(true, Repository.normalizeBranchName(null) == null);
-
-		assertEquals(true,
-				Repository.normalizeBranchName("Bug 12345 - Hello World!")
-						.equals("Bug_12345-Hello_World"));
-
-		assertEquals(true,
-				Repository.normalizeBranchName("Bug 12345 : Hello World!")
-						.equals("Bug_12345-Hello_World"));
-
-		assertEquals(true,
-				Repository.normalizeBranchName("Bug 12345 _ Hello World!")
-						.equals("Bug_12345_Hello_World"));
-
-		assertEquals(true,
-				Repository
-						.normalizeBranchName("Bug 12345   -       Hello World!")
-						.equals("Bug_12345-Hello_World"));
-
-		assertEquals(true,
-				Repository.normalizeBranchName(" Bug 12345   -   Hello World! ")
-						.equals("Bug_12345-Hello_World"));
-
-		assertEquals(true,
-				Repository
-						.normalizeBranchName(" Bug 12345   -   Hello World!   ")
-						.equals("Bug_12345-Hello_World"));
-
-		assertEquals(true,
-				Repository
-						.normalizeBranchName(
-								"Bug 12345   -   Hello______ World!")
-						.equals("Bug_12345-Hello_World"));
-
-		assertEquals(true,
-				Repository.normalizeBranchName("_Bug 12345 - Hello World!")
-						.equals("Bug_12345-Hello_World"));
-
-		assertEquals(true,
-				Repository
-						.normalizeBranchName(
-								"Bug 12345 - Hello Wo!@#$%^&*(rld {@")
-						.equals("Bug_12345-Hello_World_"));
-
-		assertEquals(true,
-				Repository.normalizeBranchName("Bug 1#$  2345 - Hello World")
-						.equals("Bug_12345-Hello_World"));
+	@Test
+	public void testNormalizeWithSlashes() {
+		assertNormalized("foo/bar/baz", "foo/bar/baz");
+		assertNormalized("foo/bar.lock/baz.lock", "foo/bar_lock/baz_lock");
+		assertNormalized("foo/.git/.git~1/bar", "foo/git/git-1/bar");
+		assertNormalized(".foo/aux/con/com3.txt/com0/prn/lpt1",
+				"foo/+aux/+con/+com3.txt/com0/+prn/+lpt1");
+		assertNormalized("foo/../bar///.--ba...z", "foo/bar/ba.z");
+	}
+
+	@Test
+	public void testNormalizeWithUnicode() {
+		assertNormalized("f\u00f6\u00f6/.b\u00e0r/*[<>|^~/b\u00e9\\z",
+				"f\u00f6\u00f6/b\u00e0r/b\u00e9-z");
+		assertNormalized("\u5165\u53e3 entrance;/.\u51fa\u53e3_*ex*it*/",
+				"\u5165\u53e3_entrance;/\u51fa\u53e3_ex-it");
 	}
 
 	@Test
 	public void testNormalizeAlreadyValidRefName() {
-		assertEquals(true,
-				Repository.normalizeBranchName("refs/heads/m.a.s.t.e.r")
-						.equals("refs/heads/m.a.s.t.e.r"));
+		assertNormalized("refs/heads/m.a.s.t.e.r", "refs/heads/m.a.s.t.e.r");
+		assertNormalized("refs/tags/v1.0-20170223", "refs/tags/v1.0-20170223");
 	}
 
 	@Test
 	public void testNormalizeTrimmedUnicodeAlreadyValidRefName() {
-		assertEquals(true,
-				Repository.normalizeBranchName(" \u00e5ngstr\u00f6m\t")
-						.equals("\u00e5ngstr\u00f6m"));
+		assertNormalized(" \u00e5ngstr\u00f6m\t", "\u00e5ngstr\u00f6m");
 	}
 }