From 122d5326593dc97e99785f394756c27e0ed14aab Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 7 Feb 2024 16:53:29 -0800 Subject: [PATCH 1/6] Refactor RealmJWT to support validating tokens with minimal allocations --- src/realm/obj.cpp | 6 +- src/realm/object-store/CMakeLists.txt | 28 +++--- src/realm/object-store/c_api/app.cpp | 2 +- src/realm/object-store/sync/app.cpp | 2 +- src/realm/object-store/sync/jwt.cpp | 89 +++++++++++++++++++ src/realm/object-store/sync/jwt.hpp | 66 ++++++++++++++ src/realm/object-store/sync/sync_user.cpp | 47 ---------- src/realm/object-store/sync/sync_user.hpp | 28 +----- src/realm/parser/driver.cpp | 2 +- src/realm/sync/network/websocket.cpp | 8 +- src/realm/sync/noinst/protocol_codec.cpp | 2 +- src/realm/sync/noinst/server/access_token.cpp | 7 +- src/realm/sync/tools/print_changeset.cpp | 5 +- src/realm/util/base64.cpp | 44 ++++----- src/realm/util/base64.hpp | 29 +++--- src/realm/util/bson/bson.cpp | 10 ++- src/realm/util/bson/bson.hpp | 4 +- src/realm/util/serializer.cpp | 2 +- src/realm/uuid.cpp | 3 +- test/object-store/c_api/c_api.cpp | 10 +-- test/object-store/realm.cpp | 11 +-- test/object-store/sync/app.cpp | 23 ++--- test/object-store/util/test_utils.cpp | 4 +- test/test_table.cpp | 4 +- test/test_util_base64.cpp | 28 +++--- 25 files changed, 265 insertions(+), 199 deletions(-) create mode 100644 src/realm/object-store/sync/jwt.cpp create mode 100644 src/realm/object-store/sync/jwt.hpp diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 00b56039f1a..3ca7530a20a 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -898,11 +898,9 @@ void out_string(std::ostream& out, std::string str) void out_binary(std::ostream& out, BinaryData bin) { - const char* start = bin.data(); - const size_t len = bin.size(); std::string encode_buffer; - encode_buffer.resize(util::base64_encoded_size(len)); - util::base64_encode(start, len, encode_buffer.data(), encode_buffer.size()); + encode_buffer.resize(util::base64_encoded_size(bin.size())); + util::base64_encode(bin, encode_buffer); out << encode_buffer; } diff --git a/src/realm/object-store/CMakeLists.txt b/src/realm/object-store/CMakeLists.txt index 083e8a10a7c..90177165ad2 100644 --- a/src/realm/object-store/CMakeLists.txt +++ b/src/realm/object-store/CMakeLists.txt @@ -92,20 +92,21 @@ set(HEADERS if(REALM_ENABLE_SYNC) list(APPEND HEADERS sync/app.hpp - sync/app_utils.hpp sync/app_credentials.hpp - sync/generic_network_transport.hpp - sync/async_open_task.hpp - sync/sync_manager.hpp - sync/sync_session.hpp - sync/sync_user.hpp sync/app_service_client.hpp + sync/app_utils.hpp + sync/async_open_task.hpp sync/auth_request_client.hpp + sync/generic_network_transport.hpp + sync/jwt.hpp sync/mongo_client.hpp sync/mongo_collection.hpp sync/mongo_database.hpp sync/push_client.hpp sync/subscribable.hpp + sync/sync_manager.hpp + sync/sync_session.hpp + sync/sync_user.hpp sync/impl/sync_client.hpp sync/impl/sync_file.hpp @@ -114,19 +115,20 @@ if(REALM_ENABLE_SYNC) list(APPEND SOURCES sync/app.cpp - sync/app_utils.cpp sync/app_credentials.cpp - sync/generic_network_transport.cpp + sync/app_utils.cpp sync/async_open_task.cpp - sync/sync_manager.cpp - sync/sync_session.cpp - sync/sync_user.cpp + sync/generic_network_transport.cpp + sync/impl/sync_file.cpp + sync/impl/sync_metadata.cpp + sync/jwt.cpp sync/mongo_client.cpp sync/mongo_collection.cpp sync/mongo_database.cpp sync/push_client.cpp - sync/impl/sync_file.cpp - sync/impl/sync_metadata.cpp) + sync/sync_manager.cpp + sync/sync_session.cpp + sync/sync_user.cpp) if(APPLE) list(APPEND HEADERS sync/impl/apple/network_reachability_observer.hpp diff --git a/src/realm/object-store/c_api/app.cpp b/src/realm/object-store/c_api/app.cpp index 39ca03da35d..f43bef96bb3 100644 --- a/src/realm/object-store/c_api/app.cpp +++ b/src/realm/object-store/c_api/app.cpp @@ -121,7 +121,7 @@ static inline bson::BsonArray parse_ejson_array(const char* serialized) return {}; } else { - return bson::BsonArray(bson::parse(serialized)); + return bson::BsonArray(bson::parse({serialized, strlen(serialized)})); } } diff --git a/src/realm/object-store/sync/app.cpp b/src/realm/object-store/sync/app.cpp index 56cf1e5315e..60a8745f023 100644 --- a/src/realm/object-store/sync/app.cpp +++ b/src/realm/object-store/sync/app.cpp @@ -1344,7 +1344,7 @@ Request App::make_streaming_request(const std::shared_ptr& user, const const auto args_json = Bson(args).to_string(); auto args_base64 = std::string(util::base64_encoded_size(args_json.size()), '\0'); - util::base64_encode(args_json.data(), args_json.size(), args_base64.data(), args_base64.size()); + util::base64_encode(args_json, args_base64); auto url = function_call_url_path() + "?baas_request=" + util::uri_percent_encode(args_base64); if (user) { diff --git a/src/realm/object-store/sync/jwt.cpp b/src/realm/object-store/sync/jwt.cpp new file mode 100644 index 00000000000..ab5560acf90 --- /dev/null +++ b/src/realm/object-store/sync/jwt.cpp @@ -0,0 +1,89 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2024 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#include + +#include +#include + +namespace realm { + +static std::string_view split_token(std::string_view jwt) noexcept +{ + constexpr static char delimiter = '.'; + + auto pos = jwt.find(delimiter); + if (pos == 0 || pos == jwt.npos) { + return {}; + } + jwt = jwt.substr(pos + 1); + + pos = jwt.find(delimiter); + if (pos == jwt.npos) { + return {}; + } + auto payload = jwt.substr(0, pos); + jwt = jwt.substr(pos + 1); + + // We don't use the signature, but verify one is present + if (jwt.size() == 0 || std::count(jwt.begin(), jwt.end(), '.') != 0) { + return {}; + } + + return payload; +} + +RealmJWT::RealmJWT(std::string_view token) +{ + auto payload = split_token(token); + if (!payload.data()) { + throw app::AppError(ErrorCodes::BadToken, "malformed JWT"); + } + + auto json_str = util::base64_decode_to_vector(payload); + if (!json_str) { + throw app::AppError(ErrorCodes::BadToken, "JWT payload could not be base64 decoded"); + } + + auto json = static_cast(bson::parse(*json_str)); + + this->token = token; + this->expires_at = static_cast(json["exp"]); + this->issued_at = static_cast(json["iat"]); + + if (json.find("user_data") != json.end()) { + this->user_data = static_cast(json["user_data"]); + } +} + +bool RealmJWT::validate(std::string_view token) +{ + auto payload = split_token(token); + if (!payload.data()) { + return false; + } + + auto json_str = util::base64_decode_to_vector(payload); + if (!json_str) { + return false; + } + + return bson::accept(*json_str); +} + +} // namespace realm diff --git a/src/realm/object-store/sync/jwt.hpp b/src/realm/object-store/sync/jwt.hpp new file mode 100644 index 00000000000..7e7b2f7dc01 --- /dev/null +++ b/src/realm/object-store/sync/jwt.hpp @@ -0,0 +1,66 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2024 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#ifndef REALM_OS_JWT_HPP +#define REALM_OS_JWT_HPP + +#include + +#include +#include + +namespace realm { +// A struct that decodes a given JWT. +struct RealmJWT { + // The raw encoded token + std::string token; + + // When the token expires. + int64_t expires_at = 0; + // When the token was issued. + int64_t issued_at = 0; + // Custom user data embedded in the encoded token. + std::optional user_data; + + RealmJWT() = default; + explicit RealmJWT(std::string_view token); + explicit RealmJWT(const std::string& token) + : RealmJWT(std::string_view(token)) + { + } + + static bool validate(std::string_view token); + + bool operator==(const RealmJWT& other) const noexcept + { + return token == other.token; + } + bool operator!=(const RealmJWT& other) const noexcept + { + return token != other.token; + } + + explicit operator bool() const noexcept + { + return !token.empty(); + } +}; + +} // namespace realm + +#endif // REALM_OS_JWT_HPP diff --git a/src/realm/object-store/sync/sync_user.cpp b/src/realm/object-store/sync/sync_user.cpp index 2dc130d8a6a..8c141c8fc28 100644 --- a/src/realm/object-store/sync/sync_user.cpp +++ b/src/realm/object-store/sync/sync_user.cpp @@ -26,55 +26,8 @@ #include #include -#include - namespace realm { -static std::string base64_decode(const std::string& in) -{ - std::string out; - out.resize(util::base64_decoded_size(in.size())); - util::base64_decode(in, &out[0], out.size()); - return out; -} - -static std::vector split_token(const std::string& jwt) -{ - constexpr static char delimiter = '.'; - - std::vector parts; - size_t pos = 0, start_from = 0; - - while ((pos = jwt.find(delimiter, start_from)) != std::string::npos) { - parts.push_back(jwt.substr(start_from, pos - start_from)); - start_from = pos + 1; - } - - parts.push_back(jwt.substr(start_from)); - - if (parts.size() != 3) { - throw app::AppError(ErrorCodes::BadToken, "jwt missing parts"); - } - - return parts; -} - -RealmJWT::RealmJWT(const std::string& token) - : token(token) -{ - auto parts = split_token(this->token); - - auto json_str = base64_decode(parts[1]); - auto json = static_cast(bson::parse(json_str)); - - this->expires_at = static_cast(json["exp"]); - this->issued_at = static_cast(json["iat"]); - - if (json.find("user_data") != json.end()) { - this->user_data = static_cast(json["user_data"]); - } -} - SyncUserIdentity::SyncUserIdentity(const std::string& id, const std::string& provider_type) : id(id) , provider_type(provider_type) diff --git a/src/realm/object-store/sync/sync_user.hpp b/src/realm/object-store/sync/sync_user.hpp index 9be89636c78..baa98b8ed11 100644 --- a/src/realm/object-store/sync/sync_user.hpp +++ b/src/realm/object-store/sync/sync_user.hpp @@ -19,13 +19,10 @@ #ifndef REALM_OS_SYNC_USER_HPP #define REALM_OS_SYNC_USER_HPP -#include -#include +#include #include - +#include #include -#include -#include #include #include @@ -42,27 +39,6 @@ class SyncManager; class SyncSession; class SyncUserMetadata; -// A struct that decodes a given JWT. -struct RealmJWT { - // The token being decoded from. - std::string token; - - // When the token expires. - int64_t expires_at = 0; - // When the token was issued. - int64_t issued_at = 0; - // Custom user data embedded in the encoded token. - util::Optional user_data; - - explicit RealmJWT(const std::string& token); - RealmJWT() = default; - - bool operator==(const RealmJWT& other) const - { - return token == other.token; - } -}; - struct SyncUserProfile { // The full name of the user. util::Optional name() const diff --git a/src/realm/parser/driver.cpp b/src/realm/parser/driver.cpp index cb87a6d064e..a5c58a56de7 100644 --- a/src/realm/parser/driver.cpp +++ b/src/realm/parser/driver.cpp @@ -1274,7 +1274,7 @@ std::unique_ptr ConstantNode::visit(ParserDriver* drv, DataType hint) size_t buffer_size = util::base64_decoded_size(encoded_size); std::string decode_buffer(buffer_size, char(0)); StringData window(text.c_str() + 4, encoded_size); - util::Optional decoded_size = util::base64_decode(window, decode_buffer.data(), buffer_size); + util::Optional decoded_size = util::base64_decode(window, decode_buffer); if (!decoded_size) { throw SyntaxError("Invalid base64 value"); } diff --git a/src/realm/sync/network/websocket.cpp b/src/realm/sync/network/websocket.cpp index 27fc941f4a9..5269eb638e1 100644 --- a/src/realm/sync/network/websocket.cpp +++ b/src/realm/sync/network/websocket.cpp @@ -45,7 +45,7 @@ std::string make_random_sec_websocket_key(std::mt19937_64& random) } char out_buffer[24]; - size_t encoded_size = util::base64_encode(random_bytes, 16, out_buffer, 24); + size_t encoded_size = util::base64_encode(random_bytes, out_buffer); REALM_ASSERT(encoded_size == 24); return std::string{out_buffer, 24}; @@ -62,11 +62,11 @@ std::string make_sec_websocket_accept(StringData sec_websocket_key) sha1_input.append(sec_websocket_key.data(), sec_websocket_key.size()); sha1_input.append(websocket_magic_string.data(), websocket_magic_string.size()); - unsigned char sha1_output[20]; - util::sha1(sha1_input.data(), sha1_input.length(), sha1_output); + char sha1_output[20]; + util::sha1(sha1_input.data(), sha1_input.length(), reinterpret_cast(sha1_output)); char base64_output[28]; - size_t base64_output_size = util::base64_encode(reinterpret_cast(sha1_output), 20, base64_output, 28); + size_t base64_output_size = util::base64_encode(sha1_output, base64_output); REALM_ASSERT(base64_output_size == 28); return std::string(base64_output, 28); diff --git a/src/realm/sync/noinst/protocol_codec.cpp b/src/realm/sync/noinst/protocol_codec.cpp index 109fd5732b6..17a96fc64e0 100644 --- a/src/realm/sync/noinst/protocol_codec.cpp +++ b/src/realm/sync/noinst/protocol_codec.cpp @@ -174,7 +174,7 @@ std::string ClientProtocol::compressed_hex_dump(BinaryData blob) std::string encode_buffer; auto encoded_size = util::base64_encoded_size(buf.size()); encode_buffer.resize(encoded_size); - util::base64_encode(buf.data(), buf.size(), encode_buffer.data(), encode_buffer.size()); + util::base64_encode(buf, encode_buffer); return encode_buffer; } diff --git a/src/realm/sync/noinst/server/access_token.cpp b/src/realm/sync/noinst/server/access_token.cpp index c42beb6a841..601254f54dd 100644 --- a/src/realm/sync/noinst/server/access_token.cpp +++ b/src/realm/sync/noinst/server/access_token.cpp @@ -245,8 +245,7 @@ bool AccessToken::parseJWT(StringData signed_token, AccessToken& token, ParseErr StringData signature_base64 = StringData{sep2 + 1, signed_token.size() - sep2_pos - 1}; std::vector signature_buffer; signature_buffer.resize(base64_decoded_size(signature_base64.size())); - Optional num_bytes_signature = - base64_decode(signature_base64, signature_buffer.data(), signature_base64.size()); + Optional num_bytes_signature = base64_decode(signature_base64, signature_buffer); if (!num_bytes_signature) { error = ParseError::invalid_base64; return false; @@ -315,7 +314,7 @@ bool AccessToken::parse(StringData signed_token, AccessToken& token, ParseError& token_buffer.resize(base64_decoded_size(token_base64.size())); // Throws StringData token_2; { - Optional num_bytes = base64_decode(token_base64, token_buffer.data(), token_buffer.size()); + Optional num_bytes = base64_decode(token_base64, token_buffer); if (!num_bytes) { error = ParseError::invalid_base64; return false; @@ -327,7 +326,7 @@ bool AccessToken::parse(StringData signed_token, AccessToken& token, ParseError& if (verifier) { std::vector buffer; buffer.resize(base64_decoded_size(signature_base64.size())); - Optional num_bytes = base64_decode(signature_base64, buffer.data(), buffer.size()); + Optional num_bytes = base64_decode(signature_base64, buffer); if (!num_bytes) { error = ParseError::invalid_base64; return false; diff --git a/src/realm/sync/tools/print_changeset.cpp b/src/realm/sync/tools/print_changeset.cpp index a318650f065..3412bc0ec77 100644 --- a/src/realm/sync/tools/print_changeset.cpp +++ b/src/realm/sync/tools/print_changeset.cpp @@ -51,11 +51,10 @@ std::string changeset_compressed_to_binary(const std::string& changeset_compress // Decode from BASE64 const size_t encoded_size = changeset_compressed.size() - (p - start); - size_t buffer_size = util::base64_decoded_size(encoded_size); std::string decode_buffer; - decode_buffer.resize(buffer_size); + decode_buffer.resize(util::base64_decoded_size(encoded_size)); StringData window(p, encoded_size); - util::Optional decoded_size = util::base64_decode(window, decode_buffer.data(), buffer_size); + util::Optional decoded_size = util::base64_decode(window, decode_buffer); if (!decoded_size || *decoded_size > encoded_size) { throw std::runtime_error("Invalid base64 value"); } diff --git a/src/realm/util/base64.cpp b/src/realm/util/base64.cpp index 3545195ef0b..e6632c32c61 100644 --- a/src/realm/util/base64.cpp +++ b/src/realm/util/base64.cpp @@ -19,7 +19,6 @@ #include #include -#include #if defined(_MSC_VER) # define REALM_RESTRICT __restrict @@ -71,21 +70,19 @@ inline unsigned int index_of_base64_byte(unsigned char c) } // unnamed namespace -namespace realm { -namespace util { +namespace realm::util { -size_t base64_encode(const char *in_buffer, size_t in_buffer_size, char* out_buffer, size_t out_buffer_size) noexcept +size_t base64_encode(Span in_buffer, Span out_buffer) noexcept { - REALM_ASSERT_EX(in_buffer_size < std::numeric_limits::max() - 2, in_buffer_size); - REALM_ASSERT_EX(in_buffer_size < 3 * (std::numeric_limits::max() / 4) - 2, in_buffer_size); - size_t encoded_size = 4 * ((in_buffer_size + 2) / 3); - REALM_ASSERT_EX(out_buffer_size >= encoded_size, out_buffer_size, encoded_size); - static_cast(out_buffer_size); + REALM_ASSERT_EX(in_buffer.size() < std::numeric_limits::max() - 2, in_buffer.size()); + REALM_ASSERT_EX(in_buffer.size() < 3 * (std::numeric_limits::max() / 4) - 2, in_buffer.size()); + size_t encoded_size = 4 * ((in_buffer.size() + 2) / 3); + REALM_ASSERT_EX(out_buffer.size() >= encoded_size, out_buffer.size(), encoded_size); - for (size_t i = 0, j = 0; i < in_buffer_size;) { - uint32_t octet_a = i < in_buffer_size ? static_cast(in_buffer[i++]) : 0; - uint32_t octet_b = i < in_buffer_size ? static_cast(in_buffer[i++]) : 0; - uint32_t octet_c = i < in_buffer_size ? static_cast(in_buffer[i++]) : 0; + for (size_t i = 0, j = 0; i < in_buffer.size();) { + uint32_t octet_a = i < in_buffer.size() ? static_cast(in_buffer[i++]) : 0; + uint32_t octet_b = i < in_buffer.size() ? static_cast(in_buffer[i++]) : 0; + uint32_t octet_c = i < in_buffer.size() ? static_cast(in_buffer[i++]) : 0; uint32_t triple = (octet_a << 0x10) + (octet_b << 0x08) + octet_c; @@ -96,7 +93,7 @@ size_t base64_encode(const char *in_buffer, size_t in_buffer_size, char* out_buf } // The last zero, one or two characters must be set to '='; - switch(in_buffer_size % 3) { + switch (in_buffer.size() % 3) { case 0: break; case 1: @@ -114,21 +111,19 @@ size_t base64_encode(const char *in_buffer, size_t in_buffer_size, char* out_buf } -Optional base64_decode(StringData input, char* out_buffer, size_t out_buffer_len) noexcept +std::optional base64_decode(Span input, Span out_buffer) noexcept { REALM_ASSERT_EX(input.size() < std::numeric_limits::max() / 3, input.size()); size_t required_buffer_len = (input.size() * 3 + 3) / 4; - REALM_ASSERT_EX(out_buffer_len >= required_buffer_len, out_buffer_len, required_buffer_len); - static_cast(out_buffer_len); + REALM_ASSERT_EX(out_buffer.size() >= required_buffer_len, out_buffer.size(), required_buffer_len); static_cast(required_buffer_len); // Guard against overlap (so that "restrict" works in the following) - REALM_ASSERT(input.data() + input.size() <= out_buffer - || input.data() >= out_buffer + out_buffer_len); + REALM_ASSERT(input.data() + input.size() <= out_buffer.data() || input.data() > &out_buffer.back()); const char* REALM_RESTRICT p = input.data(); - char* REALM_RESTRICT o = out_buffer; + char* REALM_RESTRICT o = out_buffer.data(); enum b64_byte_type { equals = 64, // used as padding at the end @@ -193,17 +188,16 @@ Optional base64_decode(StringData input, char* out_buffer, size_t out_bu return bytes_written; } -Optional> base64_decode_to_vector(StringData encoded) +std::optional> base64_decode_to_vector(Span encoded) { size_t max_size = base64_decoded_size(encoded.size()); std::vector decoded(max_size); // Throws - Optional actual_size = base64_decode(encoded, decoded.data(), decoded.size()); + auto actual_size = base64_decode(encoded, decoded); if (!actual_size) - return none; + return std::nullopt; decoded.resize(*actual_size); // Throws return decoded; } -} // namespace util -} // namespace realm +} // namespace realm::util. diff --git a/src/realm/util/base64.hpp b/src/realm/util/base64.hpp index b08eb5f0d0e..d47e9d02a64 100644 --- a/src/realm/util/base64.hpp +++ b/src/realm/util/base64.hpp @@ -19,21 +19,17 @@ #ifndef REALM_UTIL_BASE64_HPP #define REALM_UTIL_BASE64_HPP +#include +#include #include -#include -#include -namespace realm { -namespace util { +namespace realm::util { - -/// base64_encode() encodes the bnary data in \param in_buffer of size \param in_buffer_size . -/// The encoded data is placed in \param out_buffer . The size of \param \out_buffer is passed in -/// \param out_buffer_size . The output buffer out_buffer must be -/// large enough to hold the base64 encoded data. The size can be obtained from the function -/// base64_encoded_size. out_buffer_size is only used to assert that the output buffer is -/// large enough. -size_t base64_encode(const char *in_buffer, size_t in_buffer_size, char* out_buffer, size_t out_buffer_size) noexcept; +/// `base64_encode()` encodes the binary data in \param in_buffer . +/// The encoded data is placed in \param out_buffer , which must be large +/// enough to hold the base64 encoded data. The size can be obtained from the function +/// `base64_encoded_size()`. +size_t base64_encode(Span in_buffer, Span out_buffer) noexcept; /// base64_encoded_size() returns the exact size of the base64 encoded /// data as a function of the size of the input data. @@ -43,7 +39,7 @@ inline size_t base64_encoded_size(size_t in_buffer_size) noexcept } -/// Decode base64-encoded string in input, and places the result in out_buffer. +/// Decode base64-encoded string in `input`, and places the result in `out_buffer`. /// The length of the out_buffer must be at least 3 * input.size() / 4. /// /// The input must be padded base64 (i.e. the number of non-whitespace @@ -55,7 +51,7 @@ inline size_t base64_encoded_size(size_t in_buffer_size) noexcept /// /// \returns the number of successfully decoded bytes written to out_buffer, or /// none if the whole input was not valid base64. -Optional base64_decode(StringData input, char* out_buffer, size_t out_buffer_len) noexcept; +std::optional base64_decode(Span input, Span out_buffer) noexcept; /// Return an upper bound on the decoded size of a Base64-encoded data /// stream of length \a base64_size. The returned value is suitable for @@ -70,10 +66,9 @@ inline size_t base64_decoded_size(size_t base64_size) noexcept /// base64_decode_to_vector() is a convenience function that decodes \param /// encoded and returns the result in a std::vector with the correct size. /// This function returns none if the input is invalid. -Optional> base64_decode_to_vector(StringData encoded); +std::optional> base64_decode_to_vector(Span encoded); -} // namespace util -} // namespace realm +} // namespace realm::util #endif // REALM_UTIL_BASE64_HPP diff --git a/src/realm/util/bson/bson.cpp b/src/realm/util/bson/bson.cpp index 7b59673051b..58699db76e0 100644 --- a/src/realm/util/bson/bson.cpp +++ b/src/realm/util/bson/bson.cpp @@ -659,8 +659,7 @@ static constexpr std::pair bson_fancy_parsers[] = if (!base64 || !subType) throw BsonError("invalid extended json $binary"); if (subType == 0x04) { // UUID - auto stringData = StringData(reinterpret_cast(base64->data()), base64->size()); - util::Optional> uuidChrs = util::base64_decode_to_vector(stringData); + util::Optional> uuidChrs = util::base64_decode_to_vector(*base64); if (!uuidChrs) throw BsonError("Invalid base64 in $binary"); UUID::UUIDBytes bytes{}; @@ -795,10 +794,15 @@ Bson dom_obj_to_bson(const Json& json) } // anonymous namespace -Bson parse(const std::string_view& json) +Bson parse(util::Span json) { return dom_elem_to_bson(Json::parse(json)); } +bool accept(util::Span json) noexcept +{ + return Json::accept(json); +} + } // namespace bson } // namespace realm diff --git a/src/realm/util/bson/bson.hpp b/src/realm/util/bson/bson.hpp index 4f67a8144b9..a3cb8929c38 100644 --- a/src/realm/util/bson/bson.hpp +++ b/src/realm/util/bson/bson.hpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -389,7 +390,8 @@ using BsonArray = std::vector; std::ostream& operator<<(std::ostream& out, const Bson& b); -Bson parse(const std::string_view& json); +Bson parse(util::Span json); +bool accept(util::Span json) noexcept; } // namespace bson } // namespace realm diff --git a/src/realm/util/serializer.cpp b/src/realm/util/serializer.cpp index dbe23836d6a..1a803edceb7 100644 --- a/src/realm/util/serializer.cpp +++ b/src/realm/util/serializer.cpp @@ -227,7 +227,7 @@ std::string print_value<>(StringData data) if (contains_invalids(data)) { std::string encode_buffer; encode_buffer.resize(util::base64_encoded_size(len)); - util::base64_encode(start, len, encode_buffer.data(), encode_buffer.size()); + util::base64_encode(data, encode_buffer); out = "B64\"" + encode_buffer + "\""; } else { diff --git a/src/realm/uuid.cpp b/src/realm/uuid.cpp index a347e597929..f960743170a 100644 --- a/src/realm/uuid.cpp +++ b/src/realm/uuid.cpp @@ -116,11 +116,10 @@ std::string UUID::to_base64() const { char bytes[UUID::num_bytes]; std::copy(std::begin(m_bytes), std::end(m_bytes), std::begin(bytes)); - char* bytes_ptr = &bytes[0]; std::string encode_buffer; encode_buffer.resize(util::base64_encoded_size(UUID::num_bytes)); - util::base64_encode(bytes_ptr, UUID::num_bytes, encode_buffer.data(), encode_buffer.size()); + util::base64_encode(bytes, encode_buffer); return encode_buffer; } diff --git a/test/object-store/c_api/c_api.cpp b/test/object-store/c_api/c_api.cpp index 47940de31dd..c11320ed75c 100644 --- a/test/object-store/c_api/c_api.cpp +++ b/test/object-store/c_api/c_api.cpp @@ -5293,13 +5293,7 @@ TEST_CASE("C API - async_open", "[sync][pbs][c_api]") { } SECTION("cancels download and reports an error on auth error") { - // Create a token which can be parsed as a JWT but is not valid - std::string unencoded_body = nlohmann::json({{"exp", 123}, {"iat", 456}}).dump(); - std::string encoded_body; - encoded_body.resize(util::base64_encoded_size(unencoded_body.size())); - util::base64_encode(unencoded_body.data(), unencoded_body.size(), &encoded_body[0], encoded_body.size()); - auto invalid_token = "." + encoded_body + "."; - + auto expired_token = encode_fake_jwt("", 123, 456); realm_config_t* config = realm_config_new(); config->schema = Schema{object_schema}; @@ -5307,7 +5301,7 @@ TEST_CASE("C API - async_open", "[sync][pbs][c_api]") { realm_sync_config_t* sync_config = realm_sync_config_new(&user, "realm"); realm_sync_config_set_initial_subscription_handler(sync_config, task_init_subscription, false, nullptr, nullptr); - sync_config->user->log_in(invalid_token, invalid_token); + sync_config->user->log_in(expired_token, expired_token); realm_config_set_path(config, test_config.path.c_str()); realm_config_set_schema_version(config, 1); diff --git a/test/object-store/realm.cpp b/test/object-store/realm.cpp index 7fdbd8918db..b0582db0832 100644 --- a/test/object-store/realm.cpp +++ b/test/object-store/realm.cpp @@ -1139,17 +1139,12 @@ TEST_CASE("Get Realm using Async Open", "[sync][pbs][async open]") { }); } - // Create a token which can be parsed as a JWT but is not valid - std::string unencoded_body = nlohmann::json({{"exp", 123}, {"iat", 456}}).dump(); - std::string encoded_body; - encoded_body.resize(util::base64_encoded_size(unencoded_body.size())); - util::base64_encode(unencoded_body.data(), unencoded_body.size(), &encoded_body[0], encoded_body.size()); - auto invalid_token = "." + encoded_body + "."; + auto expired_token = encode_fake_jwt("", 123, 456); SECTION("can async open while waiting for a token refresh") { SyncTestFile config(init_sync_manager.app(), "realm"); auto valid_token = config.sync_config->user->access_token(); - config.sync_config->user->update_access_token(std::move(invalid_token)); + config.sync_config->user->update_access_token(std::move(expired_token)); std::atomic called{false}; auto task = Realm::get_synchronized_realm(config); @@ -1183,7 +1178,7 @@ TEST_CASE("Get Realm using Async Open", "[sync][pbs][async open]") { TestSyncManager tsm(tsm_config); SyncTestFile config(tsm.app(), "realm"); - config.sync_config->user->log_in(invalid_token, invalid_token); + config.sync_config->user->log_in(expired_token, expired_token); bool got_error = false; config.sync_config->error_handler = [&](std::shared_ptr, SyncError) { diff --git a/test/object-store/sync/app.cpp b/test/object-store/sync/app.cpp index 59577e1abc9..24c5240be8a 100644 --- a/test/object-store/sync/app.cpp +++ b/test/object-store/sync/app.cpp @@ -162,16 +162,16 @@ static std::string create_jwt(const std::string& appId) payload["my_metadata"]["name"] = "Bar Foo"; payload["my_metadata"]["occupation"] = "stock analyst"; - std::string headerStr = header.dump(); - std::string payloadStr = payload.dump(); + std::string header_str = header.dump(); + std::string payload_str = payload.dump(); std::string encoded_header; - encoded_header.resize(util::base64_encoded_size(headerStr.length())); - util::base64_encode(headerStr.data(), headerStr.length(), encoded_header.data(), encoded_header.size()); + encoded_header.resize(util::base64_encoded_size(header_str.length())); + util::base64_encode(header_str, encoded_header); std::string encoded_payload; - encoded_payload.resize(util::base64_encoded_size(payloadStr.length())); - util::base64_encode(payloadStr.data(), payloadStr.length(), encoded_payload.data(), encoded_payload.size()); + encoded_payload.resize(util::base64_encoded_size(payload_str.length())); + util::base64_encode(payload_str, encoded_payload); // Remove padding characters. while (encoded_header.back() == '=') @@ -181,13 +181,14 @@ static std::string create_jwt(const std::string& appId) std::string jwtPayload = encoded_header + "." + encoded_payload; - std::array hmac; + std::array hmac; unsigned char key[] = "My_very_confidential_secretttttt"; - util::hmac_sha256(util::unsafe_span_cast(jwtPayload), hmac, util::Span(key, 32)); + util::hmac_sha256(util::unsafe_span_cast(jwtPayload), util::unsafe_span_cast(hmac), + util::Span(key, 32)); std::string signature; signature.resize(util::base64_encoded_size(hmac.size())); - util::base64_encode(reinterpret_cast(hmac.data()), hmac.size(), signature.data(), signature.size()); + util::base64_encode(hmac, signature); while (signature.back() == '=') signature.pop_back(); std::replace(signature.begin(), signature.end(), '+', '-'); @@ -4797,7 +4798,7 @@ TEST_CASE("app: login_with_credentials unit_tests", "[sync][app][user]") { config.transport = instance_of; TestSyncManager tsm(config); auto error = failed_log_in(tsm.app()); - CHECK(error.reason() == std::string("jwt missing parts")); + CHECK(error.reason() == std::string("malformed JWT")); CHECK(error.code_string() == "BadToken"); CHECK(error.is_json_error()); CHECK(error.code() == ErrorCodes::BadToken); @@ -5416,7 +5417,7 @@ TEST_CASE("app: refresh access token unit tests", "[sync][app][user][token]") { bool processed = false; app->refresh_custom_data(app->sync_manager()->get_current_user(), [&](const Optional& error) { - CHECK(error->reason() == "jwt missing parts"); + CHECK(error->reason() == "malformed JWT"); CHECK(error->code() == ErrorCodes::BadToken); CHECK(session_route_hit); processed = true; diff --git a/test/object-store/util/test_utils.cpp b/test/object-store/util/test_utils.cpp index 9c1fbdd3c0c..5e48f135234 100644 --- a/test/object-store/util/test_utils.cpp +++ b/test/object-store/util/test_utils.cpp @@ -147,8 +147,8 @@ std::string encode_fake_jwt(const std::string& in, util::Optional exp, std::string encoded_prefix, encoded_body; encoded_prefix.resize(util::base64_encoded_size(unencoded_prefix.size())); encoded_body.resize(util::base64_encoded_size(unencoded_body.size())); - util::base64_encode(unencoded_prefix.data(), unencoded_prefix.size(), &encoded_prefix[0], encoded_prefix.size()); - util::base64_encode(unencoded_body.data(), unencoded_body.size(), &encoded_body[0], encoded_body.size()); + util::base64_encode(unencoded_prefix, encoded_prefix); + util::base64_encode(unencoded_body, encoded_body); std::string suffix = "Et9HFtf9R3GEMA0IICOfFMVXY7kkTX1wr4qCyhIf58U"; return encoded_prefix + "." + encoded_body + "." + suffix; } diff --git a/test/test_table.cpp b/test/test_table.cpp index 37874af43c1..0b6b78d09e4 100644 --- a/test/test_table.cpp +++ b/test/test_table.cpp @@ -3887,7 +3887,7 @@ TEST(Table_CollisionMapping) char buffer[12]; for (size_t i = 0; i < num_objects_with_guaranteed_collision; ++i) { const char* in = reinterpret_cast(&i); - size_t len = base64_encode(in, sizeof(i), buffer, sizeof(buffer)); + size_t len = base64_encode({in, sizeof(i)}, buffer); t0->create_object_with_primary_key(StringData{buffer, len}); } @@ -3921,7 +3921,7 @@ TEST(Table_CollisionMapping) for (size_t i = 0; i < num_objects_with_guaranteed_collision; ++i) { size_t foo = num_objects_with_guaranteed_collision + i; const char* in = reinterpret_cast(&foo); - size_t len = base64_encode(in, sizeof(foo), buffer, sizeof(buffer)); + size_t len = base64_encode({in, sizeof(foo)}, buffer); t0->create_object_with_primary_key(StringData{buffer, len}); } diff --git a/test/test_util_base64.cpp b/test/test_util_base64.cpp index 34ee6f9d81d..173578339d0 100644 --- a/test/test_util_base64.cpp +++ b/test/test_util_base64.cpp @@ -55,7 +55,7 @@ TEST(Base64_Decode) static const size_t num_tests = sizeof(inputs) / sizeof(inputs[0]); for (size_t i = 0; i < num_tests; ++i) { - r = base64_decode(inputs[i], buffer.data(), buffer.size()); + r = base64_decode({inputs[i], strlen(inputs[i])}, buffer); CHECK(r); CHECK_EQUAL(StringData(buffer.data(), *r), expected[i]); } @@ -71,7 +71,7 @@ TEST(Base64_Decode) static const size_t num_bad_tests = sizeof(bad_inputs) / sizeof(bad_inputs[0]); for (size_t i = 0; i < num_bad_tests; ++i) { - r = base64_decode(bad_inputs[i], buffer.data(), buffer.size()); + r = base64_decode({bad_inputs[i], strlen(bad_inputs[i])}, buffer); CHECK(!r); } } @@ -81,7 +81,7 @@ TEST(Base64_Decode_AdjacentBuffers) { char buffer[10] = "Zg==\0"; // "f" + blank space + terminating zero const char expected[] = "f"; - Optional r = base64_decode(buffer, buffer + 4, 3); + Optional r = base64_decode({buffer, 4}, {buffer + 4, 3}); CHECK(r); CHECK_EQUAL(*r, 1); CHECK_EQUAL(StringData{buffer + 4}, StringData{expected}); @@ -90,7 +90,6 @@ TEST(Base64_Decode_AdjacentBuffers) namespace { struct TestBuffers { - const char* decoded_buffer; size_t decoded_buffer_size; const char* encoded_buffer; @@ -105,22 +104,22 @@ TEST(Base64_Encode) buffer.resize(100); TestBuffers tbs[] = { - TestBuffers {"", 0, "", 0}, - TestBuffers {"\x00\x00\x00", 3, "AAAA", 4}, - TestBuffers {"\x00\x00\x01", 3, "AAAB", 4}, - TestBuffers {"\x80", 1, "gA==", 4}, - TestBuffers {"\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10", 16, "AQIDBAUGBwgJCgsMDQ4PEA==", 24} - + TestBuffers{"", 0, "", 0}, + TestBuffers{"\x00\x00\x00", 3, "AAAA", 4}, + TestBuffers{"\x00\x00\x01", 3, "AAAB", 4}, + TestBuffers{"\x80", 1, "gA==", 4}, + TestBuffers{"\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f\x10", 16, + "AQIDBAUGBwgJCgsMDQ4PEA==", 24}, }; const size_t num_tests = sizeof(tbs) / sizeof(tbs[0]); for (size_t i = 0; i < num_tests; ++i) { - size_t return_size = base64_encode(tbs[i].decoded_buffer, tbs[i].decoded_buffer_size, buffer.data(), buffer.size()); + size_t return_size = base64_encode({tbs[i].decoded_buffer, tbs[i].decoded_buffer_size}, buffer); CHECK_EQUAL(return_size, tbs[i].encoded_buffer_size); CHECK_EQUAL(StringData(buffer.data(), return_size), tbs[i].encoded_buffer); - Optional return_size_opt = base64_decode(StringData(tbs[i].encoded_buffer, tbs[i].encoded_buffer_size), buffer.data(), buffer.size()); + Optional return_size_opt = base64_decode({tbs[i].encoded_buffer, tbs[i].encoded_buffer_size}, buffer); CHECK(return_size_opt); CHECK_EQUAL(*return_size_opt, tbs[i].decoded_buffer_size); for (size_t j = 0; j < *return_size_opt; ++j) { @@ -131,13 +130,14 @@ TEST(Base64_Encode) TEST(Base64_DecodeToVector) { + using namespace std::string_view_literals; { - Optional> vec = base64_decode_to_vector("======"); + Optional> vec = base64_decode_to_vector("======"sv); CHECK(!vec); } { - Optional> vec = base64_decode_to_vector("SGVsb G8sIF\ndvc mxkIQ=="); + Optional> vec = base64_decode_to_vector("SGVsb G8sIF\ndvc mxkIQ=="sv); std::string str(vec->begin(), vec->end()); CHECK_EQUAL("Hello, World!", str); } From 152ff3fa30a134a6b46381f4a2f0c4f1b8243af5 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 7 Feb 2024 19:45:34 -0800 Subject: [PATCH 2/6] Filter out non-logged in users earlier when loading from the metadata realm --- .../object-store/sync/impl/sync_metadata.cpp | 33 +++++++++++++++++++ .../object-store/sync/impl/sync_metadata.hpp | 2 ++ src/realm/object-store/sync/sync_manager.cpp | 10 ++---- src/realm/object-store/sync/sync_user.cpp | 9 +---- 4 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/realm/object-store/sync/impl/sync_metadata.cpp b/src/realm/object-store/sync/impl/sync_metadata.cpp index 8fc53c30da6..fcb25731b0d 100644 --- a/src/realm/object-store/sync/impl/sync_metadata.cpp +++ b/src/realm/object-store/sync/impl/sync_metadata.cpp @@ -260,6 +260,39 @@ SyncMetadataManager::SyncMetadataManager(std::string path, bool should_encrypt, object_schema->persisted_properties[4].column_key}; } +// Some of our string columns are nullable. They never should actually be +// null as we store "" rather than null when the value isn't present, but +// be safe and handle it anyway. +static std::string_view get_string(const Obj& obj, ColKey col) +{ + auto str = obj.get(col); + return str.is_null() ? "" : std::string_view(str); +} + +static bool is_valid_user(const SyncUserMetadata::Schema& schema, const Obj& obj) +{ + // This is overly cautious and merely checking the state should suffice, + // but because this is a persisted file that can be modified it's possible + // to get invalid combinations of data. + return obj && obj.get(schema.state_col) == int64_t(SyncUser::State::LoggedIn) && + RealmJWT::validate(get_string(obj, schema.access_token_col)) && + RealmJWT::validate(get_string(obj, schema.refresh_token_col)); +} + +std::vector SyncMetadataManager::all_logged_in_users() const +{ + auto realm = get_realm(); + TableRef table = ObjectStore::table_for_object_type(realm->read_group(), c_sync_userMetadata); + std::vector users; + users.reserve(table->size()); + for (auto obj : *table) { + if (is_valid_user(m_user_schema, obj)) { + users.emplace_back(m_user_schema, realm, obj); + } + } + return users; +} + SyncUserMetadataResults SyncMetadataManager::all_unmarked_users() const { return get_users(false); diff --git a/src/realm/object-store/sync/impl/sync_metadata.hpp b/src/realm/object-store/sync/impl/sync_metadata.hpp index 2ab0d84b3cf..c41553f92f4 100644 --- a/src/realm/object-store/sync/impl/sync_metadata.hpp +++ b/src/realm/object-store/sync/impl/sync_metadata.hpp @@ -207,6 +207,8 @@ class SyncMetadataManager { friend class SyncFileActionMetadata; public: + std::vector all_logged_in_users() const; + // Return a Results object containing all users not marked for removal. SyncUserMetadataResults all_unmarked_users() const; diff --git a/src/realm/object-store/sync/sync_manager.cpp b/src/realm/object-store/sync/sync_manager.cpp index dd1cf9bb712..1ff0432fe8f 100644 --- a/src/realm/object-store/sync/sync_manager.cpp +++ b/src/realm/object-store/sync/sync_manager.cpp @@ -101,14 +101,8 @@ void SyncManager::configure(std::shared_ptr app, std::optionalall_unmarked_users(); - for (size_t i = 0; i < users.size(); i++) { - auto user_data = users.get(i); - auto refresh_token = user_data.refresh_token(); - auto access_token = user_data.access_token(); - if (!refresh_token.empty() && !access_token.empty()) { - users_to_add.push_back(std::make_shared(SyncUser::Private(), user_data, this)); - } + for (auto user : m_metadata_manager->all_logged_in_users()) { + users_to_add.push_back(std::make_shared(SyncUser::Private(), user, this)); } // Delete any users marked for death. diff --git a/src/realm/object-store/sync/sync_user.cpp b/src/realm/object-store/sync/sync_user.cpp index 8c141c8fc28..edac5583a6d 100644 --- a/src/realm/object-store/sync/sync_user.cpp +++ b/src/realm/object-store/sync/sync_user.cpp @@ -65,14 +65,7 @@ SyncUser::SyncUser(Private, const SyncUserMetadata& data, SyncManager* sync_mana , m_device_id(data.device_id()) , m_sync_manager(sync_manager) { - // Check for inconsistent state in the metadata Realm. This shouldn't happen, - // but previous versions could sometimes mark a user as logged in with an - // empty refresh token. - if (m_state == State::LoggedIn && (m_refresh_token.token.empty() || m_access_token.token.empty())) { - m_state = State::LoggedOut; - m_refresh_token = {}; - m_access_token = {}; - } + REALM_ASSERT(m_state == State::LoggedIn && !m_access_token.token.empty() && !m_refresh_token.token.empty()); } std::shared_ptr SyncUser::sync_manager() const From 5fc73a35102d74362a24db784bf1b62ee0bae3b7 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 7 Feb 2024 20:43:17 -0800 Subject: [PATCH 3/6] Delete the now-unused app metadata storage --- src/realm/object-store/sync/app.hpp | 1 - .../object-store/sync/impl/sync_metadata.cpp | 71 ------------------- .../object-store/sync/impl/sync_metadata.hpp | 32 --------- src/realm/object-store/sync/sync_manager.cpp | 9 --- src/realm/object-store/sync/sync_manager.hpp | 3 - 5 files changed, 116 deletions(-) diff --git a/src/realm/object-store/sync/app.hpp b/src/realm/object-store/sync/app.hpp index 7d3812884a8..37396fed6f0 100644 --- a/src/realm/object-store/sync/app.hpp +++ b/src/realm/object-store/sync/app.hpp @@ -39,7 +39,6 @@ class SyncUser; class SyncSession; class SyncManager; struct SyncClientConfig; -class SyncAppMetadata; namespace app { diff --git a/src/realm/object-store/sync/impl/sync_metadata.cpp b/src/realm/object-store/sync/impl/sync_metadata.cpp index fcb25731b0d..9ec347fb5b3 100644 --- a/src/realm/object-store/sync/impl/sync_metadata.cpp +++ b/src/realm/object-store/sync/impl/sync_metadata.cpp @@ -39,7 +39,6 @@ using namespace realm; namespace { static const char* const c_sync_userMetadata = "UserMetadata"; static const char* const c_sync_identityMetadata = "UserIdentity"; -static const char* const c_sync_app_metadata = "AppMetadata"; static const char* const c_sync_current_user_identity = "current_user_identity"; @@ -64,12 +63,6 @@ static const char* const c_sync_new_name = "new_name"; static const char* const c_sync_action = "action"; static const char* const c_sync_partition = "url"; -static const char* const c_sync_app_metadata_id = "id"; -static const char* const c_sync_app_metadata_deployment_model = "deployment_model"; -static const char* const c_sync_app_metadata_location = "location"; -static const char* const c_sync_app_metadata_hostname = "hostname"; -static const char* const c_sync_app_metadata_ws_hostname = "ws_hostname"; - realm::Schema make_schema() { using namespace realm; @@ -102,14 +95,6 @@ realm::Schema make_schema() { {c_sync_current_user_identity, PropertyType::String}, }}, - {c_sync_app_metadata, - { - {c_sync_app_metadata_id, PropertyType::Int, Property::IsPrimary{true}}, - {c_sync_app_metadata_deployment_model, PropertyType::String}, - {c_sync_app_metadata_location, PropertyType::String}, - {c_sync_app_metadata_hostname, PropertyType::String}, - {c_sync_app_metadata_ws_hostname, PropertyType::String}, - }}, }; } @@ -252,12 +237,6 @@ SyncMetadataManager::SyncMetadataManager(std::string path, bool should_encrypt, object_schema->persisted_properties[2].column_key, object_schema->persisted_properties[3].column_key, object_schema->persisted_properties[4].column_key, }; - - object_schema = realm->schema().find(c_sync_app_metadata); - m_app_metadata_schema = { - object_schema->persisted_properties[0].column_key, object_schema->persisted_properties[1].column_key, - object_schema->persisted_properties[2].column_key, object_schema->persisted_properties[3].column_key, - object_schema->persisted_properties[4].column_key}; } // Some of our string columns are nullable. They never should actually be @@ -524,56 +503,6 @@ std::shared_ptr SyncMetadataManager::open_realm(bool should_encrypt, bool #endif // REALM_PLATFORM_APPLE } -/// Magic key to fetch app metadata, which there should always only be one of. -static const auto app_metadata_pk = 1; - -bool SyncMetadataManager::set_app_metadata(const std::string& deployment_model, const std::string& location, - const std::string& hostname, const std::string& ws_hostname) -{ - if (m_app_metadata && m_app_metadata->hostname == hostname && m_app_metadata->ws_hostname == ws_hostname && - m_app_metadata->deployment_model == deployment_model && m_app_metadata->location == location) { - // App metadata not updated - return false; - } - - auto realm = get_realm(); - auto& schema = m_app_metadata_schema; - - // let go of stale cached copy of metadata - it will be refreshed on the next call to get_app_metadata() - m_app_metadata = util::none; - - realm->begin_transaction(); - - auto table = ObjectStore::table_for_object_type(realm->read_group(), c_sync_app_metadata); - auto obj = table->create_object_with_primary_key(app_metadata_pk); - obj.set(schema.deployment_model_col, deployment_model); - obj.set(schema.location_col, location); - obj.set(schema.hostname_col, hostname); - obj.set(schema.ws_hostname_col, ws_hostname); - - realm->commit_transaction(); - // App metadata was updated - return true; -} - -util::Optional SyncMetadataManager::get_app_metadata() -{ - if (!m_app_metadata) { - auto realm = get_realm(); - auto table = ObjectStore::table_for_object_type(realm->read_group(), c_sync_app_metadata); - if (!table->size()) - return util::none; - - auto obj = table->get_object_with_primary_key(app_metadata_pk); - auto& schema = m_app_metadata_schema; - m_app_metadata = - SyncAppMetadata{obj.get(schema.deployment_model_col), obj.get(schema.location_col), - obj.get(schema.hostname_col), obj.get(schema.ws_hostname_col)}; - } - - return m_app_metadata; -} - // MARK: - Sync user metadata SyncUserMetadata::SyncUserMetadata(Schema schema, SharedRealm realm, const Obj& obj) diff --git a/src/realm/object-store/sync/impl/sync_metadata.hpp b/src/realm/object-store/sync/impl/sync_metadata.hpp index c41553f92f4..729b2323153 100644 --- a/src/realm/object-store/sync/impl/sync_metadata.hpp +++ b/src/realm/object-store/sync/impl/sync_metadata.hpp @@ -31,23 +31,6 @@ namespace realm { class SyncMetadataManager; -// A facade for a metadata Realm object representing app metadata -class SyncAppMetadata { -public: - struct Schema { - ColKey id_col; - ColKey deployment_model_col; - ColKey location_col; - ColKey hostname_col; - ColKey ws_hostname_col; - }; - - std::string deployment_model; - std::string location; - std::string hostname; - std::string ws_hostname; -}; - // A facade for a metadata Realm object representing a sync user. class SyncUserMetadata { public: @@ -235,17 +218,6 @@ class SyncMetadataManager { util::Optional get_current_user_identity() const; void set_current_user_identity(const std::string& identity); - util::Optional get_app_metadata(); - /// Set or update the cached app server metadata. The metadata will not be updated if it has already been - /// set and the provided values are not different than the cached information. Returns true if the metadata - /// was updated. - /// @param deployment_model The deployment model reported by the app server - /// @param location The location name where the app server is located - /// @param hostname The hostname to use for the app server admin api - /// @param ws_hostname The hostname to use for the app server websocket connections - bool set_app_metadata(const std::string& deployment_model, const std::string& location, - const std::string& hostname, const std::string& ws_hostname); - /// Construct the metadata manager. /// /// If the platform supports it, setting `should_encrypt` to `true` and not specifying an encryption key will make @@ -259,14 +231,10 @@ class SyncMetadataManager { Realm::Config m_metadata_config; SyncUserMetadata::Schema m_user_schema; SyncFileActionMetadata::Schema m_file_action_schema; - SyncAppMetadata::Schema m_app_metadata_schema; std::shared_ptr get_realm() const; std::shared_ptr try_get_realm() const; std::shared_ptr open_realm(bool should_encrypt, bool caller_supplied_key); - - - util::Optional m_app_metadata; }; } // namespace realm diff --git a/src/realm/object-store/sync/sync_manager.cpp b/src/realm/object-store/sync/sync_manager.cpp index 1ff0432fe8f..c769f3a56d1 100644 --- a/src/realm/object-store/sync/sync_manager.cpp +++ b/src/realm/object-store/sync/sync_manager.cpp @@ -743,15 +743,6 @@ std::unique_ptr SyncManager::create_sync_client() const return std::make_unique(m_logger_ptr, m_config, weak_from_this()); } -util::Optional SyncManager::app_metadata() const -{ - util::CheckedLockGuard lock(m_file_system_mutex); - if (!m_metadata_manager) { - return util::none; - } - return m_metadata_manager->get_app_metadata(); -} - void SyncManager::close_all_sessions() { // log_out() will call unregister_session(), which requires m_session_mutex, diff --git a/src/realm/object-store/sync/sync_manager.hpp b/src/realm/object-store/sync/sync_manager.hpp index 782c8790252..0ca0e6359ce 100644 --- a/src/realm/object-store/sync/sync_manager.hpp +++ b/src/realm/object-store/sync/sync_manager.hpp @@ -44,7 +44,6 @@ class SyncUser; class SyncFileManager; class SyncMetadataManager; class SyncFileActionMetadata; -class SyncAppMetadata; namespace _impl { struct SyncClient; @@ -221,8 +220,6 @@ class SyncManager : public std::enable_shared_from_this { // calling this method. void reset_for_testing() REQUIRES(!m_mutex, !m_file_system_mutex, !m_user_mutex, !m_session_mutex); - // Get the app metadata for the active app. - util::Optional app_metadata() const REQUIRES(!m_file_system_mutex); // Immediately closes any open sync sessions for this sync manager void close_all_sessions() REQUIRES(!m_mutex, !m_session_mutex); From 308a99f91c5a87d1e28d56900c05e16982d845f8 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 7 Feb 2024 22:06:49 -0800 Subject: [PATCH 4/6] Push file action handling into SyncMetadataManager --- .../object-store/sync/impl/sync_metadata.cpp | 99 +++++++++++++++---- .../object-store/sync/impl/sync_metadata.hpp | 8 ++ src/realm/object-store/sync/sync_manager.cpp | 78 +-------------- src/realm/object-store/sync/sync_manager.hpp | 1 - 4 files changed, 91 insertions(+), 95 deletions(-) diff --git a/src/realm/object-store/sync/impl/sync_metadata.cpp b/src/realm/object-store/sync/impl/sync_metadata.cpp index 9ec347fb5b3..080689af844 100644 --- a/src/realm/object-store/sync/impl/sync_metadata.cpp +++ b/src/realm/object-store/sync/impl/sync_metadata.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #if REALM_PLATFORM_APPLE @@ -239,6 +240,72 @@ SyncMetadataManager::SyncMetadataManager(std::string path, bool should_encrypt, }; } +void SyncMetadataManager::perform_launch_actions(SyncFileManager& file_manager) const +{ + auto realm = get_realm(); + + // Perform our "on next startup" actions such as deleting Realm files + // which we couldn't delete immediately due to them being in use + auto actions_table = ObjectStore::table_for_object_type(realm->read_group(), c_sync_fileActionMetadata); + for (auto file_action : *actions_table) { + SyncFileActionMetadata md(m_file_action_schema, realm, file_action); + run_file_action(file_manager, md); + } + + // Delete any users marked for death. + auto users_table = ObjectStore::table_for_object_type(realm->read_group(), c_sync_userMetadata); + for (auto user : *users_table) { + if (user.get(m_user_schema.state_col) != int64_t(SyncUser::State::Removed)) + continue; + try { + SyncUserMetadata data(m_user_schema, realm, user); + file_manager.remove_user_realms(data.identity(), data.realm_file_paths()); + realm->begin_transaction(); + user.remove(); + realm->commit_transaction(); + } + catch (FileAccessError const&) { + continue; + } + } +} + +bool SyncMetadataManager::run_file_action(SyncFileManager& file_manager, SyncFileActionMetadata& md) const +{ + switch (md.action()) { + case SyncFileActionMetadata::Action::DeleteRealm: + // Delete all the files for the given Realm. + if (file_manager.remove_realm(md.original_name())) { + md.remove(); + return true; + } + break; + case SyncFileActionMetadata::Action::BackUpThenDeleteRealm: + // Copy the primary Realm file to the recovery dir, and then delete the Realm. + auto new_name = md.new_name(); + auto original_name = md.original_name(); + if (!util::File::exists(original_name)) { + // The Realm file doesn't exist anymore. + md.remove(); + return false; + } + if (new_name && !util::File::exists(*new_name) && + file_manager.copy_realm_file(original_name, *new_name)) { + // We successfully copied the Realm file to the recovery directory. + bool did_remove = file_manager.remove_realm(original_name); + // if the copy succeeded but not the delete, then running BackupThenDelete + // a second time would fail, so change this action to just delete the original file. + if (did_remove) { + md.remove(); + return true; + } + md.set_action(SyncFileActionMetadata::Action::DeleteRealm); + } + break; + } + return false; +} + // Some of our string columns are nullable. They never should actually be // null as we store "" rather than null when the value isn't present, but // be safe and handle it anyway. @@ -398,18 +465,9 @@ void SyncMetadataManager::make_file_action_metadata(StringData original_name, St StringData local_uuid, SyncFileActionMetadata::Action action, StringData new_name) const { - // This function can't use get_shared_realm() because it's called on a - // background thread and that's currently not supported by the libuv - // implementation of EventLoopSignal - auto coordinator = _impl::RealmCoordinator::get_coordinator(m_metadata_config); - auto group_ptr = coordinator->begin_read(); - auto& group = *group_ptr; - REALM_ASSERT(typeid(group) == typeid(Transaction)); - auto& transaction = static_cast(group); - transaction.promote_to_write(); - - // Retrieve or create the row for this object. - TableRef table = ObjectStore::table_for_object_type(group, c_sync_fileActionMetadata); + auto realm = get_realm(); + realm->begin_transaction(); + TableRef table = ObjectStore::table_for_object_type(realm->read_group(), c_sync_fileActionMetadata); auto& schema = m_file_action_schema; Obj obj = table->create_object_with_primary_key(original_name); @@ -418,7 +476,7 @@ void SyncMetadataManager::make_file_action_metadata(StringData original_name, St obj.set(schema.idx_action, static_cast(action)); obj.set(schema.idx_partition, partition_key_value); obj.set(schema.idx_user_identity, local_uuid); - transaction.commit(); + realm->commit_transaction(); } util::Optional SyncMetadataManager::get_file_action_metadata(StringData original_name) const @@ -433,6 +491,14 @@ util::Optional SyncMetadataManager::get_file_action_meta return SyncFileActionMetadata(std::move(schema), std::move(realm), table->get_object(row_idx)); } +bool SyncMetadataManager::perform_file_actions(SyncFileManager& file_manager, StringData path) const +{ + if (auto md = get_file_action_metadata(path)) { + return run_file_action(file_manager, *md); + } + return false; +} + std::shared_ptr SyncMetadataManager::get_realm() const { auto realm = Realm::get_shared_realm(m_metadata_config); @@ -560,11 +626,6 @@ std::string SyncUserMetadata::device_id() const return result.is_null() ? "" : std::string(result); } -inline SyncUserIdentity user_identity_from_obj(const Obj& obj) -{ - return SyncUserIdentity(obj.get(c_sync_user_id), obj.get(c_sync_provider_type)); -} - std::vector SyncUserMetadata::identities() const { REALM_ASSERT(m_realm); @@ -574,7 +635,7 @@ std::vector SyncUserMetadata::identities() const std::vector identities; for (size_t i = 0; i < linklist.size(); i++) { auto obj = linklist.get_object(i); - identities.push_back(user_identity_from_obj(obj)); + identities.emplace_back(obj.get(c_sync_user_id), obj.get(c_sync_provider_type)); } return identities; diff --git a/src/realm/object-store/sync/impl/sync_metadata.hpp b/src/realm/object-store/sync/impl/sync_metadata.hpp index 729b2323153..7f859b3341a 100644 --- a/src/realm/object-store/sync/impl/sync_metadata.hpp +++ b/src/realm/object-store/sync/impl/sync_metadata.hpp @@ -29,6 +29,7 @@ #include namespace realm { +class SyncFileManager; class SyncMetadataManager; // A facade for a metadata Realm object representing a sync user. @@ -192,6 +193,9 @@ class SyncMetadataManager { public: std::vector all_logged_in_users() const; + // Perform all pending file actions and delete any remaining data for removed users. + void perform_launch_actions(SyncFileManager& file_manager) const; + // Return a Results object containing all users not marked for removal. SyncUserMetadataResults all_unmarked_users() const; @@ -210,6 +214,8 @@ class SyncMetadataManager { // Retrieve file action metadata. util::Optional get_file_action_metadata(StringData path) const; + // Perform any stored file actions for the given path. + bool perform_file_actions(SyncFileManager& file_manager, StringData path) const; // Create file action metadata. void make_file_action_metadata(StringData original_name, StringData partition_key_value, StringData local_uuid, @@ -235,6 +241,8 @@ class SyncMetadataManager { std::shared_ptr get_realm() const; std::shared_ptr try_get_realm() const; std::shared_ptr open_realm(bool should_encrypt, bool caller_supplied_key); + + bool run_file_action(SyncFileManager& file_manager, SyncFileActionMetadata& md) const; }; } // namespace realm diff --git a/src/realm/object-store/sync/sync_manager.cpp b/src/realm/object-store/sync/sync_manager.cpp index c769f3a56d1..e53eb4888a9 100644 --- a/src/realm/object-store/sync/sync_manager.cpp +++ b/src/realm/object-store/sync/sync_manager.cpp @@ -84,46 +84,12 @@ void SyncManager::configure(std::shared_ptr app, std::optional(m_file_manager->metadata_path(), encrypt, m_config.custom_encryption_key); - REALM_ASSERT(m_metadata_manager); - - // Perform our "on next startup" actions such as deleting Realm files - // which we couldn't delete immediately due to them being in use - std::vector completed_actions; - SyncFileActionMetadataResults file_actions = m_metadata_manager->all_pending_actions(); - for (size_t i = 0; i < file_actions.size(); i++) { - auto file_action = file_actions.get(i); - if (run_file_action(file_action)) { - completed_actions.emplace_back(std::move(file_action)); - } - } - for (auto& action : completed_actions) { - action.remove(); - } + m_metadata_manager->perform_launch_actions(*m_file_manager); // Load persisted users into the users map. for (auto user : m_metadata_manager->all_logged_in_users()) { users_to_add.push_back(std::make_shared(SyncUser::Private(), user, this)); } - - // Delete any users marked for death. - std::vector dead_users; - SyncUserMetadataResults users_to_remove = m_metadata_manager->all_users_marked_for_removal(); - dead_users.reserve(users_to_remove.size()); - for (size_t i = 0; i < users_to_remove.size(); i++) { - auto user = users_to_remove.get(i); - // FIXME: delete user data in a different way? (This deletes a logged-out user's data as soon as the - // app launches again, which might not be how some apps want to treat their data.) - try { - m_file_manager->remove_user_realms(user.identity(), user.realm_file_paths()); - dead_users.emplace_back(std::move(user)); - } - catch (FileAccessError const&) { - continue; - } - } - for (auto& user : dead_users) { - user.remove(); - } } } { @@ -135,46 +101,8 @@ void SyncManager::configure(std::shared_ptr app, std::optionalget_file_action_metadata(realm_path)) { - if (run_file_action(*metadata)) { - metadata->remove(); - return true; - } - } - return false; -} - -// Perform a file action. Returns whether or not the file action can be removed. -bool SyncManager::run_file_action(SyncFileActionMetadata& md) -{ - switch (md.action()) { - case SyncFileActionMetadata::Action::DeleteRealm: - // Delete all the files for the given Realm. - return m_file_manager->remove_realm(md.original_name()); - case SyncFileActionMetadata::Action::BackUpThenDeleteRealm: - // Copy the primary Realm file to the recovery dir, and then delete the Realm. - auto new_name = md.new_name(); - auto original_name = md.original_name(); - if (!util::File::exists(original_name)) { - // The Realm file doesn't exist anymore. - return true; - } - if (new_name && !util::File::exists(*new_name) && - m_file_manager->copy_realm_file(original_name, *new_name)) { - // We successfully copied the Realm file to the recovery directory. - bool did_remove = m_file_manager->remove_realm(original_name); - // if the copy succeeded but not the delete, then running BackupThenDelete - // a second time would fail, so change this action to just delete the original file. - if (did_remove) { - return true; - } - md.set_action(SyncFileActionMetadata::Action::DeleteRealm); - return false; - } - return false; + if (m_metadata_manager) { + return m_metadata_manager->perform_file_actions(*m_file_manager, realm_path); } return false; } diff --git a/src/realm/object-store/sync/sync_manager.hpp b/src/realm/object-store/sync/sync_manager.hpp index 0ca0e6359ce..95e83e93732 100644 --- a/src/realm/object-store/sync/sync_manager.hpp +++ b/src/realm/object-store/sync/sync_manager.hpp @@ -297,7 +297,6 @@ class SyncManager : public std::enable_shared_from_this { mutable util::CheckedMutex m_mutex; - bool run_file_action(SyncFileActionMetadata&) REQUIRES(m_file_system_mutex); void init_metadata(SyncClientConfig config, const std::string& app_id); // internally create a new logger - used by configure() and set_logger_factory() From 84a0fb767a3c9a653d96ad30888d79af48f85196 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 8 Feb 2024 08:36:37 -0800 Subject: [PATCH 5/6] Remove unneccesary `mutable`s on CheckedMutex --- src/realm/db.hpp | 2 +- src/realm/object-store/impl/realm_coordinator.hpp | 2 +- src/realm/object-store/sync/app.hpp | 6 ++---- src/realm/object-store/sync/async_open_task.hpp | 2 +- src/realm/object-store/sync/impl/sync_metadata.hpp | 1 - src/realm/object-store/sync/sync_manager.hpp | 8 ++++---- src/realm/object-store/sync/sync_session.hpp | 8 ++++---- src/realm/object-store/sync/sync_user.hpp | 4 ++-- 8 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/realm/db.hpp b/src/realm/db.hpp index dd89636b809..c1237c58f4f 100644 --- a/src/realm/db.hpp +++ b/src/realm/db.hpp @@ -482,7 +482,7 @@ class DB : public std::enable_shared_from_this { class ReadLockGuard; // Member variables - mutable util::CheckedMutex m_mutex; + util::CheckedMutex m_mutex; int m_transaction_count GUARDED_BY(m_mutex) = 0; SlabAlloc m_alloc; std::unique_ptr m_history; diff --git a/src/realm/object-store/impl/realm_coordinator.hpp b/src/realm/object-store/impl/realm_coordinator.hpp index 5d88737a7b0..3608e9c8596 100644 --- a/src/realm/object-store/impl/realm_coordinator.hpp +++ b/src/realm/object-store/impl/realm_coordinator.hpp @@ -233,7 +233,7 @@ class RealmCoordinator : public std::enable_shared_from_this, Realm::Config m_config; std::shared_ptr m_db; - mutable util::CheckedMutex m_schema_cache_mutex; + util::CheckedMutex m_schema_cache_mutex; util::Optional m_cached_schema GUARDED_BY(m_schema_cache_mutex); uint64_t m_schema_version GUARDED_BY(m_schema_cache_mutex) = -1; uint64_t m_schema_transaction_version_min GUARDED_BY(m_schema_cache_mutex) = 0; diff --git a/src/realm/object-store/sync/app.hpp b/src/realm/object-store/sync/app.hpp index 37396fed6f0..f9371f66015 100644 --- a/src/realm/object-store/sync/app.hpp +++ b/src/realm/object-store/sync/app.hpp @@ -433,11 +433,9 @@ class App : public std::enable_shared_from_this, std::string get_ws_host_url() REQUIRES(!m_route_mutex); private: - // Local copy of app config - Config m_config; + const Config m_config; - // mutable to allow locking for reads in const functions - mutable util::CheckedMutex m_route_mutex; + util::CheckedMutex m_route_mutex; // The following variables hold the different paths to Atlas, depending on the // request being performed diff --git a/src/realm/object-store/sync/async_open_task.hpp b/src/realm/object-store/sync/async_open_task.hpp index 1a84fc1ec74..e1f28add434 100644 --- a/src/realm/object-store/sync/async_open_task.hpp +++ b/src/realm/object-store/sync/async_open_task.hpp @@ -76,7 +76,7 @@ class AsyncOpenTask : public std::enable_shared_from_this { std::shared_ptr<_impl::RealmCoordinator> m_coordinator GUARDED_BY(m_mutex); std::shared_ptr m_session GUARDED_BY(m_mutex); std::vector m_registered_callbacks GUARDED_BY(m_mutex); - mutable util::CheckedMutex m_mutex; + util::CheckedMutex m_mutex; const bool m_db_first_open; }; diff --git a/src/realm/object-store/sync/impl/sync_metadata.hpp b/src/realm/object-store/sync/impl/sync_metadata.hpp index 7f859b3341a..68b05459cee 100644 --- a/src/realm/object-store/sync/impl/sync_metadata.hpp +++ b/src/realm/object-store/sync/impl/sync_metadata.hpp @@ -25,7 +25,6 @@ #include #include -#include #include namespace realm { diff --git a/src/realm/object-store/sync/sync_manager.hpp b/src/realm/object-store/sync/sync_manager.hpp index 95e83e93732..3a32d88f9c9 100644 --- a/src/realm/object-store/sync/sync_manager.hpp +++ b/src/realm/object-store/sync/sync_manager.hpp @@ -295,7 +295,7 @@ class SyncManager : public std::enable_shared_from_this { std::shared_ptr get_user_for_identity(std::string const& identity) const noexcept REQUIRES(m_user_mutex); - mutable util::CheckedMutex m_mutex; + util::CheckedMutex m_mutex; void init_metadata(SyncClientConfig config, const std::string& app_id); @@ -303,7 +303,7 @@ class SyncManager : public std::enable_shared_from_this { void do_make_logger() REQUIRES(m_mutex); // Protects m_users - mutable util::CheckedMutex m_user_mutex; + util::CheckedMutex m_user_mutex; // A vector of all SyncUser objects. std::vector> m_users GUARDED_BY(m_user_mutex); @@ -315,12 +315,12 @@ class SyncManager : public std::enable_shared_from_this { mutable std::shared_ptr m_logger_ptr GUARDED_BY(m_mutex); // Protects m_file_manager and m_metadata_manager - mutable util::CheckedMutex m_file_system_mutex; + util::CheckedMutex m_file_system_mutex; std::unique_ptr m_file_manager GUARDED_BY(m_file_system_mutex); std::unique_ptr m_metadata_manager GUARDED_BY(m_file_system_mutex); // Protects m_sessions - mutable util::CheckedMutex m_session_mutex; + util::CheckedMutex m_session_mutex; // Map of sessions by path name. // Sessions remove themselves from this map by calling `unregister_session` once they're diff --git a/src/realm/object-store/sync/sync_session.hpp b/src/realm/object-store/sync/sync_session.hpp index 5c042edc281..cf80bb86780 100644 --- a/src/realm/object-store/sync/sync_session.hpp +++ b/src/realm/object-store/sync/sync_session.hpp @@ -446,8 +446,8 @@ class SyncSession : public std::enable_shared_from_this { // Return the subscription_store_base - to be used only for testing std::shared_ptr get_subscription_store_base() REQUIRES(!m_state_mutex); - mutable util::CheckedMutex m_state_mutex; - mutable util::CheckedMutex m_connection_state_mutex; + util::CheckedMutex m_state_mutex; + util::CheckedMutex m_connection_state_mutex; State m_state GUARDED_BY(m_state_mutex) = State::Inactive; @@ -457,7 +457,7 @@ class SyncSession : public std::enable_shared_from_this { ConnectionState m_connection_state GUARDED_BY(m_connection_state_mutex) = ConnectionState::Disconnected; size_t m_death_count GUARDED_BY(m_state_mutex) = 0; - mutable util::CheckedMutex m_config_mutex; + util::CheckedMutex m_config_mutex; RealmConfig m_config GUARDED_BY(m_config_mutex); const std::shared_ptr m_db; // The subscription store base is lazily created when needed, but never destroyed @@ -492,7 +492,7 @@ class SyncSession : public std::enable_shared_from_this { _impl::SyncProgressNotifier m_progress_notifier; ConnectionChangeNotifier m_connection_change_notifier; - mutable util::CheckedMutex m_external_reference_mutex; + util::CheckedMutex m_external_reference_mutex; class ExternalReference; std::weak_ptr m_external_reference GUARDED_BY(m_external_reference_mutex); diff --git a/src/realm/object-store/sync/sync_user.hpp b/src/realm/object-store/sync/sync_user.hpp index baa98b8ed11..00e39c15d1d 100644 --- a/src/realm/object-store/sync/sync_user.hpp +++ b/src/realm/object-store/sync/sync_user.hpp @@ -257,7 +257,7 @@ class SyncUser : public std::enable_shared_from_this, public Subscriba // used to locate existing files. std::vector m_legacy_identities; - mutable util::CheckedMutex m_mutex; + util::CheckedMutex m_mutex; // Set by the server. The unique ID of the user account on the Realm Application. const std::string m_identity; @@ -269,7 +269,7 @@ class SyncUser : public std::enable_shared_from_this, public Subscriba // Waiting sessions are those that should be asked to connect once this user is logged in. std::unordered_map> m_waiting_sessions; - mutable util::CheckedMutex m_tokens_mutex; + util::CheckedMutex m_tokens_mutex; // The user's refresh token. RealmJWT m_refresh_token GUARDED_BY(m_tokens_mutex); From 241668b4822d1f719454d315c29983f41a2dd7f3 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 8 Feb 2024 09:23:59 -0800 Subject: [PATCH 6/6] Don't require a final trailing slash in make_dir_recursive() --- CHANGELOG.md | 1 + src/realm/util/file.cpp | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea59ff9ca52..691de20dae5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ * Fixed invalid data in error reason string when registering a subscription change notification after the subscription has already failed. ([#6839](https://github.com/realm/realm-core/issues/6839), since v11.8.0) * Fixed crash in fulltext index using prefix search with no matches ([#7309](https://github.com/realm/realm-core/issues/7309), since v13.18.0) * Fix compilation and some warnings when building with Xcode 15.3 ([PR #7297](https://github.com/realm/realm-core/pull/7297)). +* util::make_dir_recursive() attempted to create a directory named "\0" if the path did not have a trailing slash ([PR #7329](https://github.com/realm/realm-core/pull/7329)). ### Breaking changes * None. diff --git a/src/realm/util/file.cpp b/src/realm/util/file.cpp index e30af8c5ca4..6686e5044ba 100644 --- a/src/realm/util/file.cpp +++ b/src/realm/util/file.cpp @@ -247,9 +247,13 @@ void make_dir_recursive(std::string path) path[sep] = 0; } try_make_dir(path); - if (sep < path.size()) + if (c) { path[sep] = c; - pos = sep + 1; + pos = sep + 1; + } + else { + break; + } } #endif }