serializer: disable cloning IDotvvmViewModel unless it has JsonConstr…

…uctor
riganti · Dec 6, 2022 · 71e7527 · 71e7527
1 parent 1aaa54b
commit 71e7527
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 2 deletions.
diff --git a/src/Framework/Framework/Utils/ReflectionUtils.cs b/src/Framework/Framework/Utils/ReflectionUtils.cs
@@ -569,5 +569,9 @@ internal static void ClearCaches(Type[] types)
                 cache_GetTypeHash.TryRemove(t, out _);
             }
         }
+
+        internal static bool IsInitOnly(this PropertyInfo prop) =>
+            prop.SetMethod is { ReturnParameter: {} returnParameter } &&
+            returnParameter.GetRequiredCustomModifiers().Any(t => t == typeof(System.Runtime.CompilerServices.IsExternalInit));
     }
 }
diff --git a/src/Framework/Framework/ViewModel/Serialization/ViewModelSerializationMap.cs b/src/Framework/Framework/ViewModel/Serialization/ViewModelSerializationMap.cs
@@ -154,7 +154,18 @@ public ReaderDelegate CreateReaderFactory()
             // If we have constructor property or if we have { get; init; } property, we always create new instance
             var alwaysCallConstructor = Properties.Any(p => p.TransferToServer && (
                 p.ConstructorParameter is {} ||
-                true == p.PropertyInfo?.SetMethod?.ReturnParameter?.GetRequiredCustomModifiers().Any(t => t == typeof(System.Runtime.CompilerServices.IsExternalInit))));
+                p.PropertyInfo.IsInitOnly()));
+
+            // We don't want to clone IDotvvmViewModel automatically, because the user is likely to register this specific instance somewhere
+            if (alwaysCallConstructor && typeof(IDotvvmViewModel).IsAssignableFrom(Type) && Constructor is {} && Constructor.IsDefined(typeof(JsonConstructorAttribute)))
+            {
+                var cloneReason =
+                    Properties.FirstOrDefault(p => p.TransferToServer && p.PropertyInfo.IsInitOnly()) is {} initProperty
+                        ? $"init-only property {initProperty.Name} is transferred client → server" :
+                    Properties.FirstOrDefault(p => p.TransferToServer && p.ConstructorParameter is {}) is {} ctorProperty
+                        ? $"property {ctorProperty.Name} must be injected into constructor parameter {ctorProperty.ConstructorParameter!.Name}" : "internal bug";
+                throw new Exception($"Deserialization of {Type.ToCode()} is not allowed, because it implements IDotvvmViewModel and {cloneReason}. To allow cloning the object on deserialization, mark a constructor with [JsonConstructor].");
+            }
             var constructorCall = CallConstructor(servicesParameter, propertyVars, throwImmediately: alwaysCallConstructor);
 
             // curly brackets are used for variables and methods from the context of this factory method

diff --git a/src/Samples/Tests/Tests/Control/ComboBoxTests.cs b/src/Samples/Tests/Tests/Control/ComboBoxTests.cs
@@ -230,7 +230,7 @@ public void Control_ComboBox_ItemBinding_ItemValueBinding_SelectedValue_StringTo
             RunInAllBrowsers(browser => {
                 browser.NavigateToUrl(SamplesRouteUrls.ControlSamples_ComboBox_ItemBinding_ItemValueBinding_SelectedValue_StringToInt_Error);
 
-                AssertUI.InnerText(browser.First(".exceptionMessage"), s => s.Contains("System.String") && s.Contains("not assignable") && s.Contains("System.Int32"));
+                AssertUI.InnerText(browser.First(".exceptionMessage"), s => s.Contains("string") && s.Contains("not assignable") && s.Contains("System.Int32"));
                 AssertUI.InnerText(browser.First("p.summary"), s => s.Contains("DotVVM.Framework.Compilation.DotvvmCompilationException"));
                 AssertUI.InnerText(browser.First(".errorUnderline"), s => s.Contains("{value: SelectedInt}"));
             });