Skip to content

Commit 11894ad

Browse files
bkoelmanbkoelman
committed
Harden Attr/Relationship attributes against invalid input
1 parent 9e03e0f commit 11894ad

File tree

8 files changed

+523
-3
lines changed

8 files changed

+523
-3
lines changed

‎src/JsonApiDotNetCore.Annotations/Resources/Annotations/AttrAttribute.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public AttrCapabilities Capabilities
3131
set => _capabilities = value;
3232
}
3333

34+
/// <inheritdoc />
3435
public override bool Equals(object? obj)
3536
{
3637
if (ReferenceEquals(this, obj))
@@ -48,6 +49,7 @@ public override bool Equals(object? obj)
4849
return Capabilities == other.Capabilities && base.Equals(other);
4950
}
5051

52+
/// <inheritdoc />
5153
public override int GetHashCode()
5254
{
5355
return HashCode.Combine(Capabilities, base.GetHashCode());

‎src/JsonApiDotNetCore.Annotations/Resources/Annotations/HasManyAttribute.cs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Collections;
12
using JetBrains.Annotations;
23

34
// ReSharper disable NonReadonlyMemberInGetHashCode
@@ -65,6 +66,34 @@ private bool EvaluateIsManyToMany()
6566
return false;
6667
}
6768

69+
/// <inheritdoc />
70+
public override void SetValue(object resource, object? newValue)
71+
{
72+
ArgumentGuard.NotNull(newValue);
73+
AssertIsIdentifiableCollection(newValue);
74+
75+
base.SetValue(resource, newValue);
76+
}
77+
78+
private void AssertIsIdentifiableCollection(object newValue)
79+
{
80+
if (newValue is not IEnumerable enumerable)
81+
{
82+
throw new InvalidOperationException($"Resource of type '{newValue.GetType()}' must be a collection.");
83+
}
84+
85+
foreach (object? element in enumerable)
86+
{
87+
if (element == null)
88+
{
89+
throw new InvalidOperationException("Resource collection must not contain null values.");
90+
}
91+
92+
AssertIsIdentifiable(element);
93+
}
94+
}
95+
96+
/// <inheritdoc />
6897
public override bool Equals(object? obj)
6998
{
7099
if (ReferenceEquals(this, obj))
@@ -82,6 +111,7 @@ public override bool Equals(object? obj)
82111
return _capabilities == other._capabilities && base.Equals(other);
83112
}
84113

114+
/// <inheritdoc />
85115
public override int GetHashCode()
86116
{
87117
return HashCode.Combine(_capabilities, base.GetHashCode());

‎src/JsonApiDotNetCore.Annotations/Resources/Annotations/HasOneAttribute.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,14 @@ private bool EvaluateIsOneToOne()
6464
return false;
6565
}
6666

67+
/// <inheritdoc />
68+
public override void SetValue(object resource, object? newValue)
69+
{
70+
AssertIsIdentifiable(newValue);
71+
base.SetValue(resource, newValue);
72+
}
73+
74+
/// <inheritdoc />
6775
public override bool Equals(object? obj)
6876
{
6977
if (ReferenceEquals(this, obj))
@@ -81,6 +89,7 @@ public override bool Equals(object? obj)
8189
return _capabilities == other._capabilities && base.Equals(other);
8290
}
8391

92+
/// <inheritdoc />
8493
public override int GetHashCode()
8594
{
8695
return HashCode.Combine(_capabilities, base.GetHashCode());

‎src/JsonApiDotNetCore.Annotations/Resources/Annotations/RelationshipAttribute.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ public bool CanInclude
8686
set => _canInclude = value;
8787
}
8888

89+
/// <inheritdoc />
8990
public override bool Equals(object? obj)
9091
{
9192
if (ReferenceEquals(this, obj))
@@ -103,6 +104,7 @@ public override bool Equals(object? obj)
103104
return _rightType?.ClrType == other._rightType?.ClrType && Links == other.Links && base.Equals(other);
104105
}
105106

107+
/// <inheritdoc />
106108
public override int GetHashCode()
107109
{
108110
return HashCode.Combine(_rightType?.ClrType, Links, base.GetHashCode());

‎src/JsonApiDotNetCore.Annotations/Resources/Annotations/ResourceFieldAttribute.cs‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ internal set
6868
public object? GetValue(object resource)
6969
{
7070
ArgumentGuard.NotNull(resource);
71+
AssertIsIdentifiable(resource);
7172

7273
if (Property.GetMethod == null)
7374
{
@@ -82,17 +83,18 @@ internal set
8283
{
8384
throw new InvalidOperationException(
8485
$"Unable to get property value of '{Property.DeclaringType!.Name}.{Property.Name}' on instance of type '{resource.GetType().Name}'.",
85-
exception);
86+
exception.InnerException ?? exception);
8687
}
8788
}
8889

8990
/// <summary>
9091
/// Sets the value of this field on the specified resource instance. Throws if the property is read-only or if the field does not belong to the specified
9192
/// resource instance.
9293
/// </summary>
93-
public void SetValue(object resource, object? newValue)
94+
public virtual void SetValue(object resource, object? newValue)
9495
{
9596
ArgumentGuard.NotNull(resource);
97+
AssertIsIdentifiable(resource);
9698

9799
if (Property.SetMethod == null)
98100
{
@@ -107,15 +109,25 @@ public void SetValue(object resource, object? newValue)
107109
{
108110
throw new InvalidOperationException(
109111
$"Unable to set property value of '{Property.DeclaringType!.Name}.{Property.Name}' on instance of type '{resource.GetType().Name}'.",
110-
exception);
112+
exception.InnerException ?? exception);
111113
}
112114
}
113115

116+
protected void AssertIsIdentifiable(object? resource)
117+
{
118+
if (resource != null && resource is not IIdentifiable)
119+
{
120+
throw new InvalidOperationException($"Resource of type '{resource.GetType()}' does not implement {nameof(IIdentifiable)}.");
121+
}
122+
}
123+
124+
/// <inheritdoc />
114125
public override string? ToString()
115126
{
116127
return _publicName ?? (_property != null ? _property.Name : base.ToString());
117128
}
118129

130+
/// <inheritdoc />
119131
public override bool Equals(object? obj)
120132
{
121133
if (ReferenceEquals(this, obj))
@@ -133,6 +145,7 @@ public override bool Equals(object? obj)
133145
return _publicName == other._publicName && _property == other._property;
134146
}
135147

148+
/// <inheritdoc />
136149
public override int GetHashCode()
137150
{
138151
return HashCode.Combine(_publicName, _property);

‎test/JsonApiDotNetCoreTests/UnitTests/ResourceGraph/HasManyAttributeTests.cs‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
using FluentAssertions;
2+
using JsonApiDotNetCore.Resources;
3+
using JsonApiDotNetCore.Resources.Annotations;
4+
using TestBuildingBlocks;
5+
using Xunit;
6+
7+
namespace JsonApiDotNetCoreTests.UnitTests.ResourceGraph;
8+
9+
public sealed class HasManyAttributeTests
10+
{
11+
[Fact]
12+
public void Cannot_set_value_to_null()
13+
{
14+
// Arrange
15+
var attribute = new HasManyAttribute
16+
{
17+
Property = typeof(TestResource).GetProperty(nameof(TestResource.Children))!
18+
};
19+
20+
var resource = new TestResource();
21+
22+
// Act
23+
Action action = () => attribute.SetValue(resource, null);
24+
25+
// Assert
26+
action.Should().ThrowExactly<ArgumentNullException>();
27+
}
28+
29+
[Fact]
30+
public void Cannot_set_value_to_primitive_type()
31+
{
32+
// Arrange
33+
var attribute = new HasManyAttribute
34+
{
35+
Property = typeof(TestResource).GetProperty(nameof(TestResource.Children))!
36+
};
37+
38+
var resource = new TestResource();
39+
40+
// Act
41+
Action action = () => attribute.SetValue(resource, 1);
42+
43+
// Assert
44+
action.Should().ThrowExactly<InvalidOperationException>().WithMessage("Resource of type 'System.Int32' must be a collection.");
45+
}
46+
47+
[Fact]
48+
public void Cannot_set_value_to_single_resource()
49+
{
50+
// Arrange
51+
var attribute = new HasManyAttribute
52+
{
53+
Property = typeof(TestResource).GetProperty(nameof(TestResource.Children))!
54+
};
55+
56+
var resource = new TestResource();
57+
58+
// Act
59+
Action action = () => attribute.SetValue(resource, resource);
60+
61+
// Assert
62+
action.Should().ThrowExactly<InvalidOperationException>().WithMessage($"Resource of type '{typeof(TestResource).FullName}' must be a collection.");
63+
}
64+
65+
[Fact]
66+
public void Can_set_value_to_collection_with_single_resource()
67+
{
68+
// Arrange
69+
var attribute = new HasManyAttribute
70+
{
71+
Property = typeof(TestResource).GetProperty(nameof(TestResource.Children))!
72+
};
73+
74+
var resource = new TestResource();
75+
76+
var children = new List<TestResource>
77+
{
78+
resource
79+
};
80+
81+
// Act
82+
attribute.SetValue(resource, children);
83+
84+
// Assert
85+
attribute.GetValue(resource).Should().BeOfType<List<TestResource>>().Subject.ShouldHaveCount(1);
86+
}
87+
88+
[Fact]
89+
public void Cannot_set_value_to_collection_with_null_element()
90+
{
91+
// Arrange
92+
var attribute = new HasManyAttribute
93+
{
94+
Property = typeof(TestResource).GetProperty(nameof(TestResource.Children))!
95+
};
96+
97+
var resource = new TestResource();
98+
99+
var children = new List<TestResource>
100+
{
101+
resource,
102+
null!
103+
};
104+
105+
// Act
106+
Action action = () => attribute.SetValue(resource, children);
107+
108+
// Assert
109+
action.Should().ThrowExactly<InvalidOperationException>().WithMessage("Resource collection must not contain null values.");
110+
}
111+
112+
[Fact]
113+
public void Cannot_set_value_to_collection_with_primitive_element()
114+
{
115+
// Arrange
116+
var attribute = new HasManyAttribute
117+
{
118+
Property = typeof(TestResource).GetProperty(nameof(TestResource.Children))!
119+
};
120+
121+
var resource = new TestResource();
122+
123+
var children = new List<object>
124+
{
125+
resource,
126+
1
127+
};
128+
129+
// Act
130+
Action action = () => attribute.SetValue(resource, children);
131+
132+
// Assert
133+
action.Should().ThrowExactly<InvalidOperationException>().WithMessage("Resource of type 'System.Int32' does not implement IIdentifiable.");
134+
}
135+
136+
private sealed class TestResource : Identifiable<long>
137+
{
138+
[HasMany]
139+
public IEnumerable<TestResource> Children { get; set; } = new HashSet<TestResource>();
140+
}
141+
}

‎test/JsonApiDotNetCoreTests/UnitTests/ResourceGraph/HasOneAttributeTests.cs‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
using FluentAssertions;
2+
using JsonApiDotNetCore.Resources;
3+
using JsonApiDotNetCore.Resources.Annotations;
4+
using Xunit;
5+
6+
namespace JsonApiDotNetCoreTests.UnitTests.ResourceGraph;
7+
8+
public sealed class HasOneAttributeTests
9+
{
10+
[Fact]
11+
public void Can_set_value_to_null()
12+
{
13+
// Arrange
14+
var attribute = new HasOneAttribute
15+
{
16+
Property = typeof(TestResource).GetProperty(nameof(TestResource.Parent))!
17+
};
18+
19+
var resource = new TestResource();
20+
21+
// Act
22+
attribute.SetValue(resource, null);
23+
24+
// Assert
25+
attribute.GetValue(resource).Should().BeNull();
26+
}
27+
28+
[Fact]
29+
public void Can_set_value_to_self()
30+
{
31+
// Arrange
32+
var attribute = new HasOneAttribute
33+
{
34+
Property = typeof(TestResource).GetProperty(nameof(TestResource.Parent))!
35+
};
36+
37+
var resource = new TestResource();
38+
39+
// Act
40+
attribute.SetValue(resource, resource);
41+
42+
// Assert
43+
attribute.GetValue(resource).Should().Be(resource);
44+
}
45+
46+
[Fact]
47+
public void Cannot_set_value_to_primitive_type()
48+
{
49+
// Arrange
50+
var attribute = new HasOneAttribute
51+
{
52+
Property = typeof(TestResource).GetProperty(nameof(TestResource.Parent))!
53+
};
54+
55+
var resource = new TestResource();
56+
57+
// Act
58+
Action action = () => attribute.SetValue(resource, 1);
59+
60+
// Assert
61+
action.Should().ThrowExactly<InvalidOperationException>().WithMessage("Resource of type 'System.Int32' does not implement IIdentifiable.");
62+
}
63+
64+
private sealed class TestResource : Identifiable<long>
65+
{
66+
[HasOne]
67+
public TestResource? Parent { get; set; }
68+
}
69+
}

0 commit comments

Comments
 (0)