From 8fe1e8c854a7c31e235effc4e85ef41ea73ef11d Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Wed, 17 Apr 2024 19:18:15 +0530 Subject: [PATCH] fix: Fix support for custom SSH port in Git URLs (#32678) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Support for custom SSH ports in Git URLs broken when we introduced the regex to match the URLs. The tests were also checking for invalid URLs. Ref: https://stackoverflow.com/questions/5767850/git-on-custom-ssh-port. This PR fixes that validation error, and introduced tests with the correct Git SSH URL with a custom port. [Slack thread](https://theappsmith.slack.com/archives/C0341RERY4R/p1713178681476249?thread_ts=1713175069.330019&cid=C0341RERY4R). /ok-to-test tags="@tag.Git" > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 48809d88619fa5d7d170e8f7d71297cccaecb353 > Cypress dashboard url: Click here! ## Summary by CodeRabbit - **Refactor** - Enhanced SSH URL parsing and conversion to support more URL formats and custom SSH ports, improving compatibility with browser-supported HTTPS URLs. --- .../com/appsmith/server/helpers/GitUtils.java | 12 +++++++++--- .../appsmith/server/helpers/GitUtilsTest.java | 19 +++++++++++++++---- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java index f5056a0b21..0387e059bd 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java @@ -20,8 +20,11 @@ public class GitUtils { public static final Duration RETRY_DELAY = Duration.ofSeconds(1); public static final Integer MAX_RETRIES = 20; - public static final Pattern GIT_SSH_URL_PATTERN = - Pattern.compile("^(ssh://)?git@(?.+?):/*(?.+?)(\\.git)?$"); + public static final Pattern URL_PATTERN_WITH_SCHEME = + Pattern.compile("^ssh://git@(?.+?)(:(?\\d+))?/+(?.+?)(\\.git)?$"); + + public static final Pattern URL_PATTERN_WITHOUT_SCHEME = + Pattern.compile("^git@(?.+?):/*(?.+?)(\\.git)?$"); /** * Sample repo urls : @@ -37,7 +40,10 @@ public class GitUtils { throw new AppsmithException(AppsmithError.INVALID_PARAMETER, "ssh url"); } - final Matcher match = GIT_SSH_URL_PATTERN.matcher(sshUrl); + Matcher match = URL_PATTERN_WITH_SCHEME.matcher(sshUrl); + if (!match.matches()) { + match = URL_PATTERN_WITHOUT_SCHEME.matcher(sshUrl); + } if (!match.matches()) { throw new AppsmithException( diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/GitUtilsTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/GitUtilsTest.java index d6a5176dfe..0409ec3152 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/GitUtilsTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/GitUtilsTest.java @@ -30,7 +30,7 @@ public class GitUtilsTest { assertThat(GitUtils.convertSshUrlToBrowserSupportedUrl("git@example.in:test/testRepo.git")) .isEqualTo("https://example.in/test/testRepo"); assertThat(GitUtils.convertSshUrlToBrowserSupportedUrl( - "ssh://git@example.test.net:user/test/tests/testRepo.git")) + "ssh://git@example.test.net/user/test/tests/testRepo.git")) .isEqualTo("https://example.test.net/user/test/tests/testRepo"); assertThat(GitUtils.convertSshUrlToBrowserSupportedUrl( "git@tim.tam.example.com:v3/sladeping/pyhe/SpaceJunk.git")) @@ -38,10 +38,10 @@ public class GitUtilsTest { assertThat(GitUtils.convertSshUrlToBrowserSupportedUrl("git@tim.tam.example.com:v3/sladeping/pyhe/SpaceJunk")) .isEqualTo("https://tim.tam.example.com/v3/sladeping/pyhe/SpaceJunk"); assertThat(GitUtils.convertSshUrlToBrowserSupportedUrl( - "ssh://git@tim.tam.example.com:v3/sladeping/pyhe/SpaceJunk.git")) + "ssh://git@tim.tam.example.com/v3/sladeping/pyhe/SpaceJunk.git")) .isEqualTo("https://tim.tam.example.com/v3/sladeping/pyhe/SpaceJunk"); assertThat(GitUtils.convertSshUrlToBrowserSupportedUrl( - "ssh://git@tim.tam.example.com:v3/sladeping/pyhe/SpaceJunk")) + "ssh://git@tim.tam.example.com/v3/sladeping/pyhe/SpaceJunk")) .isEqualTo("https://tim.tam.example.com/v3/sladeping/pyhe/SpaceJunk"); assertThat(GitUtils.convertSshUrlToBrowserSupportedUrl("git@127.0.0.1:test/newRepo.git")) .isEqualTo("https://127.0.0.1/test/newRepo"); @@ -49,6 +49,17 @@ public class GitUtilsTest { .isEqualTo("https://localhost/test/newRepo"); assertThat(GitUtils.convertSshUrlToBrowserSupportedUrl("git@absolute.path.com:/test/newRepo.git")) .isEqualTo("https://absolute.path.com/test/newRepo"); + + // Custom SSH port: + assertThat(GitUtils.convertSshUrlToBrowserSupportedUrl( + "ssh://git@example.test.net:1234/user/test/tests/testRepo.git")) + .isEqualTo("https://example.test.net/user/test/tests/testRepo"); + assertThat(GitUtils.convertSshUrlToBrowserSupportedUrl( + "ssh://git@tim.tam.example.com:5678/v3/sladeping/pyhe/SpaceJunk.git")) + .isEqualTo("https://tim.tam.example.com/v3/sladeping/pyhe/SpaceJunk"); + assertThat(GitUtils.convertSshUrlToBrowserSupportedUrl( + "ssh://git@tim.tam.example.com:9876/v3/sladeping/pyhe/SpaceJunk")) + .isEqualTo("https://tim.tam.example.com/v3/sladeping/pyhe/SpaceJunk"); } @Test @@ -60,7 +71,7 @@ public class GitUtilsTest { .verifyComplete(); StepVerifier.create(GitUtils.isRepoPrivate(GitUtils.convertSshUrlToBrowserSupportedUrl( - "ssh://git@example.test.net:user/test/tests/testRepo.git"))) + "ssh://git@example.test.net/user/test/tests/testRepo.git"))) .assertNext(isRepoPrivate -> assertThat(isRepoPrivate).isEqualTo(Boolean.TRUE)) .verifyComplete();