Character profile sanitization improvements (#25579)

Validate that job and antag prototypes can actually be set in character profiles, rather than just checking if the prototype exists.

Make preferences system just call existing validation code when loading prototype from database, instead of some hacked-together stuff.

Also I made the character profile validation logic take dependencies in via parameter because fuck resolves.

diff --git a/Content.Client/Preferences/ClientPreferencesManager.cs b/Content.Client/Preferences/ClientPreferencesManager.cs
index 34b2c33140df473ce3de241493f892571426213b..b518493c9d4ddd3b6e2187785d39fa4c5d46f618 100644 (file)
--- a/Content.Client/Preferences/ClientPreferencesManager.cs
+++ b/Content.Client/Preferences/ClientPreferencesManager.cs
@@ -3,8 +3,10 @@ using System.Collections.Generic;
 using System.Linq;
 using Content.Shared.Preferences;
 using Robust.Client;
+using Robust.Shared.Configuration;
 using Robust.Shared.IoC;
 using Robust.Shared.Network;
+using Robust.Shared.Prototypes;
 using Robust.Shared.Utility;
 
 namespace Content.Client.Preferences
@@ -18,6 +20,8 @@ namespace Content.Client.Preferences
     {
         [Dependency] private readonly IClientNetManager _netManager = default!;
         [Dependency] private readonly IBaseClient _baseClient = default!;
+        [Dependency] private readonly IConfigurationManager _cfg = default!;
+        [Dependency] private readonly IPrototypeManager _prototypes = default!;
 
         public event Action? OnServerDataLoaded;
 
@@ -60,7 +64,7 @@ namespace Content.Client.Preferences
 
         public void UpdateCharacter(ICharacterProfile profile, int slot)
         {
-            profile.EnsureValid();
+            profile.EnsureValid(_cfg, _prototypes);
             var characters = new Dictionary<int, ICharacterProfile>(Preferences.Characters) {[slot] = profile};
             Preferences = new PlayerPreferences(characters, Preferences.SelectedCharacterIndex, Preferences.AdminOOCColor);
             var msg = new MsgUpdateCharacter

diff --git a/Content.Server/Preferences/Managers/ServerPreferencesManager.cs b/Content.Server/Preferences/Managers/ServerPreferencesManager.cs
index bcbe99edf519fc81ecc1d6c1db691327ef63c512..e489ae28d586757a71148a62cfee0d9005aebda9 100644 (file)
--- a/Content.Server/Preferences/Managers/ServerPreferencesManager.cs
+++ b/Content.Server/Preferences/Managers/ServerPreferencesManager.cs
@@ -99,7 +99,7 @@ namespace Content.Server.Preferences.Managers
 
             var curPrefs = prefsData.Prefs!;
 
-            profile.EnsureValid();
+            profile.EnsureValid(_cfg, _protos);
 
             var profiles = new Dictionary<int, ICharacterProfile>(curPrefs.Characters)
             {
@@ -270,34 +270,7 @@ namespace Content.Server.Preferences.Managers
 
             return new PlayerPreferences(prefs.Characters.Select(p =>
             {
-                ICharacterProfile newProf;
-                switch (p.Value)
-                {
-                    case HumanoidCharacterProfile hp:
-                    {
-                        var prototypeManager = IoCManager.Resolve<IPrototypeManager>();
-                        var selectedSpecies = HumanoidAppearanceSystem.DefaultSpecies;
-
-                        if (prototypeManager.TryIndex<SpeciesPrototype>(hp.Species, out var species) && species.RoundStart)
-                        {
-                            selectedSpecies = hp.Species;
-                        }
-
-                        newProf = hp
-                            .WithJobPriorities(
-                                hp.JobPriorities.Where(job =>
-                                    _protos.HasIndex<JobPrototype>(job.Key)))
-                            .WithAntagPreferences(
-                                hp.AntagPreferences.Where(antag =>
-                                    _protos.HasIndex<AntagPrototype>(antag)))
-                            .WithSpecies(selectedSpecies);
-                        break;
-                    }
-                    default:
-                        throw new NotSupportedException();
-                }
-
-                return new KeyValuePair<int, ICharacterProfile>(p.Key, newProf);
+                return new KeyValuePair<int, ICharacterProfile>(p.Key, p.Value.Validated(_cfg, _protos));
             }), prefs.SelectedCharacterIndex, prefs.AdminOOCColor);
         }
 

diff --git a/Content.Shared/Preferences/HumanoidCharacterProfile.cs b/Content.Shared/Preferences/HumanoidCharacterProfile.cs
index 92c5b083dcb265825b61d22d487556c2cf18cde9..9cf45aaefd56dd322211e50a2bd5b7d39aa3bdb1 100644 (file)
--- a/Content.Shared/Preferences/HumanoidCharacterProfile.cs
+++ b/Content.Shared/Preferences/HumanoidCharacterProfile.cs
@@ -371,10 +371,8 @@ namespace Content.Shared.Preferences
             return Appearance.MemberwiseEquals(other.Appearance);
         }
 
-        public void EnsureValid()
+        public void EnsureValid(IConfigurationManager configManager, IPrototypeManager prototypeManager)
         {
-            var prototypeManager = IoCManager.Resolve<IPrototypeManager>();
-
             if (!prototypeManager.TryIndex<SpeciesPrototype>(Species, out var speciesPrototype) || speciesPrototype.RoundStart == false)
             {
                 Species = SharedHumanoidAppearanceSystem.DefaultSpecies;
@@ -390,15 +388,10 @@ namespace Content.Shared.Preferences
             };
 
             // ensure the species can be that sex and their age fits the founds
-            var age = Age;
-            if (speciesPrototype != null)
-            {
-                if (!speciesPrototype.Sexes.Contains(sex))
-                {
-                    sex = speciesPrototype.Sexes[0];
-                }
-                age = Math.Clamp(Age, speciesPrototype.MinAge, speciesPrototype.MaxAge);
-            }
+            if (!speciesPrototype.Sexes.Contains(sex))
+                sex = speciesPrototype.Sexes[0];
+
+            var age = Math.Clamp(Age, speciesPrototype.MinAge, speciesPrototype.MaxAge);
 
             var gender = Gender switch
             {
@@ -425,7 +418,6 @@ namespace Content.Shared.Preferences
 
             name = name.Trim();
 
-            var configManager = IoCManager.Resolve<IConfigurationManager>();
             if (configManager.GetCVar(CCVars.RestrictedNames))
             {
                 name = Regex.Replace(name, @"[^A-Z,a-z,0-9, -]", string.Empty);
@@ -487,7 +479,7 @@ namespace Content.Shared.Preferences
             };
 
             var priorities = new Dictionary<string, JobPriority>(JobPriorities
-                .Where(p => prototypeManager.HasIndex<JobPrototype>(p.Key) && p.Value switch
+                .Where(p => prototypeManager.TryIndex<JobPrototype>(p.Key, out var job) && job.SetPreference && p.Value switch
                 {
                     JobPriority.Never => false, // Drop never since that's assumed default.
                     JobPriority.Low => true,
@@ -497,7 +489,7 @@ namespace Content.Shared.Preferences
                 }));
 
             var antags = AntagPreferences
-                .Where(prototypeManager.HasIndex<AntagPrototype>)
+                .Where(id => prototypeManager.TryIndex<AntagPrototype>(id, out var antag) && antag.SetPreference)
                 .ToList();
 
             var traits = TraitPreferences
@@ -530,6 +522,13 @@ namespace Content.Shared.Preferences
             _traitPreferences.AddRange(traits);
         }
 
+        public ICharacterProfile Validated(IConfigurationManager configManager, IPrototypeManager prototypeManager)
+        {
+            var profile = new HumanoidCharacterProfile(this);
+            profile.EnsureValid(configManager, prototypeManager);
+            return profile;
+        }
+
         // sorry this is kind of weird and duplicated,
         /// working inside these non entity systems is a bit wack
         public static string GetName(string species, Gender gender)

diff --git a/Content.Shared/Preferences/ICharacterProfile.cs b/Content.Shared/Preferences/ICharacterProfile.cs
index a17a74498d387951fe020a9b743d644d5a02fbf9..a9d30639bb35929badb60f576006eba9b80c04e0 100644 (file)
--- a/Content.Shared/Preferences/ICharacterProfile.cs
+++ b/Content.Shared/Preferences/ICharacterProfile.cs
@@ -1,4 +1,6 @@
 using Content.Shared.Humanoid;
+using Robust.Shared.Configuration;
+using Robust.Shared.Prototypes;
 
 namespace Content.Shared.Preferences
 {
@@ -13,6 +15,11 @@ namespace Content.Shared.Preferences
         /// <summary>
         ///     Makes this profile valid so there's no bad data like negative ages.
         /// </summary>
-        void EnsureValid();
+        void EnsureValid(IConfigurationManager configManager, IPrototypeManager prototypeManager);
+
+        /// <summary>
+        /// Gets a copy of this profile that has <see cref="EnsureValid"/> applied, i.e. no invalid data.
+        /// </summary>
+        ICharacterProfile Validated(IConfigurationManager configManager, IPrototypeManager prototypeManager);
     }
 }