From d4ec79bb6dc6e92831d417ebaeebb2a47c2e7657 Mon Sep 17 00:00:00 2001 From: Jonathan Reams Date: Thu, 13 Jun 2024 14:22:53 -0400 Subject: [PATCH] RCORE-2096 Trim trailing slashes from base urls to prevent 404 errors (#7791) --- CHANGELOG.md | 1 + src/realm/object-store/sync/app.cpp | 26 +++++++++++++++--- test/object-store/sync/app.cpp | 42 +++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f537899370f..33f7f87f286 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### Enhancements * (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 index 05a421c7ee4..dc9dac57452 100644 --- 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, 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 handle_default_response(UniqueFunction)>&& 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(Private(), config); } @@ -206,7 +224,7 @@ SharedApp App::get_cached_app(const std::string& app_id, const std::optionalsecond; - 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(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 index 98f3cbb703b..aa66c2afc29 100644 --- 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>(); + auto socket_provider = std::make_shared(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 { + 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();