fix: support custom port in SSH URL for private repo check#41822
fix: support custom port in SSH URL for private repo check#41822YoussefMansour9 wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughExtended the SSH URL regex to capture custom ports in SCP-like URLs ( ChangesSSH URL Pattern Extension
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java`:
- Around line 75-77: The httpsUrl construction in GitUtils.java currently
appends ":" + port whenever port != null, but port can be an empty string
(pattern uses \\d*), producing "https://host:/..."; update the conditional used
when building httpsUrl (the expression assigning httpsUrl) to treat an empty
port as absent by checking that port is not null AND not empty (or matches
"\\d+"), and only append ":" + port when that check passes; keep the existing
use of match.group("host") and path unchanged.
- Line 43: The regex in the Pattern.compile call inside GitUtils declares the
named group "host" in both alternation branches which causes
PatternSyntaxException on class load; fix it by rewriting the pattern so "host"
is defined only once (e.g., pull the host subpattern out of the alternation or
convert one or both occurrences to non-named groups and then reference by group
index), update any code that reads the group to use the single "host" name or
the appropriate group index, and re-run tests to ensure other named groups like
"path" / "pathOnly" still resolve correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0c19ede1-ebcf-4042-b4ef-663f089c135e
📒 Files selected for processing (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java
d2a130c to
f6e0961
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java`:
- Line 43: The current regex in GitUtils that builds the Pattern.compile(...)
treats a numeric first path segment as a port; update the parsing to
disambiguate numeric-first segments by either (A) swapping the alternation in
the Pattern.compile(...) so the path-only branch is attempted before the port
branch, or (B) implement a dual-attempt conversion inside GitUtils that first
tries parsing with port capture and, if the result yields an unlikely port/path
interpretation, falls back to treating the numeric segment as part of the path;
then add regression tests covering both forms like "git@host:1234/repo.git" (as
path) and "git@host:1234/actual/repo.git" (as host+port) to ensure both
interpretations behave as expected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ba0ed825-183c-425a-b456-9bb4af0bf343
📒 Files selected for processing (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java
- Updated URL_PATTERN_WITHOUT_SCHEME to capture custom port number - Fixed duplicate named group 'host' by using non-capturing group alternation - Added empty port check to prevent malformed URLs like https://host:/... - Updated convertSshUrlToBrowserSupportedUrl to include port in HTTPS URL when present - Added documentation for known limitation with numeric path segments Fixes appsmithorg#32862
f6e0961 to
329660b
Compare
Description
The SSH URL pattern for private repo check did not accommodate custom ports. When connecting to a git repo on a custom port (e.g.,
git@host:2222/user/repo.git), the port was being included as part of the path, resulting in incorrect HTTPS URL likehttps://host/2222/user/repoinstead ofhttps://host:2222/user/repo.Changes
URL_PATTERN_WITHOUT_SCHEMEregex to capture custom port numberconvertSshUrlToBrowserSupportedUrl()to include port in HTTPS URL when presentTesting
git@host:2222/user/repo.git→https://host:2222/user/repo.git(new - custom port)git@host:user/repo.git→https://host/user/repo.git(existing - no port)Fixes #32862
Summary by CodeRabbit