From e37b6c7ff380ef3938dc1303d373967b5071a2fb Mon Sep 17 00:00:00 2001 From: Kevin Hellemun <17928966+OGKevin@users.noreply.github.com> Date: Mon, 2 Feb 2026 14:45:20 +0100 Subject: [PATCH] fix: host normalization Extract protocol and path stripping logic into a reusable normalize_host() function. This ensures consistent host handling across platform detection, GitHub, and GitLab auth flows, enabling proper support for enterprise instances with full URLs. This is needed as tools like glab work with GITLAB_HOST set to https://gitlab.company.com. - Add normalize_host() function to types module - Update platform detection to normalize hosts - Update GitHub/GitLab services to normalize hosts - Add unit tests for normalization Change-Id: f3759a0d5d8eed4308b5f1100b6adea7 Change-Id-Short: kwsuqpzmumrl --- src/auth/gitlab.rs | 5 ++-- src/platform/detection.rs | 6 ++-- src/platform/github.rs | 3 +- src/platform/gitlab.rs | 4 +-- src/types.rs | 27 ++++++++++++++++++ tests/unit_tests.rs | 60 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 97 insertions(+), 8 deletions(-) diff --git a/src/auth/gitlab.rs b/src/auth/gitlab.rs index 968643c..857b6ea 100644 --- a/src/auth/gitlab.rs +++ b/src/auth/gitlab.rs @@ -2,6 +2,7 @@ use crate::auth::AuthSource; use crate::error::{Error, Result}; +use crate::types::normalize_host; use reqwest::Client; use serde::Deserialize; use std::env; @@ -27,8 +28,8 @@ pub struct GitLabAuthConfig { /// 3. `GL_TOKEN` environment variable pub async fn get_gitlab_auth(host: Option<&str>) -> Result { let host = host - .map(String::from) - .or_else(|| env::var("GITLAB_HOST").ok()) + .map(normalize_host) + .or_else(|| env::var("GITLAB_HOST").ok().map(|h| normalize_host(&h))) .unwrap_or_else(|| "gitlab.com".to_string()); // Try glab CLI first diff --git a/src/platform/detection.rs b/src/platform/detection.rs index d26864f..c83a4ab 100644 --- a/src/platform/detection.rs +++ b/src/platform/detection.rs @@ -1,7 +1,7 @@ //! Platform detection from remote URLs use crate::error::{Error, Result}; -use crate::types::{Platform, PlatformConfig}; +use crate::types::{Platform, PlatformConfig, normalize_host}; use regex::Regex; use std::env; use std::sync::LazyLock; @@ -16,8 +16,8 @@ static RE_HTTPS: LazyLock = /// Detect platform (GitHub or GitLab) from a remote URL pub fn detect_platform(url: &str) -> Option { - let gh_host = env::var("GH_HOST").ok(); - let gitlab_host = env::var("GITLAB_HOST").ok(); + let gh_host = env::var("GH_HOST").ok().map(|h| normalize_host(&h)); + let gitlab_host = env::var("GITLAB_HOST").ok().map(|h| normalize_host(&h)); let hostname = extract_hostname(url)?; diff --git a/src/platform/github.rs b/src/platform/github.rs index 97dbfc7..da92f31 100644 --- a/src/platform/github.rs +++ b/src/platform/github.rs @@ -2,7 +2,7 @@ use crate::error::{Error, Result}; use crate::platform::PlatformService; -use crate::types::{Platform, PlatformConfig, PrComment, PullRequest}; +use crate::types::{Platform, PlatformConfig, PrComment, PullRequest, normalize_host}; use async_trait::async_trait; use octocrab::Octocrab; use serde::Deserialize; @@ -68,6 +68,7 @@ pub struct GitHubService { impl GitHubService { /// Create a new GitHub service pub fn new(token: &str, owner: String, repo: String, host: Option) -> Result { + let host = host.map(|h| normalize_host(&h)); let mut builder = Octocrab::builder().personal_token(token.to_string()); if let Some(ref h) = host { diff --git a/src/platform/gitlab.rs b/src/platform/gitlab.rs index 2198c5a..4547e88 100644 --- a/src/platform/gitlab.rs +++ b/src/platform/gitlab.rs @@ -2,7 +2,7 @@ use crate::error::{Error, Result}; use crate::platform::PlatformService; -use crate::types::{Platform, PlatformConfig, PrComment, PullRequest}; +use crate::types::{Platform, PlatformConfig, PrComment, PullRequest, normalize_host}; use async_trait::async_trait; use reqwest::Client; use serde::{Deserialize, Serialize}; @@ -64,7 +64,7 @@ const DEFAULT_TIMEOUT_SECS: u64 = 30; impl GitLabService { /// Create a new GitLab service pub fn new(token: String, owner: String, repo: String, host: Option) -> Result { - let host = host.unwrap_or_else(|| "gitlab.com".to_string()); + let host = host.map_or_else(|| "gitlab.com".to_string(), |h| normalize_host(&h)); let project_path = format!("{owner}/{repo}"); let client = Client::builder() diff --git a/src/types.rs b/src/types.rs index 368e1c6..adc1587 100644 --- a/src/types.rs +++ b/src/types.rs @@ -152,3 +152,30 @@ pub struct PlatformConfig { /// Custom host (None for github.com/gitlab.com) pub host: Option, } + +/// Normalize a host value by stripping any protocol prefix and trailing slashes. +/// +/// This handles cases where users provide a full URL (e.g., `https://gitlab.example.com/`) +/// instead of a bare hostname (e.g., `gitlab.example.com`). +/// +/// # Examples +/// +/// ``` +/// use jj_ryu::normalize_host; +/// +/// assert_eq!(normalize_host("gitlab.com"), "gitlab.com"); +/// assert_eq!(normalize_host("https://gitlab.com"), "gitlab.com"); +/// assert_eq!(normalize_host("http://gitlab.com/"), "gitlab.com"); +/// assert_eq!(normalize_host("https://gitlab.example.com/some/path"), "gitlab.example.com"); +/// ``` +#[must_use] +pub fn normalize_host(host: &str) -> String { + let host = host + .trim() + .trim_start_matches("https://") + .trim_start_matches("http://"); + + let host = host.split('/').next().unwrap_or(host); + + host.to_string() +} diff --git a/tests/unit_tests.rs b/tests/unit_tests.rs index baddca6..7ccab95 100644 --- a/tests/unit_tests.rs +++ b/tests/unit_tests.rs @@ -164,6 +164,66 @@ mod analysis_test { } } +mod normalize_host_test { + use jj_ryu::normalize_host; + + #[test] + fn test_bare_hostname_unchanged() { + assert_eq!(normalize_host("gitlab.com"), "gitlab.com"); + assert_eq!( + normalize_host("github.enterprise.com"), + "github.enterprise.com" + ); + } + + #[test] + fn test_strips_https_protocol() { + assert_eq!(normalize_host("https://gitlab.com"), "gitlab.com"); + assert_eq!( + normalize_host("https://gitlab.example.com"), + "gitlab.example.com" + ); + } + + #[test] + fn test_strips_http_protocol() { + assert_eq!(normalize_host("http://gitlab.com"), "gitlab.com"); + assert_eq!( + normalize_host("http://gitlab.example.com"), + "gitlab.example.com" + ); + } + + #[test] + fn test_strips_trailing_slash() { + assert_eq!(normalize_host("gitlab.com/"), "gitlab.com"); + assert_eq!(normalize_host("https://gitlab.com/"), "gitlab.com"); + } + + #[test] + fn test_strips_path() { + assert_eq!(normalize_host("https://gitlab.com/some/path"), "gitlab.com"); + assert_eq!(normalize_host("gitlab.com/api/v4"), "gitlab.com"); + } + + #[test] + fn test_strips_whitespace() { + assert_eq!(normalize_host(" gitlab.com "), "gitlab.com"); + assert_eq!(normalize_host(" https://gitlab.com "), "gitlab.com"); + } + #[test] + fn test_github_enterprise_host() { + assert_eq!( + normalize_host("https://github.mycompany.com"), + "github.mycompany.com" + ); + assert_eq!( + normalize_host("github.mycompany.com"), + "github.mycompany.com" + ); + } +} + mod detection_test { use jj_ryu::error::Error; use jj_ryu::platform::{detect_platform, parse_repo_info};