From 3a9c6632e280789921e7567ce040194164e554fb Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Fri, 23 Aug 2024 09:19:54 -0400 Subject: [PATCH] Removed 301/308 redirection support from App --- src/realm/object-store/sync/app.cpp | 150 +++++++--------------- src/realm/object-store/sync/app.hpp | 20 +-- src/realm/object-store/sync/app_utils.cpp | 22 ---- src/realm/object-store/sync/app_utils.hpp | 2 - 4 files changed, 51 insertions(+), 143 deletions(-) diff --git a/src/realm/object-store/sync/app.cpp b/src/realm/object-store/sync/app.cpp index 9306807912a..e66e0ee2084 100644 --- a/src/realm/object-store/sync/app.cpp +++ b/src/realm/object-store/sync/app.cpp @@ -189,7 +189,6 @@ constexpr static std::string_view s_sync_path = "/realm-sync"; constexpr static uint64_t s_default_timeout_ms = 60000; constexpr static std::string_view s_username_password_provider_key = "local-userpass"; constexpr static std::string_view s_user_api_key_provider_key_path = "api_keys"; -constexpr static int s_max_http_redirects = 20; static util::FlatMap> s_apps_cache; // app_id -> base_url -> app std::mutex s_apps_mutex; } // anonymous namespace @@ -978,18 +977,17 @@ void App::delete_user(const std::shared_ptr& user, UniqueFunctionuser_id(); - user->detach_and_tear_down(); - m_metadata_store->delete_user(*m_file_manager, user_id); - emit_change_to_subscribers(); - } - completion(std::move(error)); - }); + do_authenticated_request(HttpMethod::del, url_for_path("/auth/delete"), "", user, RequestTokenType::AccessToken, + [completion = std::move(completion), user, this](const Response& response) { + auto error = AppUtils::check_for_errors(response); + if (!error) { + auto user_id = user->user_id(); + user->detach_and_tear_down(); + m_metadata_store->delete_user(*m_file_manager, user_id); + emit_change_to_subscribers(); + } + completion(std::move(error)); + }); } void App::link_user(const std::shared_ptr& user, const AppCredentials& credentials, @@ -1054,34 +1052,32 @@ std::string App::get_app_route(const Optional& hostname) const } void App::request_location(UniqueFunction)>&& completion, - std::optional&& new_hostname, std::optional&& redir_location, - int redirect_count) -{ - // Request the new location information at the new base url hostname; or redir response location if a redirect - // occurred during the initial location request. redirect_count is used to track the number of sequential - // redirect responses received during the location update and return an error if this count exceeds - // max_http_redirects. If neither new_hostname nor redir_location is provided, the current value of m_base_url - // will be used. - std::string app_route; - std::string base_url; + std::optional&& new_hostname) +{ + // Request the new location information the original configured base_url or the new_hostname + // if the base_url is being updated. If a new_hostname has not been provided and the location + // has already been requested, this function does nothing. + std::string app_route; // The app_route for the server to query the location + std::string base_url; // The configured base_url hostname used for querying the location { util::CheckedUniqueLock lock(m_route_mutex); // Skip if the location info has already been initialized and a new hostname is not provided - if (!new_hostname && !redir_location && m_location_updated) { + if (!new_hostname && m_location_updated) { // Release the lock before calling the completion function lock.unlock(); completion(util::none); return; } - base_url = new_hostname.value_or(m_base_url); - // If this is for a redirect after querying new_hostname, then use the redirect location - if (redir_location) - app_route = get_app_route(redir_location); - // If this is querying the new_hostname, then use that location - else if (new_hostname) + // If this is querying the new_hostname, then use that to query the location + if (new_hostname) { + base_url = *new_hostname; app_route = get_app_route(new_hostname); - else + } + // Otherwise, use the current hostname + else { app_route = get_app_route(); + base_url = m_base_url; + } REALM_ASSERT(!app_route.empty()); } @@ -1093,46 +1089,15 @@ void App::request_location(UniqueFunction)>&& compl log_debug("App: request location: %1", req.url); m_config.transport->send_request_to_server(req, [self = shared_from_this(), completion = std::move(completion), - base_url = std::move(base_url), - redirect_count](const Response& response) mutable { - // Check to see if a redirect occurred - if (AppUtils::is_redirect_status_code(response.http_status_code)) { - // Make sure we don't do too many redirects (max_http_redirects (20) is an arbitrary number) - if (redirect_count >= s_max_http_redirects) { - completion(AppError{ErrorCodes::ClientTooManyRedirects, - util::format("number of redirections exceeded %1", s_max_http_redirects), - {}, - response.http_status_code}); - return; - } - // Handle the redirect response when requesting the location - extract the - // new location header field and resend the request. - auto redir_location = AppUtils::extract_redir_location(response.headers); - if (!redir_location) { - // Location not found in the response, pass error response up the chain - completion(AppError{ErrorCodes::ClientRedirectError, - "Redirect response missing location header", - {}, - response.http_status_code}); - return; - } - // try to request the location info at the new location in the redirect response - // retry_count is passed in to track the number of subsequent redirection attempts - self->request_location(std::move(completion), std::move(base_url), std::move(redir_location), - redirect_count + 1); - return; - } - + base_url = std::move(base_url)](const Response& response) { // Location request was successful - update the location info - auto update_response = self->update_location(response, base_url); - if (update_response) { - self->log_error("App: request location failed (%1%2): %3", update_response->code_string(), - update_response->additional_status_code - ? util::format(" %1", *update_response->additional_status_code) - : "", - update_response->reason()); + auto error = self->update_location(response, base_url); + if (error) { + self->log_error("App: request location failed (%1%2): %3", error->code_string(), + error->additional_status_code ? util::format(" %1", *error->additional_status_code) : "", + error->reason()); } - completion(update_response); + completion(error); }); } @@ -1169,8 +1134,7 @@ std::optional App::update_location(const Response& response, const std return util::none; } -void App::update_location_and_resend(std::unique_ptr&& request, IntermediateCompletion&& completion, - Optional&& redir_location) +void App::update_location_and_resend(std::unique_ptr&& request, IntermediateCompletion&& completion) { // Update the location information if a redirect response was received or m_location_updated == false // and then send the request to the server with request.url updated to the new AppServices hostname. @@ -1192,13 +1156,13 @@ void App::update_location_and_resend(std::unique_ptr&& request, Interme // Retry the original request with the updated url auto& request_ref = *request; self->m_config.transport->send_request_to_server( - request_ref, [self = std::move(self), completion = std::move(completion), - request = std::move(request)](const Response& response) mutable { - self->check_for_redirect_response(std::move(request), response, std::move(completion)); + request_ref, + [completion = std::move(completion), request = std::move(request)](const Response& response) mutable { + completion(std::move(request), response); }); }, // The base_url is not changing for this request - util::none, std::move(redir_location)); + util::none); } void App::post(std::string&& route, UniqueFunction)>&& completion, const BsonDocument& body) @@ -1213,7 +1177,11 @@ void App::post(std::string&& route, UniqueFunction)>&& c void App::do_request(std::unique_ptr&& request, IntermediateCompletion&& completion, bool update_location) { // Verify the request URL to make sure it is valid - util::Uri::parse(request->url); + if (auto valid_url = util::Uri::try_parse(request->url); !valid_url.is_ok()) { + completion(std::move(request), AppUtils::make_apperror_response( + AppError{valid_url.get_status().code(), valid_url.get_status().reason()})); + return; + } // Refresh the location info when app is created or when requested (e.g. after a websocket redirect) // to ensure the http and websocket URL information is up to date. @@ -1235,36 +1203,12 @@ void App::do_request(std::unique_ptr&& request, IntermediateCompletion& // If location info has already been updated, then send the request directly auto& request_ref = *request; m_config.transport->send_request_to_server( - request_ref, [self = shared_from_this(), completion = std::move(completion), - request = std::move(request)](const Response& response) mutable { - self->check_for_redirect_response(std::move(request), response, std::move(completion)); + request_ref, + [completion = std::move(completion), request = std::move(request)](const Response& response) mutable { + completion(std::move(request), response); }); } -void App::check_for_redirect_response(std::unique_ptr&& request, const Response& response, - IntermediateCompletion&& completion) -{ - // If this isn't a redirect response, then we're done - if (!AppUtils::is_redirect_status_code(response.http_status_code)) { - return completion(std::move(request), response); - } - - // Handle a redirect response when sending the original request - extract the location - // header field and resend the request. - auto redir_location = AppUtils::extract_redir_location(response.headers); - if (!redir_location) { - // Location not found in the response, pass error response up the chain - return completion(std::move(request), - AppUtils::make_clienterror_response(ErrorCodes::ClientRedirectError, - "Redirect response missing location header", - response.http_status_code)); - } - - // Request the location info at the new location - once this is complete, the original - // request will be sent to the new server - update_location_and_resend(std::move(request), std::move(completion), std::move(redir_location)); -} - void App::do_authenticated_request(HttpMethod method, std::string&& route, std::string&& body, const std::shared_ptr& user, RequestTokenType token_type, util::UniqueFunction&& completion) diff --git a/src/realm/object-store/sync/app.hpp b/src/realm/object-store/sync/app.hpp index 3fe735ca648..ddee73aeccf 100644 --- a/src/realm/object-store/sync/app.hpp +++ b/src/realm/object-store/sync/app.hpp @@ -525,12 +525,8 @@ class App : public std::enable_shared_from_this, /// a new hostname is provided, the app metadata will be refreshed using the new hostname. /// @param completion The callback that will be called with the error on failure or empty on success /// @param new_hostname The (Original) new hostname to request the location from - /// @param redir_location The location provided by the last redirect response when querying location - /// @param redirect_count The current number of redirects that have occurred in a row - void request_location(util::UniqueFunction)>&& completion, - std::optional&& new_hostname = std::nullopt, - std::optional&& redir_location = std::nullopt, int redirect_count = 0) - REQUIRES(!m_route_mutex); + void request_location(util::UniqueFunction)>&& completion, + std::optional&& new_hostname) REQUIRES(!m_route_mutex); /// Update the location metadata from the location response /// @param response The response returned from the location request @@ -542,9 +538,8 @@ class App : public std::enable_shared_from_this, /// Update the app metadata and resend the request with the updated metadata /// @param request The original request object that needs to be sent after the update /// @param completion The original completion object that will be called with the response to the request - /// @param new_hostname If provided, the metadata will be requested from this hostname - void update_location_and_resend(std::unique_ptr&& request, IntermediateCompletion&& completion, - std::optional&& new_hostname = util::none) REQUIRES(!m_route_mutex); + void update_location_and_resend(std::unique_ptr&& request, IntermediateCompletion&& completion) + REQUIRES(!m_route_mutex); void post(std::string&& route, util::UniqueFunction)>&& completion, const bson::BsonDocument& body) REQUIRES(!m_route_mutex); @@ -559,13 +554,6 @@ class App : public std::enable_shared_from_this, std::unique_ptr make_request(HttpMethod method, std::string&& url, const std::shared_ptr& user, RequestTokenType, std::string&& body) const; - /// Process the redirect response received from the last request that was sent to the server - /// @param request The request to be performed (in case it needs to be sent again) - /// @param response The response from the send_request_to_server operation - /// @param completion Returns the response from the server if not a redirect - void check_for_redirect_response(std::unique_ptr&& request, const Response& response, - IntermediateCompletion&& completion) REQUIRES(!m_route_mutex); - void do_authenticated_request(HttpMethod, std::string&& route, std::string&& body, const std::shared_ptr& user, RequestTokenType, util::UniqueFunction&&) override REQUIRES(!m_route_mutex); diff --git a/src/realm/object-store/sync/app_utils.cpp b/src/realm/object-store/sync/app_utils.cpp index 2226496ea87..82e10746998 100644 --- a/src/realm/object-store/sync/app_utils.cpp +++ b/src/realm/object-store/sync/app_utils.cpp @@ -50,28 +50,6 @@ bool AppUtils::is_success_status_code(int status_code) return status_code == 0 || (status_code < 300 && status_code >= 200); } -bool AppUtils::is_redirect_status_code(int status_code) -{ - using namespace realm::sync; - // If the response contains a redirection, then return true - if (auto code = HTTPStatus(status_code); - code == HTTPStatus::MovedPermanently || code == HTTPStatus::PermanentRedirect) { - return true; - } - return false; -} - -std::optional AppUtils::extract_redir_location(const std::map& headers) -{ - // Look for case insensitive redirect "location" in headers - auto location = AppUtils::find_header("location", headers); - if (location && !location->second.empty() && util::Uri::try_parse(location->second).is_ok()) { - // If the location is valid, return it wholesale (e.g., it could include a path for API proxies) - return location->second; - } - return std::nullopt; -} - // Create a Response object with the given client error, message and optional http status code Response AppUtils::make_clienterror_response(ErrorCodes::Error code, const std::string_view message, std::optional http_status) diff --git a/src/realm/object-store/sync/app_utils.hpp b/src/realm/object-store/sync/app_utils.hpp index 2f6d1e4f666..371bbb3e45b 100644 --- a/src/realm/object-store/sync/app_utils.hpp +++ b/src/realm/object-store/sync/app_utils.hpp @@ -38,8 +38,6 @@ class AppUtils { static const std::pair* find_header(const std::string& key_name, const std::map& search_map); static bool is_success_status_code(int status_code); - static bool is_redirect_status_code(int status_code); - static std::optional extract_redir_location(const std::map& headers); }; } // namespace realm::app