[Databuckets] Nested Databuckets Protections and Improvements (#4748)

* Check for valid JSON before using it * Do not allow nested keys to set be set an expiration * Prevent overwriting of existing object or array * Nested deletion support * Update data_bucket.cpp * Test cases * More test cases, fix * Update databuckets.cpp * Update databuckets.cpp * Basic databucket tests * Update databuckets.cpp * Update databuckets.cpp
2026-06-11 20:08:37 +00:00 · 2025-03-04 13:16:21 -06:00
parent 3638d157b2
commit 0615864d51
8 changed files with 422 additions and 52 deletions
@@ -27,7 +27,8 @@ void DataBucket::SetData(const std::string &bucket_key, const std::string &bucke
 void DataBucket::SetData(const DataBucketKey &k_)
 {
 	DataBucketKey k = k_; // copy the key so we can modify it
-	if (k.key.find(NESTED_KEY_DELIMITER) != std::string::npos) {
+	bool is_nested = k.key.find(NESTED_KEY_DELIMITER) != std::string::npos;
+	if (is_nested) {
 		k.key = Strings::Split(k.key, NESTED_KEY_DELIMITER).front();
 	}

@@ -63,6 +64,10 @@ void DataBucket::SetData(const DataBucketKey &k_)
 		if (isalpha(k.expires[0]) || isalpha(k.expires[k.expires.length() - 1])) {
 			expires_time_unix = static_cast<int64>(std::time(nullptr)) + Strings::TimeToSeconds(k.expires);
 		}
+		if (is_nested) {
+			LogDataBuckets("Nested keys can't expire; set expiration on the parent key");
+			expires_time_unix = 0;
+		}
 	}

 	b.expires = expires_time_unix;
@@ -75,26 +80,45 @@ void DataBucket::SetData(const DataBucketKey &k_)
 		std::string existing_value = r.id > 0 ? r.value : "{}";
 		json json_value = json::object();

-		try {
-			json_value = json::parse(existing_value);
-		} catch (json::parse_error &e) {
-			LogDataBucketsDetail("Failed to parse JSON for key [{}]: {}", k_.key, e.what());
-			json_value = json::object(); // Reset to an empty object on error
+		// Check if the JSON is valid
+		if (Strings::IsValidJson(existing_value)) {
+			try {
+				json_value = json::parse(existing_value);
+			} catch (json::parse_error &e) {
+				LogDataBuckets("Failed to parse JSON for key [{}] [{}]", k_.key, e.what());
+				json_value = json::object(); // Reset to an empty object on error
+			}
 		}

 		// Recursively merge new key-value pair into the JSON object
 		auto nested_keys = Strings::Split(k_.key, NESTED_KEY_DELIMITER);
+		auto top_key = nested_keys.front();
+		// remove the top-level key
+		nested_keys.erase(nested_keys.begin());
+
 		json *current = &json_value;

 		for (size_t i = 0; i < nested_keys.size(); ++i) {
 			const std::string &key_part = nested_keys[i];
+
 			if (i == nested_keys.size() - 1) {
+
+				LogDataBucketsDetail("Setting key [{}] key_part [{}]", k.key, key_part);
+
+				// If the key already exists and is an object or array, prevent overwriting to avoid data loss
+				if (current->contains(key_part) &&
+					((*current)[key_part].is_object() || (*current)[key_part].is_array())) {
+					LogDataBuckets("Attempted to overwrite an existing object or array at key [{}] - skipping", k_.key);
+					return;
+				}
+
 				// Set the value at the final key
 				(*current)[key_part] = k_.value;
 			} else {
 				// Traverse or create nested objects
 				if (!current->contains(key_part)) {
 					(*current)[key_part] = json::object();
+					LogDataBucketsDetail("Creating nested root key [{}] key_part [{}]", k.key, key_part);
 				} else if (!(*current)[key_part].is_object()) {
 					// If key exists but is not an object, reset to object to avoid conflicts
 					(*current)[key_part] = json::object();
@@ -105,7 +129,7 @@ void DataBucket::SetData(const DataBucketKey &k_)

 		// Serialize JSON back to string
 		b.value = json_value.dump();
-		b.key_ = nested_keys.front(); // Use the top-level key
+		b.key_ = top_key; // Use the top-level key
 	}

 	if (bucket_id) {
@@ -142,12 +166,20 @@ DataBucketsRepository::DataBuckets DataBucket::ExtractNestedValue(
 	const std::string &full_key)
 {
 	auto nested_keys = Strings::Split(full_key, NESTED_KEY_DELIMITER);
+	auto top_key = nested_keys.front();
+	nested_keys.erase(nested_keys.begin());
 	json json_value;

+	// Check if the JSON is valid
+	if (!Strings::IsValidJson(bucket.value)) {
+		LogDataBuckets("Invalid JSON for key [{}]", bucket.key_);
+		return DataBucketsRepository::NewEntity();
+	}
+
 	try {
 		json_value = json::parse(bucket.value); // Parse the JSON
 	} catch (json::parse_error &ex) {
-		LogDataBucketsDetail("Failed to parse JSON for key [{}]: {}", bucket.key_, ex.what());
+		LogDataBuckets("Failed to parse JSON for key [{}] [{}]", bucket.key_, ex.what());
 		return DataBucketsRepository::NewEntity(); // Return empty entity on parse error
 	}

@@ -336,44 +368,116 @@ bool DataBucket::GetDataBuckets(Mob *mob)

 bool DataBucket::DeleteData(const DataBucketKey &k)
 {
-	if (CanCache(k)) {
-		size_t size_before = g_data_bucket_cache.size();
+	bool is_nested_key = k.key.find(NESTED_KEY_DELIMITER) != std::string::npos;

-		// delete from cache where contents match
-		g_data_bucket_cache.erase(
-			std::remove_if(
-				g_data_bucket_cache.begin(),
-				g_data_bucket_cache.end(),
-				[&](DataBucketsRepository::DataBuckets &e) {
-					return CheckBucketMatch(e, k);
-				}
-			),
-			g_data_bucket_cache.end()
-		);
+	if (!is_nested_key) {
+		// Update cache
+		if (CanCache(k)) {
+			// delete from cache where contents match
+			g_data_bucket_cache.erase(
+				std::remove_if(
+					g_data_bucket_cache.begin(),
+					g_data_bucket_cache.end(),
+					[&](DataBucketsRepository::DataBuckets &e) {
+						return CheckBucketMatch(e, k);
+					}
+				),
+				g_data_bucket_cache.end()
+			);
+		}

-		LogDataBuckets(
-			"Deleting bucket key [{}] bot_id [{}] account_id [{}] character_id [{}] npc_id [{}] bot_id [{}] zone_id [{}] instance_id [{}] cache size before [{}] after [{}]",
-			k.key,
-			k.bot_id,
-			k.account_id,
-			k.character_id,
-			k.npc_id,
-			k.bot_id,
-			k.zone_id,
-			k.instance_id,
-			size_before,
-			g_data_bucket_cache.size()
+		// Regular key deletion, no nesting involved
+		return DataBucketsRepository::DeleteWhere(
+			database,
+			fmt::format("{} `key` = '{}'", DataBucket::GetScopedDbFilters(k), k.key)
 		);
 	}

-	return DataBucketsRepository::DeleteWhere(
-		database,
-		fmt::format(
-			"{} `key` = '{}'",
-			DataBucket::GetScopedDbFilters(k),
-			k.key
-		)
-	);
+	// If it's a nested key, retrieve the top-level JSON object
+	auto top_level_key = Strings::Split(k.key, NESTED_KEY_DELIMITER).front();
+	DataBucketKey top_level_k = k;
+	top_level_k.key = top_level_key;
+
+	auto r = GetData(top_level_k);
+	if (r.id == 0 || r.value.empty() || !Strings::IsValidJson(r.value)) {
+		LogDataBuckets("Attempted to delete nested key [{}] but parent key [{}] does not exist or is invalid JSON", k.key, top_level_key);
+		return false;
+	}
+
+	json json_value;
+	try {
+		json_value = json::parse(r.value);
+	} catch (json::parse_error &ex) {
+		LogDataBuckets("Failed to parse JSON for key [{}] [{}]", top_level_key, ex.what());
+		return false;
+	}
+
+	// Recursively remove the nested key
+	auto nested_keys = Strings::Split(k.key, NESTED_KEY_DELIMITER);
+	auto top_key = nested_keys.front();
+	nested_keys.erase(nested_keys.begin());
+	json *current = &json_value;
+
+	for (size_t i = 0; i < nested_keys.size(); ++i) {
+		const std::string &key_part = nested_keys[i];
+
+		if (i == nested_keys.size() - 1) {
+			// Last key in the hierarchy - delete it
+			if (current->contains(key_part)) {
+				current->erase(key_part);
+				LogDataBuckets("Deleted nested key [{}] from [{}]", key_part, k.key);
+			} else {
+				LogDataBuckets("Key [{}] not found in JSON - nothing to delete", k.key);
+				return false;
+			}
+		} else {
+			if (!current->contains(key_part) || !(*current)[key_part].is_object()) {
+				LogDataBuckets("Parent key [{}] does not exist or is not an object", key_part);
+				return false;
+			}
+			current = &(*current)[key_part];
+		}
+	}
+
+	// If the JSON object is now empty, delete the top-level key
+	if (json_value.empty()) {
+		LogDataBuckets("Top-level key [{}] is now empty, deleting entire entry", top_level_key);
+
+		// delete cache
+		if (CanCache(k)) {
+			g_data_bucket_cache.erase(
+				std::remove_if(
+					g_data_bucket_cache.begin(),
+					g_data_bucket_cache.end(),
+					[&](DataBucketsRepository::DataBuckets &e) {
+						return CheckBucketMatch(e, top_level_k);
+					}
+				),
+				g_data_bucket_cache.end()
+			);
+		}
+
+		return DataBucketsRepository::DeleteWhere(
+			database,
+			fmt::format("{} `key` = '{}'", DataBucket::GetScopedDbFilters(k), top_level_key)
+		);
+	}
+
+	// Otherwise, update the existing JSON without the deleted key
+	r.value = json_value.dump();
+	DataBucketsRepository::UpdateOne(database, r);
+
+	// Update cache
+	if (CanCache(k)) {
+		for (auto &e : g_data_bucket_cache) {
+			if (CheckBucketMatch(e, top_level_k)) {
+				e.value = r.value;
+				break;
+			}
+		}
+	}
+
+	return true;
 }

 std::string DataBucket::GetDataExpires(const DataBucketKey &k)