Skip to content

Commit

Permalink
Merge pull request #296 from MarkEWaite/explore-JENKINS-46054
Browse files Browse the repository at this point in the history 
Fix JENKINS-48818 and keep JENKINS-46054 fixed
  • Loading branch information
@MarkEWaite
MarkEWaite committed Jan 25, 2018
2 parents 21914aa + 88fa4fe commit 2dcbb2d
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 96 deletions.
23 changes: 14 additions & 9 deletions src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java
View file
Expand Up @@ -144,22 +144,27 @@ public class CliGitAPIImpl extends LegacyCompatibleGitAPIImpl {

/* git config --get-regex applies the regex to match keys, and returns all matches (including substring matches).
* Thus, a config call:
* git config -f .gitmodules --get-regexp "^submodule\.([^ ]+)\.url"
* git config -f .gitmodules --get-regexp "^submodule\.(.+)\.url"
* will report two lines of output if the submodule URL includes ".url":
* submodule.modules/JENKINS-46504.url.path modules/JENKINS-46504.url
* submodule.modules/JENKINS-46504.url.url https://github.com/MarkEWaite/JENKINS-46054.url
* The code originally used the same pattern for get-regexp and for output parsing.
* By using the same pattern in both places, it incorrectly took the first line
* of output as the URL of a submodule (when it is instead the path of a submodule).
* By using the same pattern in both places, it incorrectly took some substrings
* as the submodule remote name, instead of taking the longest match.
* See SubmodulePatternStringTest for test cases.
*/
private final static String SUBMODULE_REMOTE_PATTERN_CONFIG_KEY = "^submodule\\.([^ ]+)\\.url";

/* See comments for SUBMODULE_REMOTE_PATTERN_CONFIG_KEY to explain why this
* regular expression string adds the trailing space character as part of its match.
* Without the trailing whitespace character in the pattern, too many matches are found.
private final static String SUBMODULE_REMOTE_PATTERN_CONFIG_KEY = "^submodule\\.(.+)\\.url";

/* See comments for SUBMODULE_REMOTE_PATTERN_CONFIG_KEY to explain
* why this regular expression string adds the trailing space
* characters and the sequence of non-space characters as part of
* its match. The ending sequence of non-white-space characters
* is the repository URL in the output of the 'git config' command.
* Relies on repository URL not containing a whitespace character,
* per RFC1738.
*/
/* Package protected for testing */
final static String SUBMODULE_REMOTE_PATTERN_STRING = SUBMODULE_REMOTE_PATTERN_CONFIG_KEY + "\\b\\s";
final static String SUBMODULE_REMOTE_PATTERN_STRING = SUBMODULE_REMOTE_PATTERN_CONFIG_KEY + "\\s+[^\\s]+$";

private void warnIfWindowsTemporaryDirNameHasSpaces() {
if (!isWindows()) {
Expand Down
145 changes: 58 additions & 87 deletions src/test/java/org/jenkinsci/plugins/gitclient/SubmodulePatternStringTest.java
View file
@@ -1,110 +1,81 @@
package org.jenkinsci.plugins.gitclient;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.jvnet.hudson.test.Issue;

@RunWith(Parameterized.class)
public class SubmodulePatternStringTest {

private Pattern submoduleConfigPattern;
private String remoteName = "simple-name";
private final String remoteName;
private final String submoduleConfigOutput;
private final Matcher matcher;

@Before
public void configure() {
// String patternString = "^submodule\\.([^ ]+)\\.url ";
String patternString = CliGitAPIImpl.SUBMODULE_REMOTE_PATTERN_STRING;
submoduleConfigPattern = Pattern.compile(patternString, Pattern.MULTILINE);
}
private static final Pattern SUBMODULE_CONFIG_PATTERN = Pattern.compile(CliGitAPIImpl.SUBMODULE_REMOTE_PATTERN_STRING, Pattern.MULTILINE);

@Issue("JENKINS-46054")
@Test
public void urlEmbeddedInRepoURL() {
String repoUrl = "file://gitroot/thirdparty.url.repo.git";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
assertTrue("Match not found for '" + submoduleConfigOutput + "'", matcher.find());
assertThat(matcher.group(1), is(remoteName));
public SubmodulePatternStringTest(String repoUrl, String remoteName)
{
this.remoteName = remoteName;
this.submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
this.matcher = SUBMODULE_CONFIG_PATTERN.matcher(submoduleConfigOutput);
}

@Issue("JENKINS-46054")
@Test
public void urlEmbeddedInRepoURLsubmoduleEmbeddedDot() {
String repoUrl = "https://mark.url:some%20pass.urlify@gitroot/repo.git";
remoteName = "simple.name";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
assertTrue("Match not found for '" + submoduleConfigOutput + "'", matcher.find());
assertThat(matcher.group(1), is(remoteName));
/*
* Permutations of repository URLs and remote names with various
* protocols and remote names, permuted with various suffixes.
*
* Tests file, ssh (both forms), git, and https.
*/
@Parameterized.Parameters(name = "{0}-{1}")
public static Collection repoAndRemote() {
List<Object[]> arguments = new ArrayList<>();
String[] repoUrls = {
"file://gitroot/thirdparty.url.repo",
"git://gitroot/repo",
"git@github.com:jenkinsci/JENKINS-46054",
"https://github.com/MarkEWaite/JENKINS-46054",
"https://mark.url:some%20pass.urlify@gitroot/repo",
"ssh://git.example.com/MarkEWaite/JENKINS-46054",
};
String[] remoteNames = {
"has space",
"has.url space",
"simple",
"simple.name",
"simple.url.name",
};
String [] suffixes = {
"",
".git",
".url",
".url.git",
};
for (String repoUrlParam : repoUrls) {
for (String repoUrlSuffix : suffixes) {
for (String remoteNameParam : remoteNames) {
for (String remoteNameSuffix : suffixes) {
Object[] item = {repoUrlParam + repoUrlSuffix, remoteNameParam + remoteNameSuffix};
arguments.add(item);
}
}
}
}
return arguments;
}

@Issue("JENKINS-46054")
@Test
public void urlEmbeddedInSubmoduleRepoNameEndsWithURL() {
String repoUrl = "https://gitroot/repo.url";
remoteName = "simple.name";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
public void urlFoundInSubmoduleConfigOutput() {
assertTrue("Match not found for '" + submoduleConfigOutput + "'", matcher.find());
assertThat(matcher.group(1), is(remoteName));
}

@Issue("JENKINS-46054")
@Test
public void urlEmbeddedInSubmoduleRepoNameEndsWithURLSpace() {
String repoUrl = "https://gitroot/repo.url ";
remoteName = "simple.name";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
assertTrue("Match not found for '" + submoduleConfigOutput + "'", matcher.find());
assertThat(matcher.group(1), is(remoteName));
}

@Issue("JENKINS-46054")
@Test
public void urlEmbeddedInSubmoduleNameAndRepoNameEndsWithURL() {
String repoUrl = "https://gitroot/repo.url.git";
remoteName = "simple.name.url";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
assertTrue("Match not found for '" + submoduleConfigOutput + "'", matcher.find());
assertThat(matcher.group(1), is(remoteName));
}

@Issue("JENKINS-46054")
@Test
public void urlExploratoryTestFailureCase() {
/* See https://github.com/MarkEWaite/JENKINS-46054.url/ */
String repoUrl = "https://github.com/MarkEWaite/JENKINS-46054.url";
remoteName = "modules/JENKINS-46504.url";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
assertTrue("Match not found for '" + submoduleConfigOutput + "'", matcher.find());
assertThat(matcher.group(1), is(remoteName));
}

@Issue("JENKINS-46054")
@Test
public void remoteNameIncludesSubmodule() {
/* See https://github.com/MarkEWaite/JENKINS-46054.url/ */
String repoUrl = "https://github.com/MarkEWaite/JENKINS-46054.url";
remoteName = "submodule.JENKINS-46504.url";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
assertTrue("Match not found for '" + submoduleConfigOutput + "'", matcher.find());
assertThat(matcher.group(1), is(remoteName));
}

@Issue("JENKINS-46054")
@Test
public void urlEmbeddedInRepoNameEndsWithURLEmptyRemoteName() {
String repoUrl = "https://github.com/MarkEWaite/JENKINS-46054.url.git";
remoteName = "";
String submoduleConfigOutput = "submodule." + remoteName + ".url " + repoUrl;
Matcher matcher = submoduleConfigPattern.matcher(submoduleConfigOutput);
assertFalse("Unexpected match found for '" + submoduleConfigOutput + "'", matcher.find());
}
}

0 comments on commit 2dcbb2d

Please sign in to comment.