RCORE-2096 Trim trailing slashes from base urls to prevent 404 errors (…

…#7791)
realm · Jun 13, 2024 · d4ec79b · d4ec79b
1 parent b576cbc
commit d4ec79b
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ### Enhancements
 * <New feature description> (PR [#????](https://github.com/realm/realm-core/pull/????))
+* It is no longer an error to set a base url for an App with a trailing slash - for example, `https://services.cloud.mongodb.com/` instead of `https://services.cloud.mongodb.com` - before this change that would result in a 404 error from the server ([PR #7791](https://github.com/realm/realm-core/pull/7791)).
 * Performance has been improved for range queries on integers and timestamps. Requires that you use the "BETWEEN" operation in MQL or the Query::between() method when you build the query. (PR [#7785](https://github.com/realm/realm-core/pull/7785))
 * None.
 

diff --git a/src/realm/object-store/sync/app.cpp b/src/realm/object-store/sync/app.cpp
@@ -157,6 +157,24 @@ HttpHeaders get_request_headers(const std::shared_ptr<User>& user, RequestTokenT
     return headers;
 }
 
+std::string trim_base_url(std::string base_url)
+{
+    while (!base_url.empty() && base_url.back() == '/') {
+        base_url.pop_back();
+    }
+
+    return base_url;
+}
+
+std::string base_url_from_app_config(const AppConfig& app_config)
+{
+    if (!app_config.base_url) {
+        return std::string{App::default_base_url()};
+    }
+
+    return trim_base_url(*app_config.base_url);
+}
+
 UniqueFunction<void(const Response&)> handle_default_response(UniqueFunction<void(Optional<AppError>)>&& completion)
 {
     return [completion = std::move(completion)](const Response& response) {
@@ -190,7 +208,7 @@ SharedApp App::get_app(CacheMode mode, const AppConfig& config) NO_THREAD_SAFETY
 {
     if (mode == CacheMode::Enabled) {
         std::lock_guard lock(s_apps_mutex);
-        auto& app = s_apps_cache[config.app_id][config.base_url.value_or(std::string(App::default_base_url()))];
+        auto& app = s_apps_cache[config.app_id][base_url_from_app_config(config)];
         if (!app) {
             app = std::make_shared<App>(Private(), config);
         }
@@ -206,7 +224,7 @@ SharedApp App::get_cached_app(const std::string& app_id, const std::optional<std
     if (auto it = s_apps_cache.find(app_id); it != s_apps_cache.end()) {
         const auto& apps_by_url = it->second;
 
-        auto app_it = base_url ? apps_by_url.find(*base_url) : apps_by_url.begin();
+        auto app_it = base_url ? apps_by_url.find(trim_base_url(*base_url)) : apps_by_url.begin();
         if (app_it != apps_by_url.end()) {
             return app_it->second;
         }
@@ -233,7 +251,7 @@ void App::close_all_sync_sessions()
 
 App::App(Private, const AppConfig& config)
     : m_config(config)
-    , m_base_url(m_config.base_url.value_or(std::string(App::default_base_url())))
+    , m_base_url(base_url_from_app_config(m_config))
     , m_request_timeout_ms(m_config.default_request_timeout_ms.value_or(s_default_timeout_ms))
     , m_file_manager(std::make_unique<SyncFileManager>(config))
     , m_metadata_store(create_metadata_store(config, *m_file_manager))
@@ -393,7 +411,7 @@ void App::update_hostname(const std::string& host_url, const std::string& ws_hos
                           const std::string& new_base_url)
 {
     log_debug("App: update_hostname: %1 | %2 | %3", host_url, ws_host_url, new_base_url);
-    m_base_url = new_base_url;
+    m_base_url = trim_base_url(new_base_url);
     // If a new host url was returned from the server, use it to configure the routes
     // Otherwise, use the m_base_url value
     std::string base_url = host_url.length() > 0 ? host_url : m_base_url;

diff --git a/test/object-store/sync/app.cpp b/test/object-store/sync/app.cpp
@@ -3282,6 +3282,48 @@ TEST_CASE("app: sync integration", "[sync][pbs][app][baas]") {
     }
 }
 
+
+TEST_CASE("app: trailing slash in base url", "[sync][app]") {
+    auto logger = util::Logger::get_default_logger();
+
+    const auto schema = get_default_schema();
+
+    SyncServer server({});
+    auto transport = std::make_shared<HookedTransport<UnitTestTransport>>();
+    auto socket_provider = std::make_shared<HookedSocketProvider>(logger, "");
+    OfflineAppSession::Config oas_config(transport);
+    oas_config.base_url = util::format("http://localhost:%1/", server.port());
+    oas_config.socket_provider = socket_provider;
+    OfflineAppSession oas(oas_config);
+    AutoVerifiedEmailCredentials creds;
+    auto app = oas.app();
+    const auto partition = random_string(100);
+
+    transport->request_hook = [&](const Request& req) -> std::optional<Response> {
+        if (req.url.find("/location") == std::string::npos) {
+            return std::nullopt;
+        }
+
+        REQUIRE(req.url == util::format("http://localhost:%1/api/client/v2.0/app/app_id/location", server.port()));
+        return Response{
+            200,
+            0,
+            {},
+            nlohmann::json(nlohmann::json::object({
+                               {"hostname", util::format("http://localhost:%1", server.port())},
+                               {"ws_hostname", util::format("ws://localhost:%1", server.port())},
+                               {"sync_route", util::format("ws://localhost:%1/realm-sync", server.port())},
+                           }))
+                .dump(),
+        };
+    };
+
+    SyncTestFile realm_config(oas, "test");
+
+    auto r = Realm::get_shared_realm(realm_config);
+    REQUIRE(!wait_for_download(*r));
+}
+
 TEST_CASE("app: redirect handling", "[sync][pbs][app]") {
     auto logger = util::Logger::get_default_logger();