From 36d293fee2847f7ae3e1c00a81a68113dfa6dab8 Mon Sep 17 00:00:00 2001 From: Adrian Gonzalez Date: Sun, 11 Aug 2013 14:56:29 +0800 Subject: [PATCH 1/2] Relaxed remote repository url validation. (fixes #15) --- .../stash/hook/MirrorRepositoryHook.java | 45 ++++++------------- .../static/mirror-repository-hook.soy | 2 +- .../stash/hook/MirrorRepositoryHookTest.java | 12 +++-- 3 files changed, 20 insertions(+), 39 deletions(-) diff --git a/src/main/java/com/englishtown/stash/hook/MirrorRepositoryHook.java b/src/main/java/com/englishtown/stash/hook/MirrorRepositoryHook.java index c8eccf7..e29d5de 100644 --- a/src/main/java/com/englishtown/stash/hook/MirrorRepositoryHook.java +++ b/src/main/java/com/englishtown/stash/hook/MirrorRepositoryHook.java @@ -102,7 +102,7 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep try { final String password = passwordEncryptor.decrypt(settings.password); - final URI authenticatedUrl = getAuthenticatedUrl(settings.mirrorRepoUrl, settings.username, password); + final String authenticatedUrl = getAuthenticatedUrl(settings.mirrorRepoUrl, settings.username, password); executor.submit(new Callable() { @@ -119,7 +119,7 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep String result = builder .command("push") .argument("--prune") // this deletes locally deleted branches - .argument(authenticatedUrl.toString()) + .argument(authenticatedUrl) .argument("+refs/heads/*:refs/heads/*") // Only mirror heads .argument("+refs/tags/*:refs/tags/*") // and tags .errorHandler(passwordHandler) @@ -149,19 +149,18 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep } } - protected URI getAuthenticatedUrl(String mirrorRepoUrl, String username, String password) throws URISyntaxException { + protected String getAuthenticatedUrl(String mirrorRepoUrl, String username, String password) throws URISyntaxException { - URI uri = URI.create(mirrorRepoUrl); - - // ssh doesn't have username/password - if (uri.getScheme().equals("ssh")) { - return uri; + // Only http(s) has username/password + if (!mirrorRepoUrl.toLowerCase().startsWith("http")) { + return mirrorRepoUrl; } + URI uri = URI.create(mirrorRepoUrl); String userInfo = username + ":" + password; return new URI(uri.getScheme(), userInfo, uri.getHost(), uri.getPort(), - uri.getPath(), uri.getQuery(), uri.getFragment()); + uri.getPath(), uri.getQuery(), uri.getFragment()).toString(); } @@ -232,41 +231,26 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep boolean result = true; boolean isHttp = false; - boolean isSsh = false; if (ms.mirrorRepoUrl.isEmpty()) { result = false; errors.addFieldError(SETTING_MIRROR_REPO_URL + ms.suffix, "The mirror repo url is required."); } else { - URI uri; try { - uri = URI.create(ms.mirrorRepoUrl); + URI uri = URI.create(ms.mirrorRepoUrl); String scheme = uri.getScheme().toLowerCase(); - if (scheme.equals("ssh")) { - isSsh = true; - if (!ms.mirrorRepoUrl.startsWith("ssh://git@")) { - result = false; - errors.addFieldError(SETTING_MIRROR_REPO_URL + ms.suffix, - "An ssh URL should start with ssh://git@"); - } - } else if (scheme.startsWith("http")) { + if (scheme.startsWith("http")) { isHttp = true; if (ms.mirrorRepoUrl.contains("@")) { result = false; errors.addFieldError(SETTING_MIRROR_REPO_URL + ms.suffix, "The username and password should not be included."); } - } else { - result = false; - errors.addFieldError(SETTING_MIRROR_REPO_URL + ms.suffix, - "The mirror repo url must be a ssh or http(s) URL."); } - } catch (Exception ex) { - result = false; - errors.addFieldError(SETTING_MIRROR_REPO_URL + ms.suffix, - "The mirror repo url must be a valid ssh or http(s) URL."); + // Not a valid url, assume it is something git can read + } } @@ -281,9 +265,8 @@ public class MirrorRepositoryHook implements AsyncPostReceiveRepositoryHook, Rep result = false; errors.addFieldError(SETTING_PASSWORD + ms.suffix, "The password is required when using http(s)."); } - } - // SSH should not have username or password - if (isSsh) { + } else { + // Only http should have username or password ms.password = ms.username = ""; } diff --git a/src/main/resources/static/mirror-repository-hook.soy b/src/main/resources/static/mirror-repository-hook.soy index d8250a7..0bff37c 100644 --- a/src/main/resources/static/mirror-repository-hook.soy +++ b/src/main/resources/static/mirror-repository-hook.soy @@ -48,7 +48,7 @@ {/param} {param isRequired: true /} {param descriptionText: stash_i18n('com.englishtown.stash.hook.mirror.strings.mirrorRepoUrl.description', - 'The ssh or http(s) URL to the remote mirrored repo') /} + 'The GIT URL (ssh, git, http(s), file) to the remote mirrored repo') /} {param extraClasses: 'long et-mirror-repo' /} {param errorTexts: $errors ? $errors['mirrorRepoUrl' + $index] : null /} {/call} diff --git a/src/test/java/com/englishtown/stash/hook/MirrorRepositoryHookTest.java b/src/test/java/com/englishtown/stash/hook/MirrorRepositoryHookTest.java index a60ea6a..4e88203 100644 --- a/src/test/java/com/englishtown/stash/hook/MirrorRepositoryHookTest.java +++ b/src/test/java/com/englishtown/stash/hook/MirrorRepositoryHookTest.java @@ -175,10 +175,8 @@ public class MirrorRepositoryHookTest { @Test public void testGetAuthenticatedUrl() throws Exception { - URI result; - - result = hook.getAuthenticatedUrl(mirrorRepoUrlHttp, username, password); - assertEquals(repository, result.toString()); + String result = hook.getAuthenticatedUrl(mirrorRepoUrlHttp, username, password); + assertEquals(repository, result); } @@ -239,8 +237,8 @@ public class MirrorRepositoryHookTest { errors = mock(SettingsValidationErrors.class); hook.validate(settings, errors, repo); verify(errors, never()).addFormError(anyString()); - verify(errors).addFieldError(eq(MirrorRepositoryHook.SETTING_MIRROR_REPO_URL + "0"), anyString()); - verify(errors).addFieldError(anyString(), anyString()); + verify(errors, never()).addFieldError(eq(MirrorRepositoryHook.SETTING_MIRROR_REPO_URL + "0"), anyString()); + verify(errors, never()).addFieldError(anyString(), anyString()); errors = mock(SettingsValidationErrors.class); hook.validate(settings, errors, repo); @@ -252,7 +250,7 @@ public class MirrorRepositoryHookTest { errors = mock(SettingsValidationErrors.class); hook.validate(settings, errors, repo); verify(errors, never()).addFormError(anyString()); - verify(errors).addFieldError(eq(MirrorRepositoryHook.SETTING_MIRROR_REPO_URL + "0"), anyString()); + verify(errors, never()).addFieldError(eq(MirrorRepositoryHook.SETTING_MIRROR_REPO_URL + "0"), anyString()); verify(errors, never()).addFieldError(eq(MirrorRepositoryHook.SETTING_USERNAME + "0"), anyString()); verify(errors, never()).addFieldError(eq(MirrorRepositoryHook.SETTING_PASSWORD + "0"), anyString()); From 7b93f99f977fba3d4eee6b48ee4c15697c6e21d4 Mon Sep 17 00:00:00 2001 From: Adrian Gonzalez Date: Sun, 11 Aug 2013 15:10:03 +0800 Subject: [PATCH 2/2] Bumped version to 1.7.0 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 5ba0f92..a0acbe8 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ com.englishtown stash-hook-mirror - 1.6.0 + 1.7.0 Englishtown