Merge pull request #55096 from CyrusNajmabadi/serializationFailure

Ensure we deserialize properly, even in the presence of values we don't understand
dotnet · Jul 25, 2021 · 0574a33 · 0574a33
2 parents 1cba150 + 3e5fb1f
commit 0574a33
Showing 1 changed file with 10 additions and 8 deletions.
diff --git a/src/Workspaces/Core/Portable/Options/SerializableOptionSet.cs b/src/Workspaces/Core/Portable/Options/SerializableOptionSet.cs
@@ -310,8 +310,7 @@ public static SerializableOptionSet Deserialize(ObjectReader reader, IOptionServ
             var builder = ImmutableDictionary.CreateBuilder<OptionKey, object?>();
             for (var i = 0; i < count; i++)
             {
-                if (!TryDeserializeOptionKey(reader, lookup, out var optionKey))
-                    continue;
+                var optionKeyOpt = TryDeserializeOptionKey(reader, lookup);
 
                 var kind = (OptionValueKind)reader.ReadInt32();
                 var readValue = kind switch
@@ -322,6 +321,10 @@ public static SerializableOptionSet Deserialize(ObjectReader reader, IOptionServ
                     _ => reader.ReadValue(),
                 };
 
+                if (optionKeyOpt == null)
+                    continue;
+
+                var optionKey = optionKeyOpt.Value;
                 if (!serializableOptions.Contains(optionKey.Option))
                     continue;
 
@@ -362,7 +365,7 @@ public static SerializableOptionSet Deserialize(ObjectReader reader, IOptionServ
             var changedKeysBuilder = ImmutableHashSet.CreateBuilder<OptionKey>();
             for (var i = 0; i < count; i++)
             {
-                if (TryDeserializeOptionKey(reader, lookup, out var optionKey))
+                if (TryDeserializeOptionKey(reader, lookup) is { } optionKey)
                     changedKeysBuilder.Add(optionKey);
             }
 
@@ -374,7 +377,7 @@ public static SerializableOptionSet Deserialize(ObjectReader reader, IOptionServ
                 languages, workspaceOptionSet, serializableOptions, serializableOptionValues,
                 changedOptionKeysSerializable, changedOptionKeysNonSerializable: ImmutableHashSet<OptionKey>.Empty);
 
-            static bool TryDeserializeOptionKey(ObjectReader reader, ILookup<string, IOption> lookup, out OptionKey deserializedOptionKey)
+            static OptionKey? TryDeserializeOptionKey(ObjectReader reader, ILookup<string, IOption> lookup)
             {
                 var name = reader.ReadString();
                 var feature = reader.ReadString();
@@ -386,13 +389,12 @@ static bool TryDeserializeOptionKey(ObjectReader reader, ILookup<string, IOption
                     if (option.Feature == feature &&
                         option.IsPerLanguage == isPerLanguage)
                     {
-                        deserializedOptionKey = new OptionKey(option, language);
-                        return true;
+                        return new OptionKey(option, language);
                     }
                 }
 
-                deserializedOptionKey = default;
-                return false;
+                Debug.Fail($"Failed to deserialize: {name}-{feature}-{isPerLanguage}-{language}");
+                return null;
             }
         }