Merge pull request #848 from jyotisingh/hg_credentials_bug_57

#57 - Fixed bug with hg material checkout when using basic auth with hg v2.0 or higher

common/src/com/thoughtworks/go/config/materials/mercurial/HgMaterial.java

@@ -16,13 +16,6 @@
 
 package com.thoughtworks.go.config.materials.mercurial;
 
-import java.io.File;
-import java.util.List;
-import java.util.Map;
-import java.util.UUID;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
 import com.thoughtworks.go.config.materials.ScmMaterial;
 import com.thoughtworks.go.config.materials.ScmMaterialConfig;
 import com.thoughtworks.go.config.materials.SubprocessExecutionContext;
@@ -34,17 +27,20 @@
 import com.thoughtworks.go.domain.materials.mercurial.HgCommand;
 import com.thoughtworks.go.domain.materials.mercurial.HgMaterialInstance;
 import com.thoughtworks.go.domain.materials.svn.MaterialUrl;
-import com.thoughtworks.go.util.GoConstants;
 import com.thoughtworks.go.util.FileUtil;
-import com.thoughtworks.go.util.command.ConsoleResult;
-import com.thoughtworks.go.util.command.HgUrlArgument;
-import com.thoughtworks.go.util.command.InMemoryStreamConsumer;
-import com.thoughtworks.go.util.command.ProcessOutputStreamConsumer;
-import com.thoughtworks.go.util.command.UrlArgument;
+import com.thoughtworks.go.util.GoConstants;
+import com.thoughtworks.go.util.command.*;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.math.NumberUtils;
 import org.apache.log4j.Logger;
 
+import java.io.File;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 import static com.thoughtworks.go.util.ExceptionUtils.bomb;
 import static com.thoughtworks.go.util.ExceptionUtils.bombIfFailedToRunCommandLine;
 import static com.thoughtworks.go.util.FileUtil.createParentFolderIfNotExist;
@@ -55,7 +51,7 @@
  */
 public class HgMaterial extends ScmMaterial {
     private static final Pattern HG_VERSION_PATTERN = Pattern.compile(".*\\(.*\\s+(\\d(\\.\\d)+.*)\\)");
-    private static final Logger LOG = Logger.getLogger(HgMaterial.class);
+    private static final Logger LOGGER = Logger.getLogger(HgMaterial.class);
     private HgUrlArgument url;
 
     //TODO: use iBatis to set the type for us, and we can get rid of this field.
@@ -194,18 +190,18 @@ ValidationBean handleException(Exception e, String hgVersionConsoleOut) {
                 return defaultResponse;
             }
         } catch (Exception e1) {
-            LOG.debug("Problem validating HG", e);
+            LOGGER.debug("Problem validating HG", e);
             return defaultResponse;
         }
     }
 
 
     private HgCommand hg(File baseDir, ProcessOutputStreamConsumer outputStreamConsumer) throws Exception {
         File workingFolder = workingdir(baseDir);
-        HgCommand hgCommand = new HgCommand(getFingerprint(), workingFolder, getBranch());
-        if (!isHgRepository(workingFolder) || isRepositoryChanged(hgCommand, workingFolder)) {
-            if (LOG.isDebugEnabled()) {
-                LOG.debug("Invalid hg working copy or repository changed. Delete folder: " + workingFolder);
+        HgCommand hgCommand = new HgCommand(getFingerprint(), workingFolder, getBranch(), getUrl());
+        if (!isHgRepository(workingFolder) || isRepositoryChanged(hgCommand)) {
+            if (LOGGER.isDebugEnabled()) {
+                LOGGER.debug("Invalid hg working copy or repository changed. Delete folder: " + workingFolder);
             }
             FileUtil.deleteFolder(workingFolder);
         }
@@ -221,13 +217,9 @@ private boolean isHgRepository(File workingFolder) {
         return new File(workingFolder, ".hg").isDirectory();
     }
 
-    private boolean isRepositoryChanged(HgCommand hgCommand, File workingDirectory) {
+    private boolean isRepositoryChanged(HgCommand hgCommand) {
         ConsoleResult result = hgCommand.workingRepositoryUrl();
-        if (LOG.isTraceEnabled()) {
-            LOG.trace("Current repository url of [" + workingDirectory + "]: " + result.outputForDisplayAsString());
-            LOG.trace("Target repository url: " + url);
-        }
-        return !MaterialUrl.sameUrl(url.forCommandline(), result.outputAsString());
+        return !MaterialUrl.sameUrl(url.defaultRemoteUrl(), new HgUrlArgument(result.outputAsString()).defaultRemoteUrl());
     }
 
     public String getUserName() {

common/src/com/thoughtworks/go/domain/materials/mercurial/HgCommand.java

@@ -16,21 +16,17 @@
 
 package com.thoughtworks.go.domain.materials.mercurial;
 
-import java.io.File;
-import java.io.UnsupportedEncodingException;
-import java.net.URLDecoder;
-import java.util.List;
-
 import com.thoughtworks.go.domain.materials.Modification;
 import com.thoughtworks.go.domain.materials.Revision;
 import com.thoughtworks.go.domain.materials.SCMCommand;
-import com.thoughtworks.go.util.command.CommandLine;
-import com.thoughtworks.go.util.command.ConsoleResult;
-import com.thoughtworks.go.util.command.InMemoryStreamConsumer;
-import com.thoughtworks.go.util.command.ProcessOutputStreamConsumer;
-import com.thoughtworks.go.util.command.UrlArgument;
+import com.thoughtworks.go.util.command.*;
 import org.apache.log4j.Logger;
 
+import java.io.File;
+import java.io.UnsupportedEncodingException;
+import java.net.URLDecoder;
+import java.util.List;
+
 import static com.thoughtworks.go.util.ExceptionUtils.bomb;
 import static com.thoughtworks.go.util.ExceptionUtils.bombUnless;
 import static com.thoughtworks.go.util.command.CommandLine.createCommandLine;
@@ -42,16 +38,18 @@ public class HgCommand extends SCMCommand {
     private final File workingDir;
     private static String templatePath;
     private final String branch;
+    private final String url;
 
-    public HgCommand(String materialFingerprint, File workingDir, String branch) {
+    public HgCommand(String materialFingerprint, File workingDir, String branch, String url) {
         super(materialFingerprint);
         this.workingDir = workingDir;
         this.branch = branch;
+        this.url = url;
     }
 
 
     private boolean pull(ProcessOutputStreamConsumer outputStreamConsumer) {
-        CommandLine hg = hg("pull", "-b", branch);
+        CommandLine hg = hg("pull", "-b", branch, "--config", String.format("paths.default=%s", url));
         return execute(hg, outputStreamConsumer) == 0;
     }
 
@@ -132,7 +130,13 @@ public List<Modification> modificationsSince(Revision revision) {
 
     public ConsoleResult workingRepositoryUrl() {
         CommandLine hg = hg("showconfig", "paths.default");
-        return execute(hg);
+
+        final ConsoleResult result = execute(hg);
+        if (LOGGER.isTraceEnabled()) {
+            LOGGER.trace("Current repository url of [" + workingDir + "]: " + result.outputForDisplayAsString());
+            LOGGER.trace("Target repository url: " + url);
+        }
+        return result;
     }
 
     private CommandLine hg(String... arguments) {

common/test/com/thoughtworks/go/config/materials/mercurial/HgMaterialTest.java

@@ -16,13 +16,6 @@
 
 package com.thoughtworks.go.config.materials.mercurial;
 
-import java.io.File;
-import java.io.IOException;
-import java.util.Date;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-
 import com.thoughtworks.go.domain.materials.Material;
 import com.thoughtworks.go.domain.materials.Modification;
 import com.thoughtworks.go.domain.materials.TestSubprocessExecutionContext;
@@ -32,10 +25,8 @@
 import com.thoughtworks.go.helper.HgTestRepo;
 import com.thoughtworks.go.helper.MaterialsMother;
 import com.thoughtworks.go.helper.TestRepo;
-import com.thoughtworks.go.util.FileUtil;
-import com.thoughtworks.go.util.JsonUtils;
-import com.thoughtworks.go.util.JsonValue;
-import com.thoughtworks.go.util.TestFileUtil;
+import com.thoughtworks.go.util.*;
+import com.thoughtworks.go.util.command.ConsoleResult;
 import com.thoughtworks.go.util.command.InMemoryStreamConsumer;
 import com.thoughtworks.go.util.json.JsonMap;
 import org.hamcrest.Matchers;
@@ -45,15 +36,23 @@
 import org.junit.Test;
 import org.junit.matchers.JUnitMatchers;
 
+import java.io.File;
+import java.io.IOException;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+
 import static com.thoughtworks.go.util.DateUtils.parseISO8601;
 import static com.thoughtworks.go.util.command.ProcessOutputStreamConsumer.inMemoryConsumer;
 import static java.lang.String.format;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.core.Is.is;
 import static org.hamcrest.core.IsNull.nullValue;
 import static org.hamcrest.core.StringContains.containsString;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class HgMaterialTest {
     private HgMaterial hgMaterial;
@@ -103,31 +102,51 @@ public void teardown() {
 
     @Test
     public void shouldRefreshWorkingFolderWhenRepositoryChanged() throws Exception {
-        new HgCommand(null, workingFolder, "default").clone(inMemoryConsumer(), hgTestRepo.url());
+        new HgCommand(null, workingFolder, "default", hgTestRepo.url().forCommandline()).clone(inMemoryConsumer(), hgTestRepo.url());
         File testFile = createNewFileInWorkingFolder();
 
         HgTestRepo hgTestRepo2 = new HgTestRepo("hgTestRepo2");
         hgMaterial = MaterialsMother.hgMaterial(hgTestRepo2.projectRepositoryUrl());
         hgMaterial.latestModification(workingFolder, new TestSubprocessExecutionContext());
 
-        String workingUrl = new HgCommand(null, workingFolder, "default").workingRepositoryUrl().outputAsString();
+        String workingUrl = new HgCommand(null, workingFolder, "default", hgTestRepo.url().forCommandline()).workingRepositoryUrl().outputAsString();
         assertThat(workingUrl, is(hgTestRepo2.projectRepositoryUrl()));
         assertThat(testFile.exists(), is(false));
     }
 
     @Test
     public void shouldNotRefreshWorkingFolderWhenFileProtocolIsUsed() throws Exception {
-        new HgCommand(null, workingFolder, "default").clone(inMemoryConsumer(), hgTestRepo.url());
+        new HgCommand(null, workingFolder, "default", hgTestRepo.url().forCommandline()).clone(inMemoryConsumer(), hgTestRepo.url());
         File testFile = createNewFileInWorkingFolder();
 
         hgMaterial = MaterialsMother.hgMaterial("file://" + hgTestRepo.projectRepositoryUrl());
         hgMaterial.updateTo(outputStreamConsumer, new StringRevision("0"), workingFolder, new TestSubprocessExecutionContext());
 
-        String workingUrl = new HgCommand(null, workingFolder, "default").workingRepositoryUrl().outputAsString();
+        String workingUrl = new HgCommand(null, workingFolder, "default", hgTestRepo.url().forCommandline()).workingRepositoryUrl().outputAsString();
         assertThat(workingUrl, is(hgTestRepo.projectRepositoryUrl()));
         assertThat(testFile.exists(), is(true));
     }
 
+    @Test
+    public void shouldNotRefreshWorkingDirectoryIfDefaultRemoteUrlDoesNotContainPasswordButMaterialUrlDoes() throws Exception {
+        final HgMaterial material = new HgMaterial("http://user:pwd@domain:9999/path", null);
+        final HgCommand hgCommand = mock(HgCommand.class);
+        final ConsoleResult consoleResult = mock(ConsoleResult.class);
+        when(consoleResult.outputAsString()).thenReturn("http://user@domain:9999/path");
+        when(hgCommand.workingRepositoryUrl()).thenReturn(consoleResult);
+        assertFalse((Boolean) ReflectionUtil.invoke(material, "isRepositoryChanged", hgCommand));
+    }
+
+    @Test
+    public void shouldRefreshWorkingDirectoryIfUsernameInDefaultRemoteUrlIsDifferentFromOneInMaterialUrl() throws Exception {
+        final HgMaterial material = new HgMaterial("http://some_new_user:pwd@domain:9999/path", null);
+        final HgCommand hgCommand = mock(HgCommand.class);
+        final ConsoleResult consoleResult = mock(ConsoleResult.class);
+        when(consoleResult.outputAsString()).thenReturn("http://user:pwd@domain:9999/path");
+        when(hgCommand.workingRepositoryUrl()).thenReturn(consoleResult);
+        assertTrue((Boolean) ReflectionUtil.invoke(material, "isRepositoryChanged", hgCommand));
+    }
+
     @Test
     public void shouldGetModifications() throws Exception {
         List<Modification> mods = hgMaterial.modificationsSince(workingFolder, new StringRevision(REVISION_0), new TestSubprocessExecutionContext());

common/test/com/thoughtworks/go/domain/materials/mercurial/HgCommandTest.java

@@ -16,12 +16,6 @@
 
 package com.thoughtworks.go.domain.materials.mercurial;
 
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
-
 import com.thoughtworks.go.domain.materials.Modification;
 import com.thoughtworks.go.domain.materials.Revision;
 import com.thoughtworks.go.util.TestFileUtil;
@@ -36,6 +30,12 @@
 import org.junit.Ignore;
 import org.junit.Test;
 
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
 import static com.thoughtworks.go.util.FileUtil.deleteFolder;
 import static com.thoughtworks.go.util.command.ProcessOutputStreamConsumer.inMemoryConsumer;
 import static org.hamcrest.CoreMatchers.containsString;
@@ -66,7 +66,7 @@ public void setUp() throws IOException {
 
         setUpServerRepoFromHgBundle(serverRepo, new File("../common/test-resources/data/hgrepo.hgbundle"));
         workingDirectory = new File(clientRepo.getPath());
-        hgCommand = new HgCommand(null, workingDirectory, "default");
+        hgCommand = new HgCommand(null, workingDirectory, "default", serverRepo.getAbsolutePath());
         hgCommand.clone(outputStreamConsumer, new UrlArgument(serverRepo.getAbsolutePath()));
     }
 
@@ -116,8 +116,8 @@ public void shouldGetModifications() throws Exception {
     @Test
     public void shouldNotGetModificationsFromOtherBranches() throws Exception {
         makeACommitToSecondBranch();
-
         hg(workingDirectory, "pull").runOrBomb(null);
+
         List<Modification> actual = hgCommand.modificationsSince(new StringRevision(REVISION_0));
         assertThat(actual.size(), is(2));
         assertThat(actual.get(0).getRevision(), is(REVISION_2));
@@ -187,7 +187,7 @@ public void shouldThrowExceptionForBadConnection() throws Exception {
     @Test
     public void shouldCloneOnlyTheSpecifiedBranchAndPointToIt() {
         String branchName = "second";
-        HgCommand hg = new HgCommand(null, secondBranchWorkingCopy, branchName);
+        HgCommand hg = new HgCommand(null, secondBranchWorkingCopy, branchName, serverRepo.getAbsolutePath());
         hg.clone(outputStreamConsumer, new UrlArgument(serverRepo.getAbsolutePath() + "#" + branchName));
 
         String currentBranch = hg(secondBranchWorkingCopy, "branch").runOrBomb(null).outputAsString();
@@ -260,9 +260,8 @@ private void setUpServerRepoFromHgBundle(File serverRepo, File hgBundleFile) {
     }
 
     private void makeACommitToSecondBranch() {
-        HgCommand hg = new HgCommand(null, secondBranchWorkingCopy, "second");
+        HgCommand hg = new HgCommand(null, secondBranchWorkingCopy, "second", serverRepo.getAbsolutePath());
         hg.clone(outputStreamConsumer, new UrlArgument(serverRepo.getAbsolutePath()));
         createNewFileAndPushUpstream(secondBranchWorkingCopy);
     }
-
 }

common/test/com/thoughtworks/go/helper/HgTestRepo.java

@@ -16,11 +16,6 @@
 
 package com.thoughtworks.go.helper;
 
-import java.io.File;
-import java.io.IOException;
-import java.util.Collections;
-import java.util.List;
-
 import com.thoughtworks.go.config.materials.ScmMaterialConfig;
 import com.thoughtworks.go.config.materials.mercurial.HgMaterial;
 import com.thoughtworks.go.config.materials.mercurial.HgMaterialConfig;
@@ -36,6 +31,11 @@
 import com.thoughtworks.go.util.command.UrlArgument;
 import org.apache.commons.io.FileUtils;
 
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+
 import static com.thoughtworks.go.util.command.ProcessOutputStreamConsumer.inMemoryConsumer;
 import static org.junit.Assert.fail;
 
@@ -60,7 +60,7 @@ public HgTestRepo(String workingCopyName) throws IOException {
         setUpServerRepoFromHgBundle(remoteRepo, bundleToExtract);
 
         File workingCopy = new File(tempFolder, workingCopyName);
-        hgCommand = new HgCommand(null, workingCopy, "default");
+        hgCommand = new HgCommand(null, workingCopy, "default", remoteRepo.getAbsolutePath());
         InMemoryStreamConsumer output = inMemoryConsumer();
         if (hgCommand.clone(output, new UrlArgument(remoteRepo.getAbsolutePath())) != 0) {
             fail("Error creating repository\n" + output.getAllOutput());

common/test/com/thoughtworks/go/util/command/HgUrlArgumentTest.java

@@ -28,4 +28,34 @@ public void shouldMaskThePasswordInDisplayName(){
          assertThat(hgUrlArgument.forDisplay(), is("http://user:******@url##branch"));
      }
 
+    @Test
+    public void shouldReturnAURLWithoutPassword(){
+        assertThat(new HgUrlArgument("http://user:pwd@url##branch").defaultRemoteUrl(), is("http://user@url#branch"));
+    }
+
+    @Test
+    public void shouldReturnAURLWhenPasswordIsNotSpecified() throws Exception {
+        assertThat(new HgUrlArgument("http://user@url##branch").defaultRemoteUrl(), is("http://user@url#branch"));
+    }
+
+    @Test
+    public void shouldReturnTheURLWhenNoCredentialsAreSpecified() throws Exception {
+        assertThat(new HgUrlArgument("http://url##branch").defaultRemoteUrl(), is("http://url#branch"));
+    }
+
+    @Test
+    public void shouldReturnUrlWithoutPasswordWhenUrlIncludesPort() throws Exception {
+        assertThat(new HgUrlArgument("http://user:pwd@domain:9887/path").defaultRemoteUrl(), is("http://user@domain:9887/path"));
+    }
+
+    @Test
+    public void shouldNotModifyAbsoluteFilePaths() throws Exception {
+        assertThat(new HgUrlArgument("/tmp/foo").defaultRemoteUrl(), is("/tmp/foo"));
+    }
+
+    @Test
+    public void shouldNotModifyFileURIS() throws Exception {
+        assertThat(new HgUrlArgument("file://junk").defaultRemoteUrl(), is("file://junk"));
+    }
+
 }