fix(config): carry the public port into derived endpoints (#2329)

fluxer_app_proxy/src/bootstrap.rs

@@ -2,6 +2,7 @@
 
 use crate::config::AppProxyConfig;
 use crate::discovery_cache::DiscoveryResponse;
+use reqwest::Url;
 use serde::Serialize;
 
 #[derive(Serialize)]
@@ -43,11 +44,14 @@ pub fn build_bootstrap_script(
     geoip: &serde_json::Value,
     nonce: &str,
 ) -> String {
+    let api_public_endpoint =
+        api_public_endpoint(config.bootstrap_api_public_endpoint.as_deref(), discovery);
+
     let payload = BootstrapPayload {
         config: BootstrapConfig {
             release_channel: config.release_channel.as_str(),
             bootstrap_api_endpoint: &config.bootstrap_api_endpoint,
-            bootstrap_api_public_endpoint: config.bootstrap_api_public_endpoint.as_deref(),
+            bootstrap_api_public_endpoint: api_public_endpoint,
         },
         instance: &discovery.data,
         geoip,
@@ -56,7 +60,7 @@ pub fn build_bootstrap_script(
     let legacy = LegacyConfig {
         release_channel: config.release_channel.as_str(),
         bootstrap_api_endpoint: &config.bootstrap_api_endpoint,
-        bootstrap_api_public_endpoint: config.bootstrap_api_public_endpoint.as_deref(),
+        bootstrap_api_public_endpoint: api_public_endpoint,
     };
 
     let bootstrap_json = escape_json_for_script(&serde_json::to_string(&payload).unwrap());
@@ -67,6 +71,50 @@ pub fn build_bootstrap_script(
     )
 }
 
+fn api_public_endpoint<'a>(
+    configured: Option<&'a str>,
+    discovery: &'a DiscoveryResponse,
+) -> Option<&'a str> {
+    let configured = configured?;
+    let Some(discovered) = discovered_api_public(discovery) else {
+        return Some(configured);
+    };
+    if has_explicit_port(configured) || !has_explicit_port(discovered) {
+        return Some(configured);
+    }
+    let (Ok(configured_url), Ok(discovered_url)) = (Url::parse(configured), Url::parse(discovered))
+    else {
+        return Some(configured);
+    };
+    if configured_url.scheme() != discovered_url.scheme()
+        || configured_url.host_str() != discovered_url.host_str()
+        || configured_url.path() != discovered_url.path()
+    {
+        return Some(configured);
+    }
+    Some(discovered)
+}
+
+fn discovered_api_public(discovery: &DiscoveryResponse) -> Option<&str> {
+    discovery
+        .data
+        .get("endpoints")
+        .and_then(|endpoints| endpoints.get("api_public"))
+        .and_then(serde_json::Value::as_str)
+        .map(str::trim)
+        .filter(|value| !value.is_empty())
+}
+
+fn has_explicit_port(url: &str) -> bool {
+    let Some(scheme_end) = url.find("://") else {
+        return false;
+    };
+    let rest = &url[scheme_end + 3..];
+    let authority_end = rest.find(['/', '\\', '?', '#']).unwrap_or(rest.len());
+    let host = rest[..authority_end].rsplit('@').next().unwrap_or_default();
+    host[host.rfind(']').map_or(0, |index| index + 1)..].contains(':')
+}
+
 const MEDIA_PRECONNECT_TAG: &str = r#"<link rel="preconnect" href="{{MEDIA_ENDPOINT}}">"#;
 const STATIC_PRECONNECT_TAGS: [&str; 2] = [
     r#"<link rel="preconnect" href="{{STATIC_CDN_ENDPOINT}}">"#,
@@ -411,4 +459,108 @@ mod tests {
         let json = serde_json::to_string(&payload).unwrap();
         assert!(!json.contains("bootstrapApiPublicEndpoint"));
     }
+
+    fn discovery_offering(api_public: &str) -> DiscoveryResponse {
+        DiscoveryResponse {
+            data: serde_json::json!({"endpoints": {"api_public": api_public}}),
+        }
+    }
+
+    #[test]
+    fn the_discovered_port_repairs_a_portless_configured_endpoint() {
+        let discovery = discovery_offering("https://chat.example.test:8443/api");
+        assert_eq!(
+            api_public_endpoint(Some("https://chat.example.test/api"), &discovery),
+            Some("https://chat.example.test:8443/api")
+        );
+    }
+
+    #[test]
+    fn a_configured_endpoint_that_already_carries_a_port_is_left_alone() {
+        let discovery = discovery_offering("https://chat.example.test:8443/api");
+        assert_eq!(
+            api_public_endpoint(Some("https://chat.example.test:9443/api"), &discovery),
+            Some("https://chat.example.test:9443/api")
+        );
+    }
+
+    #[test]
+    fn a_discovered_endpoint_on_another_host_is_ignored() {
+        let discovery = discovery_offering("https://api.example.test:8443/api");
+        assert_eq!(
+            api_public_endpoint(Some("https://chat.example.test/api"), &discovery),
+            Some("https://chat.example.test/api")
+        );
+    }
+
+    #[test]
+    fn a_discovered_endpoint_on_another_path_is_ignored() {
+        let discovery = discovery_offering("https://chat.example.test:8443/v9/api");
+        assert_eq!(
+            api_public_endpoint(Some("https://chat.example.test/api"), &discovery),
+            Some("https://chat.example.test/api")
+        );
+    }
+
+    #[test]
+    fn a_discovered_endpoint_on_another_scheme_is_ignored() {
+        let discovery = discovery_offering("http://chat.example.test:8443/api");
+        assert_eq!(
+            api_public_endpoint(Some("https://chat.example.test/api"), &discovery),
+            Some("https://chat.example.test/api")
+        );
+    }
+
+    #[test]
+    fn a_discovery_document_without_a_snapshot_leaves_the_configured_endpoint_alone() {
+        let discovery = DiscoveryResponse {
+            data: serde_json::json!({}),
+        };
+        assert_eq!(
+            api_public_endpoint(Some("https://chat.example.test/api"), &discovery),
+            Some("https://chat.example.test/api")
+        );
+    }
+
+    #[test]
+    fn a_discovery_document_without_a_public_api_endpoint_changes_nothing() {
+        let discovery = DiscoveryResponse {
+            data: serde_json::json!({"endpoints": {"api_public": "   "}}),
+        };
+        assert_eq!(
+            api_public_endpoint(Some("https://chat.example.test/api"), &discovery),
+            Some("https://chat.example.test/api")
+        );
+    }
+
+    #[test]
+    fn a_portless_discovered_endpoint_leaves_a_default_install_alone() {
+        let discovery = discovery_offering("https://chat.example.test/api");
+        assert_eq!(
+            api_public_endpoint(Some("https://chat.example.test/api"), &discovery),
+            Some("https://chat.example.test/api")
+        );
+    }
+
+    #[test]
+    fn an_unconfigured_public_endpoint_is_never_invented_from_discovery() {
+        let discovery = discovery_offering("https://chat.example.test:8443/api");
+        assert_eq!(api_public_endpoint(None, &discovery), None);
+    }
+
+    #[test]
+    fn the_boot_script_hands_the_repaired_endpoint_to_both_globals() {
+        let discovery = discovery_offering("https://chat.example.test:8443/api");
+        let mut config = AppProxyConfig::from_env();
+        config.bootstrap_api_public_endpoint = Some("https://chat.example.test/api".to_owned());
+        let script =
+            build_bootstrap_script(&config, &discovery, &serde_json::json!({}), "scriptnonce");
+        assert!(
+            script.contains(r#""bootstrapApiPublicEndpoint":"https://chat.example.test:8443/api""#)
+        );
+        assert!(script.contains(
+            r#""PUBLIC_BOOTSTRAP_API_PUBLIC_ENDPOINT":"https://chat.example.test:8443/api""#
+        ));
+        assert!(!script.contains(r#""https://chat.example.test/api""#));
+    }
 }

fluxer_app_proxy/src/config.rs

@@ -200,9 +200,7 @@ impl AppProxyConfig {
                 env!("CARGO_PKG_VERSION"),
             ),
             bootstrap_api_endpoint: cfg::read_env("PUBLIC_BOOTSTRAP_API_ENDPOINT", "/api"),
-            bootstrap_api_public_endpoint: cfg::non_empty_env(
-                "PUBLIC_BOOTSTRAP_API_PUBLIC_ENDPOINT",
-            ),
+            bootstrap_api_public_endpoint: resolve_bootstrap_api_public_endpoint_from_env(),
             csp: CspConfig::from_env(),
             geoip_source,
             geoip_s3_config,
@@ -275,6 +273,27 @@ fn resolve_postgres_prepared_statements_from_env() -> bool {
     resolve_postgres_prepared_statements(|name| env::var(name).ok())
 }
 
+fn resolve_bootstrap_api_public_endpoint_from_env() -> Option<String> {
+    resolve_bootstrap_api_public_endpoint(|name| env::var(name).ok())
+}
+
+fn resolve_bootstrap_api_public_endpoint<F>(mut read_var: F) -> Option<String>
+where
+    F: FnMut(&str) -> Option<String>,
+{
+    let endpoint = read_var("PUBLIC_BOOTSTRAP_API_PUBLIC_ENDPOINT")
+        .map(|value| value.trim().to_owned())
+        .filter(|value| !value.is_empty())?;
+    let base_domain = read_var("FLUXER_BASE_DOMAIN").unwrap_or_default();
+    let public_port = read_var("FLUXER_PUBLIC_PORT").and_then(|port| port.trim().parse().ok());
+
+    Some(cfg::normalize_public_endpoint(
+        &endpoint,
+        &base_domain,
+        public_port,
+    ))
+}
+
 fn resolve_time_freeze_enabled<F>(mut read_var: F) -> bool
 where
     F: FnMut(&str) -> Option<String>,
@@ -373,6 +392,68 @@ mod tests {
         resolve_postgres_prepared_statements(|name| env.get(name).map(|value| value.to_string()))
     }
 
+    fn resolve_bootstrap_endpoint_from_pairs(pairs: &[(&str, &str)]) -> Option<String> {
+        let env: HashMap<&str, &str> = pairs.iter().copied().collect();
+        resolve_bootstrap_api_public_endpoint(|name| env.get(name).map(|value| value.to_string()))
+    }
+
+    #[test]
+    fn a_non_default_public_port_reaches_the_boot_html_api_endpoint() {
+        assert_eq!(
+            resolve_bootstrap_endpoint_from_pairs(&[
+                (
+                    "PUBLIC_BOOTSTRAP_API_PUBLIC_ENDPOINT",
+                    "http://fluxer.example/api",
+                ),
+                ("FLUXER_BASE_DOMAIN", "fluxer.example"),
+                ("FLUXER_PUBLIC_PORT", "19080"),
+            ]),
+            Some("http://fluxer.example:19080/api".to_owned())
+        );
+    }
+
+    #[test]
+    fn a_default_public_port_leaves_the_boot_html_api_endpoint_alone() {
+        assert_eq!(
+            resolve_bootstrap_endpoint_from_pairs(&[
+                (
+                    "PUBLIC_BOOTSTRAP_API_PUBLIC_ENDPOINT",
+                    "https://fluxer.example/api",
+                ),
+                ("FLUXER_BASE_DOMAIN", "fluxer.example"),
+                ("FLUXER_PUBLIC_PORT", "443"),
+            ]),
+            Some("https://fluxer.example/api".to_owned())
+        );
+    }
+
+    #[test]
+    fn the_boot_html_api_endpoint_keeps_a_port_it_already_carries() {
+        assert_eq!(
+            resolve_bootstrap_endpoint_from_pairs(&[
+                (
+                    "PUBLIC_BOOTSTRAP_API_PUBLIC_ENDPOINT",
+                    "http://fluxer.example:19080/api",
+                ),
+                ("FLUXER_BASE_DOMAIN", "fluxer.example"),
+                ("FLUXER_PUBLIC_PORT", "19080"),
+            ]),
+            Some("http://fluxer.example:19080/api".to_owned())
+        );
+    }
+
+    #[test]
+    fn the_boot_html_api_endpoint_is_untouched_without_a_base_domain_and_port() {
+        assert_eq!(
+            resolve_bootstrap_endpoint_from_pairs(&[(
+                "PUBLIC_BOOTSTRAP_API_PUBLIC_ENDPOINT",
+                "http://fluxer.example/api",
+            )]),
+            Some("http://fluxer.example/api".to_owned())
+        );
+        assert_eq!(resolve_bootstrap_endpoint_from_pairs(&[]), None);
+    }
+
     #[test]
     fn csp_config_default_has_no_extra_sources() {
         let c = CspConfig::default();