Refactor freezing of JS objects

- Shallow-freezing of objects is never used in our codebase, so remove
   that code path.
 - Deep-freeze is bugged in recent versions of SpiderMonkey (see bug
   https://bugzilla.mozilla.org/show_bug.cgi?id=1930258). Until a fix
   and/or a better API is provided, reimplement this feature by
   recusively freezing object properties.
 - Add tests for the deepfreeze function.

source/graphics/MapGenerator.cpp

@@ -388,7 +388,7 @@ Script::StructuredClone RunMapGenerationScript(const StopToken stopToken, std::a
 	}
 
 	// Prevent unintentional modifications to the settings object by random map scripts
-	if (!Script::FreezeObject(rq, settingsVal, true))
+	if (!Script::DeepFreezeObject(rq, settingsVal))
 	{
 		LOGERROR("RunMapGenerationScript: Failed to deepfreeze settings");
 		return nullptr;

source/lobby/XmppClient.cpp

@@ -704,7 +704,7 @@ void XmppClient::CreateGUIMessage(
 
 	JS::RootedObject messageObj(rq.cx, message.toObjectOrNull());
 	SetGUIMessageProperty(rq, messageObj, args...);
-	Script::FreezeObject(rq, message, true);
+	Script::DeepFreezeObject(rq, message);
 	m_GuiMessageQueue.push_back(JS::Heap<JS::Value>(message));
 }
 
@@ -755,7 +755,7 @@ JS::Value XmppClient::GuiPollNewMessages(const ScriptInterface& guiInterface)
 		if (true)
 		{
 			Script::SetProperty(rq, historicMessage, "historic", true);
-			Script::FreezeObject(rq, historicMessage, true);
+			Script::DeepFreezeObject(rq, historicMessage);
 			m_HistoricGuiMessages.push_back(JS::Heap<JS::Value>(historicMessage));
 		}
 		else

source/ps/Replay.cpp

@@ -279,7 +279,7 @@ void CReplayPlayer::Replay(const bool serializationtest, const int rejointesttur
 			ScriptRequest rq(g_Game->GetSimulation2()->GetScriptInterface());
 			JS::RootedValue data(rq.cx);
 			Script::ParseJSON(rq, line, &data);
-			Script::FreezeObject(rq, data, true);
+			Script::DeepFreezeObject(rq, data);
 			commands.emplace_back(SimulationCommand(player, rq.cx, data));
 		}
 		else if (type == "hash" || type == "hash-quick")

source/ps/scripting/JSInterface_Mod.cpp

@@ -124,7 +124,7 @@ JS::Value GetEngineInfo(const ScriptInterface& scriptInterface)
 		 "engine_version", engine_version,
 		 "mods", mods);
 
-	Script::FreezeObject(rq, metainfo, true);
+	Script::DeepFreezeObject(rq, metainfo);
 
 	return metainfo;
 }

source/scriptinterface/Object.h

@@ -150,17 +150,35 @@ inline bool GetObjectClassName(const ScriptRequest& rq, JS::HandleValue val, T&
 	return GetObjectClassName(rq, obj, name);
 }
 
-inline bool FreezeObject(const ScriptRequest& rq, JS::HandleValue objVal, bool deep)
+inline bool DeepFreezeObject(const ScriptRequest& rq, JS::HandleValue objVal)
 {
 	if (!objVal.isObject())
+	{
+		LOGERROR("DeepFreezeObject expected object type!");
+		return false;
+	}
+
+	// Get all properties and recursively freeze those that are objects
+	JS::RootedObject obj(rq.cx, &objVal.toObject());
+	JS::RootedIdVector props(rq.cx);
+	if (!js::GetPropertyKeys(rq.cx, obj, JSITER_HIDDEN, &props))
 		return false;
 
-	JS::RootedObject obj(rq.cx, &objVal.toObject());
+	for (size_t i = 0; i < props.length(); ++i)
+	{
+		JS::RootedId id(rq.cx, props[i]);
+		JS::RootedValue val(rq.cx);
+		if (!JS_IdToValue(rq.cx, id, &val))
+			return false;
 
-	if (deep)
-		return JS_DeepFreezeObject(rq.cx, obj);
-	else
-		return JS_FreezeObject(rq.cx, obj);
+		if (!val.isObject())
+			continue;
+
+		if (!DeepFreezeObject(rq, val))
+			return false;
+	}
+
+	return JS_FreezeObject(rq.cx, obj);
 }
 
 /**

source/scriptinterface/ScriptInterface.cpp

@@ -214,7 +214,7 @@ JS::Value deepfreeze(const ScriptInterface& scriptInterface, JS::HandleValue val
 		return JS::UndefinedValue();
 	}
 
-	Script::FreezeObject(rq, val, true);
+	Script::DeepFreezeObject(rq, val);
 	return val;
 }
 

source/scriptinterface/tests/test_ScriptInterface.h

@@ -134,6 +134,24 @@ public:
 		}
 	}
 
+	void test_deepfreeze()
+	{
+		ScriptInterface script("Test", "Test", g_ScriptContext);
+		TestLogger logger;
+
+		TS_ASSERT(!script.LoadScript(L"testFreezeObject.js", "var obj1 = { foo: \"a\"}; deepfreeze(obj1); obj1.bar = \"b\";"));
+		TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "Object is not extensible");
+
+		TS_ASSERT(!script.LoadScript(L"testDeepFreezeObject.js", "var obj2 = { foo: \"a\"}; deepfreeze(obj2); obj2.foo = \"b\";"));
+		TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "\"foo\" is read-only");
+
+		TS_ASSERT(!script.LoadScript(L"testFreezeArray.js", "var array = [0]; deepfreeze(array); array.push(1);"));
+		TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "can\'t define array index property past the end of an array with non-writable length");
+
+		TS_ASSERT(!script.LoadScript(L"testFreezeFunction.js", "var fn = a => a+1; deepfreeze(fn); fn.name = \"foo\";"));
+		TS_ASSERT_STR_CONTAINS(logger.GetOutput(), "\"name\" is read-only");
+	}
+
 	/**
 	 * This test is mainly to make sure that all required template overloads get instantiated at least once so that compiler errors
 	 * in these functions are revealed instantly (but it also tests the basic functionality of these functions).

source/simulation2/Simulation2.cpp

@@ -171,7 +171,7 @@ public:
 		for (const SimulationCommand& command : commands)
 		{
 			JS::RootedValue tmpCommand(rqNew.cx, Script::CloneValueFromOtherCompartment(newScript, oldScript, command.data));
-			Script::FreezeObject(rqNew, tmpCommand, true);
+			Script::DeepFreezeObject(rqNew, tmpCommand);
 			SimulationCommand cmd(command.player, rqNew.cx, tmpCommand);
 			newCommands.emplace_back(std::move(cmd));
 		}
@@ -849,7 +849,7 @@ void CSimulation2::LoadMapSettings()
 	// Initialize here instead of in Update()
 	ScriptFunction::CallVoid(rq, global, "LoadMapSettings", m->m_MapSettings);
 
-	Script::FreezeObject(rq, m->m_InitAttributes, true);
+	Script::DeepFreezeObject(rq, m->m_InitAttributes);
 	GetScriptInterface().SetGlobal("InitAttributes", m->m_InitAttributes, true, true, true);
 
 	if (!m->m_StartupScript.empty())

source/simulation2/scripting/EngineScriptConversions.cpp

@@ -73,7 +73,7 @@ template<> void Script::ToJSVal<CParamNode>(const ScriptRequest& rq,  JS::Mutabl
 	// Prevent modifications to the object, so that it's safe to share between
 	// components and to reconstruct on deserialization
 	if (ret.isObject())
-		Script::FreezeObject(rq, ret, true);
+		Script::DeepFreezeObject(rq, ret);
 }
 
 template<> void Script::ToJSVal<const CParamNode*>(const ScriptRequest& rq,  JS::MutableHandleValue ret, const CParamNode* const& val)

source/simulation2/system/TurnManager.cpp

@@ -223,7 +223,7 @@ void CTurnManager::AddCommand(int client, int player, JS::HandleValue data, u32
 
 	ScriptRequest rq(m_Simulation2.GetScriptInterface());
 
-	Script::FreezeObject(rq, data, true);
+	Script::DeepFreezeObject(rq, data);
 
 	size_t command_in_turns = turn - (m_CurrentTurn+1);
 	if (m_QueuedCommands.size() <= command_in_turns)
@@ -295,7 +295,7 @@ void CTurnManager::QuickSave(JS::HandleValue GUIMetadata)
 
 	m_QuickSaveMetadata.set(Script::DeepCopy(rq, GUIMetadata));
 	// Freeze state to ensure that consectuvie loads don't modify the state
-	Script::FreezeObject(rq, m_QuickSaveMetadata, true);
+	Script::DeepFreezeObject(rq, m_QuickSaveMetadata);
 
 	LOGMESSAGERENDER("Quicksaved game");
 }