Skip to content

Commit

Permalink
Disallow asigning to init-only properties in bindings
Browse files Browse the repository at this point in the history
  • Loading branch information
exyi committed Feb 16, 2025
1 parent 7972151 commit 58dc9dd
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ public Expression<BindingDelegate> CompileToDelegate(
{
var replacementVisitor = new BindingCompiler.ParameterReplacementVisitor(dataContext);
var expr = replacementVisitor.Visit(expression.Expression);
var initOnlyPropertyChecker = binding is IStaticCommandBinding
? InitOnlyPropertyCheckingVisitor.InstanceDynamicError // allow this client-side
: InitOnlyPropertyCheckingVisitor.Instance;
expr = initOnlyPropertyChecker.Visit(expr);
expr = new ExpressionNullPropagationVisitor(e => true).Visit(expr);
expr = ExpressionUtils.ConvertToObject(expr);
expr = replacementVisitor.WrapExpression(expr, contextObject: binding);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
using System;
using System.Linq.Expressions;
using System.Reflection;
using System.Runtime.CompilerServices;
using DotVVM.Framework.Utils;

namespace DotVVM.Framework.Compilation.Binding
{
/// <summary> Checks that assigned properties have setters without the <see cref="IsExternalInit" /> attribute.

Check failure on line 9 in src/Framework/Framework/Compilation/Binding/InitOnlyPropertyCheckingVisitor.cs

View workflow job for this annotation

GitHub Actions / Build published projects without warnings (Release)

Ambiguous reference in cref attribute: 'IsExternalInit'. Assuming 'IsExternalInit', but could have also matched other overloads including 'IsExternalInit'.

Check failure on line 9 in src/Framework/Framework/Compilation/Binding/InitOnlyPropertyCheckingVisitor.cs

View workflow job for this annotation

GitHub Actions / Build published projects without warnings (Release)

Ambiguous reference in cref attribute: 'IsExternalInit'. Assuming 'IsExternalInit', but could have also matched other overloads including 'IsExternalInit'.

Check failure on line 9 in src/Framework/Framework/Compilation/Binding/InitOnlyPropertyCheckingVisitor.cs

View workflow job for this annotation

GitHub Actions / Build published projects without warnings (Debug)

Ambiguous reference in cref attribute: 'IsExternalInit'. Assuming 'IsExternalInit', but could have also matched other overloads including 'IsExternalInit'.

Check failure on line 9 in src/Framework/Framework/Compilation/Binding/InitOnlyPropertyCheckingVisitor.cs

View workflow job for this annotation

GitHub Actions / Build published projects without warnings (Debug)

Ambiguous reference in cref attribute: 'IsExternalInit'. Assuming 'IsExternalInit', but could have also matched other overloads including 'IsExternalInit'.
/// <paramref name="staticError"/> specifies if the error should be thrown immediately, or only when actually executed at runtime. </summary>
public class InitOnlyPropertyCheckingVisitor(bool staticError = true): ExpressionVisitor
{
public static InitOnlyPropertyCheckingVisitor Instance { get; } = new InitOnlyPropertyCheckingVisitor(true);
public static InitOnlyPropertyCheckingVisitor InstanceDynamicError { get; } = new InitOnlyPropertyCheckingVisitor(false);

protected override Expression VisitBinary(BinaryExpression node)
{
if (node is { NodeType: ExpressionType.Assign, Left: MemberExpression { Member: PropertyInfo assignedProperty } } &&
assignedProperty.IsInitOnly())
{
var message = $"Property '{assignedProperty.DeclaringType!.Name}.{assignedProperty.Name}' is init-only and cannot be assigned to in bindings executed server-side. You can only assign to such properties in staticCommand bindings executed on the client.";
if (staticError)
{
throw new Exception(message);
}
else
{
return Expression.Throw(Expression.New(typeof(Exception).GetConstructor([typeof(string)]), [Expression.Constant(message)]));

Check failure on line 28 in src/Framework/Framework/Compilation/Binding/InitOnlyPropertyCheckingVisitor.cs

View workflow job for this annotation

GitHub Actions / Build published projects without warnings (Release)

Possible null reference argument for parameter 'constructor' in 'NewExpression Expression.New(ConstructorInfo constructor, params Expression[]? arguments)'.

Check failure on line 28 in src/Framework/Framework/Compilation/Binding/InitOnlyPropertyCheckingVisitor.cs

View workflow job for this annotation

GitHub Actions / Build published projects without warnings (Release)

Possible null reference argument for parameter 'constructor' in 'NewExpression Expression.New(ConstructorInfo constructor, params Expression[]? arguments)'.

Check failure on line 28 in src/Framework/Framework/Compilation/Binding/InitOnlyPropertyCheckingVisitor.cs

View workflow job for this annotation

GitHub Actions / Build published projects without warnings (Debug)

Possible null reference argument for parameter 'constructor' in 'NewExpression Expression.New(ConstructorInfo constructor, params Expression[]? arguments)'.

Check failure on line 28 in src/Framework/Framework/Compilation/Binding/InitOnlyPropertyCheckingVisitor.cs

View workflow job for this annotation

GitHub Actions / Build published projects without warnings (Debug)

Possible null reference argument for parameter 'constructor' in 'NewExpression Expression.New(ConstructorInfo constructor, params Expression[]? arguments)'.
}
}

return base.VisitBinary(node);
}
}
}
16 changes: 16 additions & 0 deletions src/Tests/Binding/BindingCompilationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1354,6 +1354,17 @@ public void BindingCompiler_AmbiguousMatches()
var result = ExecuteBinding("Strings.SomeResource", new[] { new NamespaceImport("DotVVM.Framework.Tests.Ambiguous"), new NamespaceImport("DotVVM.Framework.Tests") }, new TestViewModel());
Assert.AreEqual("hello", result);
}

[TestMethod]
public void BindingCompiler_InitOnlyPropertyCannotBeAsigned()
{
var vm = new TestViewModelWithInitOnlyProperties() { MyProperty = 999 };

var exception = XAssert.ThrowsAny<Exception>(() => ExecuteBinding("_this.MyProperty = 1", vm));
XAssert.Contains("Property 'TestViewModelWithInitOnlyProperties.MyProperty' is init-only", exception.Message);

Assert.AreEqual(999, vm.MyProperty);
}
}
public class TestViewModel
{
Expand Down Expand Up @@ -1574,6 +1585,11 @@ public class TestViewModel5
public int[] Array { get; set; } = new int[] { 1, 2, 3 };
}

public class TestViewModelWithInitOnlyProperties
{
public int MyProperty { get; init; }
}

public struct TestStruct
{
public int Int { get; set; }
Expand Down

0 comments on commit 58dc9dd

Please sign in to comment.