sdk: Check if server name points to homeserver during discovery (#3218)

The small first commit reintroduces `sanitize_server_name` in the public API. In Fractal, we use it to validate the string in the input before allowing the user to trigger the discovery.

The main commit changes a bit the discovery process: before, server names like `example.org` would only be checked for the presence of a well-known, and only URLs like `https://example.org` would be checked as a homeserver. Now, providing `example.org` will also check if `https://example.org` is the URL of a homeserver.

Sadly I don't think it's possible to add tests for it as it would require to have a homeserver accessible via HTTPS.

---

* sdk: Restore sanitize_server_name in the public API

Signed-off-by: Kévin Commaille <[email protected]>

* sdk: Check if a provided server name points to a homeserver during discovery

Before, only URLs like `https://example.org` would be checked as a homeserver.
Providing `example.org` will check if `https://example.org` is the URL of a homeserver.

Signed-off-by: Kévin Commaille <[email protected]>

* Refactor to avoid duplication

Signed-off-by: Kévin Commaille <[email protected]>

---------

Signed-off-by: Kévin Commaille <[email protected]>

Author: zecakeh

crates/matrix-sdk/src/client/builder.rs

@@ -418,48 +418,8 @@ impl ClientBuilder {
             }
 
             HomeserverConfig::ServerNameOrUrl(server_name_or_url) => {
-                // Store the result to return at the end. If this doesn't get modified, then the
-                // supplied name is neither a server name, nor a valid URL.
-                let mut homeserver_details: Option<(
-                    String,
-                    Option<discover_homeserver::Response>,
-                )> = None;
-                let mut discovery_error: Option<ClientBuildError> = None;
-
-                // Attempt discovery as a server name first.
-                let sanitize_result = sanitize_server_name(&server_name_or_url);
-                if let Ok(server_name) = sanitize_result.as_ref() {
-                    let protocol = if server_name_or_url.starts_with("http://") {
-                        UrlScheme::Http
-                    } else {
-                        UrlScheme::Https
-                    };
-
-                    match discover_homeserver(server_name.clone(), protocol, &http_client).await {
-                        Ok(well_known) => {
-                            homeserver_details =
-                                Some((well_known.homeserver.base_url.clone(), Some(well_known)));
-                        }
-                        Err(e) => {
-                            debug!(error = %e, "Well-known discovery failed.");
-                            discovery_error = Some(e);
-                        }
-                    }
-                }
-
-                // When discovery fails, or the input isn't a valid server name, fallback to
-                // trying a homeserver URL if supplied.
-                if homeserver_details.is_none() {
-                    if let Ok(homeserver_url) = Url::parse(&server_name_or_url) {
-                        // Make sure the URL is definitely for a homeserver.
-                        if check_is_homeserver(&homeserver_url, &http_client).await {
-                            homeserver_details = Some((homeserver_url.to_string(), None));
-                        }
-                    }
-                }
-
-                homeserver_details
-                    .ok_or(discovery_error.unwrap_or(ClientBuildError::InvalidServerName))?
+                discover_homeserver_from_server_name_or_url(server_name_or_url, &http_client)
+                    .await?
             }
         };
 
@@ -522,10 +482,58 @@ impl ClientBuilder {
     }
 }
 
+/// Discovers a homeserver from a server name or a URL.
+///
+/// Tries well-known discovery and checking if the URL points to a homeserver.
+async fn discover_homeserver_from_server_name_or_url(
+    mut server_name_or_url: String,
+    http_client: &HttpClient,
+) -> Result<(String, Option<discover_homeserver::Response>), ClientBuildError> {
+    let mut discovery_error: Option<ClientBuildError> = None;
+
+    // Attempt discovery as a server name first.
+    let sanitize_result = sanitize_server_name(&server_name_or_url);
+
+    if let Ok(server_name) = sanitize_result.as_ref() {
+        let protocol = if server_name_or_url.starts_with("http://") {
+            UrlScheme::Http
+        } else {
+            UrlScheme::Https
+        };
+
+        match discover_homeserver(server_name.clone(), protocol, http_client).await {
+            Ok(well_known) => {
+                return Ok((well_known.homeserver.base_url.clone(), Some(well_known)));
+            }
+            Err(e) => {
+                debug!(error = %e, "Well-known discovery failed.");
+                discovery_error = Some(e);
+
+                // Check if the server name points to a homeserver.
+                server_name_or_url = match protocol {
+                    UrlScheme::Http => format!("http://{server_name}"),
+                    UrlScheme::Https => format!("https://{server_name}"),
+                }
+            }
+        }
+    }
+
+    // When discovery fails, or the input isn't a valid server name, fallback to
+    // trying a homeserver URL.
+    if let Ok(homeserver_url) = Url::parse(&server_name_or_url) {
+        // Make sure the URL is definitely for a homeserver.
+        if check_is_homeserver(&homeserver_url, http_client).await {
+            return Ok((homeserver_url.to_string(), None));
+        }
+    }
+
+    Err(discovery_error.unwrap_or(ClientBuildError::InvalidServerName))
+}
+
 /// Creates a server name from a user supplied string. The string is first
 /// sanitized by removing whitespace, the http(s) scheme and any trailing
 /// slashes before being parsed.
-fn sanitize_server_name(s: &str) -> crate::Result<OwnedServerName, IdParseError> {
+pub fn sanitize_server_name(s: &str) -> crate::Result<OwnedServerName, IdParseError> {
     ServerName::parse(
         s.trim().trim_start_matches("http://").trim_start_matches("https://").trim_end_matches('/'),
     )

crates/matrix-sdk/src/client/mod.rs

@@ -97,7 +97,7 @@ use crate::{
 mod builder;
 pub(crate) mod futures;
 
-pub use self::builder::{ClientBuildError, ClientBuilder};
+pub use self::builder::{sanitize_server_name, ClientBuildError, ClientBuilder};
 
 #[cfg(not(target_arch = "wasm32"))]
 type NotificationHandlerFut = Pin<Box<dyn Future<Output = ()> + Send>>;

crates/matrix-sdk/src/lib.rs

@@ -62,7 +62,9 @@ pub mod widget;
 
 pub use account::Account;
 pub use authentication::{AuthApi, AuthSession, SessionTokens};
-pub use client::{Client, ClientBuildError, ClientBuilder, LoopCtrl, SessionChange};
+pub use client::{
+    sanitize_server_name, Client, ClientBuildError, ClientBuilder, LoopCtrl, SessionChange,
+};
 #[cfg(feature = "image-proc")]
 pub use error::ImageError;
 pub use error::{