Support refspecs with wildcard in middle (not only at end)

The following refspec, which can be used to fetch GitHub pull requests,
is supported by C Git but was not yet by JGit:

  +refs/pull/*/head:refs/remotes/origin/pr/*

The reason is that the wildcard in the source is in the middle.

This change also includes more validation (e.g. "refs//heads" is not
valid) and test cases.

Bug: 405099
Change-Id: I9bcef7785a0762ed0a98ca95a0bdf8879d5702aa

org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/RefSpecTest.java

@@ -1,6 +1,7 @@
 /*
  * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
  * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
+ * Copyright (C) 2013, Robin Stocker <robin@nibor.org>
  * and other copyright owners as documented in the project's IP log.
  *
  * This program and the accompanying materials are made available
@@ -46,6 +47,7 @@ package org.eclipse.jgit.transport;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
@@ -294,4 +296,152 @@ public class RefSpecTest {
 		assertEquals(src, r.getSource());
 		assertEquals(dst, r.getDestination());
 	}
+
+	@Test
+	public void isWildcardShouldWorkForWildcardSuffixAndComponent() {
+		assertTrue(RefSpec.isWildcard("refs/heads/*"));
+		assertTrue(RefSpec.isWildcard("refs/pull/*/head"));
+		assertFalse(RefSpec.isWildcard("refs/heads/a"));
+	}
+
+	@Test
+	public void testWildcardInMiddleOfSource() {
+		RefSpec a = new RefSpec("+refs/pull/*/head:refs/remotes/origin/pr/*");
+		assertTrue(a.isWildcard());
+		assertTrue(a.matchSource("refs/pull/a/head"));
+		assertTrue(a.matchSource("refs/pull/foo/head"));
+		assertTrue(a.matchSource("refs/pull/foo/bar/head"));
+		assertFalse(a.matchSource("refs/pull/foo"));
+		assertFalse(a.matchSource("refs/pull/head"));
+		assertFalse(a.matchSource("refs/pull/foo/head/more"));
+		assertFalse(a.matchSource("refs/pullx/head"));
+
+		RefSpec b = a.expandFromSource("refs/pull/foo/head");
+		assertEquals("refs/remotes/origin/pr/foo", b.getDestination());
+		RefSpec c = a.expandFromDestination("refs/remotes/origin/pr/foo");
+		assertEquals("refs/pull/foo/head", c.getSource());
+	}
+
+	@Test
+	public void testWildcardInMiddleOfDestionation() {
+		RefSpec a = new RefSpec("+refs/heads/*:refs/remotes/origin/*/head");
+		assertTrue(a.isWildcard());
+		assertTrue(a.matchDestination("refs/remotes/origin/a/head"));
+		assertTrue(a.matchDestination("refs/remotes/origin/foo/head"));
+		assertTrue(a.matchDestination("refs/remotes/origin/foo/bar/head"));
+		assertFalse(a.matchDestination("refs/remotes/origin/foo"));
+		assertFalse(a.matchDestination("refs/remotes/origin/head"));
+		assertFalse(a.matchDestination("refs/remotes/origin/foo/head/more"));
+		assertFalse(a.matchDestination("refs/remotes/originx/head"));
+
+		RefSpec b = a.expandFromSource("refs/heads/foo");
+		assertEquals("refs/remotes/origin/foo/head", b.getDestination());
+		RefSpec c = a.expandFromDestination("refs/remotes/origin/foo/head");
+		assertEquals("refs/heads/foo", c.getSource());
+	}
+
+	@Test
+	public void testWildcardMirror() {
+		RefSpec a = new RefSpec("*:*");
+		assertTrue(a.isWildcard());
+		assertTrue(a.matchSource("a"));
+		assertTrue(a.matchSource("foo"));
+		assertTrue(a.matchSource("foo/bar"));
+		assertTrue(a.matchDestination("a"));
+		assertTrue(a.matchDestination("foo"));
+		assertTrue(a.matchDestination("foo/bar"));
+
+		RefSpec b = a.expandFromSource("refs/heads/foo");
+		assertEquals("refs/heads/foo", b.getDestination());
+		RefSpec c = a.expandFromDestination("refs/heads/foo");
+		assertEquals("refs/heads/foo", c.getSource());
+	}
+
+	@Test
+	public void testWildcardAtStart() {
+		RefSpec a = new RefSpec("*/head:refs/heads/*");
+		assertTrue(a.isWildcard());
+		assertTrue(a.matchSource("a/head"));
+		assertTrue(a.matchSource("foo/head"));
+		assertTrue(a.matchSource("foo/bar/head"));
+		assertFalse(a.matchSource("/head"));
+		assertFalse(a.matchSource("a/head/extra"));
+
+		RefSpec b = a.expandFromSource("foo/head");
+		assertEquals("refs/heads/foo", b.getDestination());
+		RefSpec c = a.expandFromDestination("refs/heads/foo");
+		assertEquals("foo/head", c.getSource());
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void invalidWhenSourceOnlyAndWildcard() {
+		assertNotNull(new RefSpec("refs/heads/*"));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void invalidWhenDestinationOnlyAndWildcard() {
+		assertNotNull(new RefSpec(":refs/heads/*"));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void invalidWhenOnlySourceWildcard() {
+		assertNotNull(new RefSpec("refs/heads/*:refs/heads/foo"));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void invalidWhenOnlyDestinationWildcard() {
+		assertNotNull(new RefSpec("refs/heads/foo:refs/heads/*"));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void invalidWhenMoreThanOneWildcardInSource() {
+		assertNotNull(new RefSpec("refs/heads/*/*:refs/heads/*"));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void invalidWhenMoreThanOneWildcardInDestination() {
+		assertNotNull(new RefSpec("refs/heads/*:refs/heads/*/*"));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void invalidWhenWildcardAfterText() {
+		assertNotNull(new RefSpec("refs/heads/wrong*:refs/heads/right/*"));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void invalidWhenWildcardBeforeText() {
+		assertNotNull(new RefSpec("*wrong:right/*"));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void invalidWhenWildcardBeforeTextAtEnd() {
+		assertNotNull(new RefSpec("refs/heads/*wrong:right/*"));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void invalidSourceDoubleSlashes() {
+		assertNotNull(new RefSpec("refs/heads//wrong"));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void invalidSlashAtStart() {
+		assertNotNull(new RefSpec("/foo:/foo"));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void invalidDestinationDoubleSlashes() {
+		assertNotNull(new RefSpec(":refs/heads//wrong"));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void invalidSetSource() {
+		RefSpec a = new RefSpec("refs/heads/*:refs/remotes/origin/*");
+		a.setSource("refs/heads/*/*");
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void invalidSetDestination() {
+		RefSpec a = new RefSpec("refs/heads/*:refs/remotes/origin/*");
+		a.setDestination("refs/remotes/origin/*/*");
+	}
 }

org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties

@@ -271,6 +271,7 @@ invalidTagOption=Invalid tag option: {0}
 invalidTimeout=Invalid timeout: {0}
 invalidURL=Invalid URL {0}
 invalidWildcards=Invalid wildcards {0}
+invalidRefSpec=Invalid refspec {0}
 invalidWindowSize=Invalid window size
 isAStaticFlagAndHasNorevWalkInstance={0} is a static flag and has no RevWalk instance
 JRELacksMD5Implementation=JRE lacks MD5 implementation

org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java

@@ -333,6 +333,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String invalidTimeout;
 	/***/ public String invalidURL;
 	/***/ public String invalidWildcards;
+	/***/ public String invalidRefSpec;
 	/***/ public String invalidWindowSize;
 	/***/ public String isAStaticFlagAndHasNorevWalkInstance;
 	/***/ public String JRELacksMD5Implementation;

org.eclipse.jgit/src/org/eclipse/jgit/transport/RefSpec.java

@@ -43,8 +43,8 @@
 
 package org.eclipse.jgit.transport;
 
-import java.text.MessageFormat;
 import java.io.Serializable;
+import java.text.MessageFormat;
 
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.lib.Constants;
@@ -73,7 +73,7 @@ public class RefSpec implements Serializable {
 	 * @return true if provided string is a wildcard ref spec component.
 	 */
 	public static boolean isWildcard(final String s) {
-		return s != null && s.endsWith(WILDCARD_SUFFIX);
+		return s != null && s.contains("*"); //$NON-NLS-1$
 	}
 
 	/** Does this specification ask for forced updated (rewind/reset)? */
@@ -112,6 +112,7 @@ public class RefSpec implements Serializable {
 	 * <li><code>+refs/heads/master</code></li>
 	 * <li><code>+refs/heads/master:refs/remotes/origin/master</code></li>
 	 * <li><code>+refs/heads/*:refs/remotes/origin/*</code></li>
+	 * <li><code>+refs/pull/&#42;/head:refs/remotes/origin/pr/*</code></li>
 	 * <li><code>:refs/heads/master</code></li>
 	 * </ul>
 	 *
@@ -132,18 +133,24 @@ public class RefSpec implements Serializable {
 			s = s.substring(1);
 			if (isWildcard(s))
 				throw new IllegalArgumentException(MessageFormat.format(JGitText.get().invalidWildcards, spec));
-			dstName = s;
+			dstName = checkValid(s);
 		} else if (c > 0) {
-			srcName = s.substring(0, c);
+			String src = s.substring(0, c);
-			dstName = s.substring(c + 1);
+			String dst = s.substring(c + 1);
-			if (isWildcard(srcName) && isWildcard(dstName))
+			if (isWildcard(src) && isWildcard(dst)) {
+				// Both contain wildcard
 				wildcard = true;
-			else if (isWildcard(srcName) || isWildcard(dstName))
+			} else if (isWildcard(src) || isWildcard(dst)) {
+				// If either source or destination has wildcard, the other one
+				// must have as well.
 				throw new IllegalArgumentException(MessageFormat.format(JGitText.get().invalidWildcards, spec));
+			}
+			srcName = checkValid(src);
+			dstName = checkValid(dst);
 		} else {
 			if (isWildcard(s))
 				throw new IllegalArgumentException(MessageFormat.format(JGitText.get().invalidWildcards, spec));
-			srcName = s;
+			srcName = checkValid(s);
 		}
 	}
 
@@ -215,7 +222,7 @@ public class RefSpec implements Serializable {
 	 */
 	public RefSpec setSource(final String source) {
 		final RefSpec r = new RefSpec(this);
-		r.srcName = source;
+		r.srcName = checkValid(source);
 		if (isWildcard(r.srcName) && r.dstName == null)
 			throw new IllegalStateException(JGitText.get().destinationIsNotAWildcard);
 		if (isWildcard(r.srcName) != isWildcard(r.dstName))
@@ -254,7 +261,7 @@ public class RefSpec implements Serializable {
 	 */
 	public RefSpec setDestination(final String destination) {
 		final RefSpec r = new RefSpec(this);
-		r.dstName = destination;
+		r.dstName = checkValid(destination);
 		if (isWildcard(r.dstName) && r.srcName == null)
 			throw new IllegalStateException(JGitText.get().sourceIsNotAWildcard);
 		if (isWildcard(r.srcName) != isWildcard(r.dstName))
@@ -350,8 +357,7 @@ public class RefSpec implements Serializable {
 		final String psrc = srcName, pdst = dstName;
 		wildcard = false;
 		srcName = name;
-		dstName = pdst.substring(0, pdst.length() - 1)
+		dstName = expandWildcard(name, psrc, pdst);
-				+ name.substring(psrc.length() - 1);
 		return this;
 	}
 
@@ -392,8 +398,7 @@ public class RefSpec implements Serializable {
 	private RefSpec expandFromDstImp(final String name) {
 		final String psrc = srcName, pdst = dstName;
 		wildcard = false;
-		srcName = psrc.substring(0, psrc.length() - 1)
+		srcName = expandWildcard(name, pdst, psrc);
-				+ name.substring(pdst.length() - 1);
 		dstName = name;
 		return this;
 	}
@@ -414,12 +419,50 @@ public class RefSpec implements Serializable {
 		return expandFromDestination(r.getName());
 	}
 
-	private boolean match(final String refName, final String s) {
+	private boolean match(final String name, final String s) {
 		if (s == null)
 			return false;
-		if (isWildcard())
+		if (isWildcard()) {
-			return refName.startsWith(s.substring(0, s.length() - 1));
+			int wildcardIndex = s.indexOf('*');
-		return refName.equals(s);
+			String prefix = s.substring(0, wildcardIndex);
+			String suffix = s.substring(wildcardIndex + 1);
+			return name.length() > prefix.length() + suffix.length()
+					&& name.startsWith(prefix) && name.endsWith(suffix);
+		}
+		return name.equals(s);
+	}
+
+	private static String expandWildcard(String name, String patternA,
+			String patternB) {
+		int a = patternA.indexOf('*');
+		int trailingA = patternA.length() - (a + 1);
+		int b = patternB.indexOf('*');
+		String match = name.substring(a, name.length() - trailingA);
+		return patternB.substring(0, b) + match + patternB.substring(b + 1);
+	}
+
+	private static String checkValid(String spec) {
+		if (spec != null && !isValid(spec))
+			throw new IllegalArgumentException(MessageFormat.format(
+					JGitText.get().invalidRefSpec, spec));
+		return spec;
+	}
+
+	private static boolean isValid(final String s) {
+		if (s.startsWith("/")) //$NON-NLS-1$
+			return false;
+		if (s.contains("//")) //$NON-NLS-1$
+			return false;
+		int i = s.indexOf('*');
+		if (i != -1) {
+			if (s.indexOf('*', i + 1) > i)
+				return false;
+			if (i > 0 && s.charAt(i - 1) != '/')
+				return false;
+			if (i < s.length() - 1 && s.charAt(i + 1) != '/')
+				return false;
+		}
+		return true;
 	}
 
 	public int hashCode() {