From a51899c2036984fa7a04290ca1364a02af34f95a Mon Sep 17 00:00:00 2001 From: Robin Stocker Date: Sat, 4 May 2013 16:43:51 +0200 Subject: [PATCH] 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 --- .../eclipse/jgit/transport/RefSpecTest.java | 150 ++++++++++++++++++ .../eclipse/jgit/internal/JGitText.properties | 1 + .../org/eclipse/jgit/internal/JGitText.java | 1 + .../org/eclipse/jgit/transport/RefSpec.java | 81 +++++++--- 4 files changed, 214 insertions(+), 19 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/RefSpecTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/RefSpecTest.java index 871741f69..3f5fcbbf0 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/RefSpecTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/RefSpecTest.java @@ -1,6 +1,7 @@ /* * Copyright (C) 2008, Robin Rosenberg * Copyright (C) 2008, Shawn O. Pearce + * Copyright (C) 2013, Robin Stocker * 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/*/*"); + } } 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 8b6211efb..33489eb67 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/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 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 29bec97ee..d75ed4ee2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/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; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefSpec.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefSpec.java index 9b38b2933..66ffc3abe 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/RefSpec.java +++ b/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; @@ -59,7 +59,7 @@ import org.eclipse.jgit.lib.Ref; public class RefSpec implements Serializable { private static final long serialVersionUID = 1L; - /** + /** * Suffix for wildcard ref spec component, that indicate matching all refs * with specified prefix. */ @@ -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 { *
  • +refs/heads/master
    • *
  • +refs/heads/master:refs/remotes/origin/master
    • *
  • +refs/heads/*:refs/remotes/origin/*
    • + *
  • +refs/pull/*/head:refs/remotes/origin/pr/*
    • *
  • :refs/heads/master
    • * * @@ -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); - dstName = s.substring(c + 1); - if (isWildcard(srcName) && isWildcard(dstName)) + String src = s.substring(0, c); + String dst = s.substring(c + 1); + 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) - + name.substring(psrc.length() - 1); + dstName = expandWildcard(name, psrc, pdst); 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) - + name.substring(pdst.length() - 1); + srcName = expandWildcard(name, pdst, psrc); 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()) - return refName.startsWith(s.substring(0, s.length() - 1)); - return refName.equals(s); + if (isWildcard()) { + int wildcardIndex = s.indexOf('*'); + 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() {