Skip to content

Commit a1dcf87

Browse files
author
Bart Koelman
committed
Fixed: handle nulls in request body
1 parent 1a57ea0 commit a1dcf87

29 files changed

+501
-70
lines changed

‎src/JsonApiDotNetCore/Serialization/JsonConverters/SingleOrManyDataConverterFactory.cs

-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ public override SingleOrManyData<T> Read(ref Utf8JsonReader reader, Type typeToC
5050
{
5151
if (isManyData)
5252
{
53-
// [TODO-NRT]: Add tests for downstream failure on `null` array element.
5453
objects.Add(new T());
5554
}
5655

‎src/JsonApiDotNetCore/Serialization/Request/Adapters/BaseDataAdapter.cs renamed to ‎src/JsonApiDotNetCore/Serialization/Request/Adapters/BaseAdapter.cs

+21-11
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
using JetBrains.Annotations;
22
using JsonApiDotNetCore.Serialization.Objects;
3+
using SysNotNull = System.Diagnostics.CodeAnalysis.NotNullAttribute;
34

45
namespace JsonApiDotNetCore.Serialization.Request.Adapters
56
{
67
/// <summary>
78
/// Contains shared assertions for derived types.
89
/// </summary>
9-
public abstract class BaseDataAdapter
10+
public abstract class BaseAdapter
1011
{
1112
[AssertionMethod]
1213
protected static void AssertHasData<T>(SingleOrManyData<T> data, RequestAdapterState state)
@@ -19,36 +20,45 @@ protected static void AssertHasData<T>(SingleOrManyData<T> data, RequestAdapterS
1920
}
2021

2122
[AssertionMethod]
22-
protected static void AssertHasSingleValue<T>(SingleOrManyData<T> data, bool allowNull, RequestAdapterState state)
23+
protected static void AssertDataHasSingleValue<T>(SingleOrManyData<T> data, bool allowNull, RequestAdapterState state)
2324
where T : class, IResourceIdentity, new()
2425
{
2526
if (data.SingleValue == null)
2627
{
2728
if (!allowNull)
2829
{
29-
throw new ModelConversionException(state.Position,
30-
data.ManyValue == null
31-
? "Expected an object in 'data' element, instead of 'null'."
32-
: "Expected an object in 'data' element, instead of an array.", null);
30+
if (data.ManyValue == null)
31+
{
32+
AssertObjectIsNotNull(data.SingleValue, state);
33+
}
34+
35+
throw new ModelConversionException(state.Position, "Expected an object, instead of an array.", null);
3336
}
3437

3538
if (data.ManyValue != null)
3639
{
37-
throw new ModelConversionException(state.Position, "Expected an object or 'null' in 'data' element, instead of an array.", null);
40+
throw new ModelConversionException(state.Position, "Expected an object or 'null', instead of an array.", null);
3841
}
3942
}
4043
}
4144

4245
[AssertionMethod]
43-
protected static void AssertHasManyValue<T>(SingleOrManyData<T> data, RequestAdapterState state)
46+
protected static void AssertDataHasManyValue<T>(SingleOrManyData<T> data, RequestAdapterState state)
4447
where T : class, IResourceIdentity, new()
4548
{
4649
if (data.ManyValue == null)
4750
{
4851
throw new ModelConversionException(state.Position,
49-
data.SingleValue == null
50-
? "Expected an array in 'data' element, instead of 'null'."
51-
: "Expected an array in 'data' element, instead of an object.", null);
52+
data.SingleValue == null ? "Expected an array, instead of 'null'." : "Expected an array, instead of an object.", null);
53+
}
54+
}
55+
56+
protected static void AssertObjectIsNotNull<T>([SysNotNull] T? value, RequestAdapterState state)
57+
where T : class
58+
{
59+
if (value is null)
60+
{
61+
throw new ModelConversionException(state.Position, "Expected an object, instead of 'null'.", null);
5262
}
5363
}
5464
}

‎src/JsonApiDotNetCore/Serialization/Request/Adapters/DocumentInOperationsRequestAdapter.cs

+3-8
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77

88
namespace JsonApiDotNetCore.Serialization.Request.Adapters
99
{
10-
/// <inheritdoc />
11-
public sealed class DocumentInOperationsRequestAdapter : IDocumentInOperationsRequestAdapter
10+
/// <inheritdoc cref="IDocumentInOperationsRequestAdapter" />
11+
public sealed class DocumentInOperationsRequestAdapter : BaseAdapter, IDocumentInOperationsRequestAdapter
1212
{
1313
private readonly IJsonApiOptions _options;
1414
private readonly IAtomicOperationObjectAdapter _atomicOperationObjectAdapter;
@@ -59,13 +59,8 @@ private IList<OperationContainer> ConvertOperations(IEnumerable<AtomicOperationO
5959

6060
foreach (AtomicOperationObject? atomicOperationObject in atomicOperationObjects)
6161
{
62-
// [TODO-NRT]: Add tests for downstream failure on `null` object.
63-
if (atomicOperationObject == null)
64-
{
65-
throw new ModelConversionException(state.Position, null, null);
66-
}
67-
6862
using IDisposable _ = state.Position.PushArrayIndex(operationIndex);
63+
AssertObjectIsNotNull(atomicOperationObject, state);
6964

7065
OperationContainer operation = _atomicOperationObjectAdapter.Convert(atomicOperationObject, state);
7166
operations.Add(operation);

‎src/JsonApiDotNetCore/Serialization/Request/Adapters/RelationshipDataAdapter.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
namespace JsonApiDotNetCore.Serialization.Request.Adapters
1010
{
1111
/// <inheritdoc cref="IRelationshipDataAdapter" />
12-
public sealed class RelationshipDataAdapter : BaseDataAdapter, IRelationshipDataAdapter
12+
public sealed class RelationshipDataAdapter : BaseAdapter, IRelationshipDataAdapter
1313
{
1414
private static readonly CollectionConverter CollectionConverter = new();
1515

@@ -85,15 +85,15 @@ private static SingleOrManyData<ResourceIdentifierObject> ToIdentifierData(Singl
8585
private IIdentifiable? ConvertToOneRelationshipData(SingleOrManyData<ResourceIdentifierObject> data, ResourceIdentityRequirements requirements,
8686
RequestAdapterState state)
8787
{
88-
AssertHasSingleValue(data, true, state);
88+
AssertDataHasSingleValue(data, true, state);
8989

9090
return data.SingleValue != null ? _resourceIdentifierObjectAdapter.Convert(data.SingleValue, requirements, state) : null;
9191
}
9292

9393
private IEnumerable ConvertToManyRelationshipData(SingleOrManyData<ResourceIdentifierObject> data, RelationshipAttribute relationship,
9494
ResourceIdentityRequirements requirements, bool useToManyElementType, RequestAdapterState state)
9595
{
96-
AssertHasManyValue(data, state);
96+
AssertDataHasManyValue(data, state);
9797

9898
int arrayIndex = 0;
9999
var rightResources = new List<IIdentifiable>();

‎src/JsonApiDotNetCore/Serialization/Request/Adapters/ResourceDataAdapter.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
namespace JsonApiDotNetCore.Serialization.Request.Adapters
77
{
88
/// <inheritdoc cref="IResourceDataAdapter" />
9-
public class ResourceDataAdapter : BaseDataAdapter, IResourceDataAdapter
9+
public class ResourceDataAdapter : BaseAdapter, IResourceDataAdapter
1010
{
1111
private readonly IResourceDefinitionAccessor _resourceDefinitionAccessor;
1212
private readonly IResourceObjectAdapter _resourceObjectAdapter;
@@ -29,7 +29,7 @@ public IIdentifiable Convert(SingleOrManyData<ResourceObject> data, ResourceIden
2929
AssertHasData(data, state);
3030

3131
using IDisposable _ = state.Position.PushElement("data");
32-
AssertHasSingleValue(data, false, state);
32+
AssertDataHasSingleValue(data, false, state);
3333

3434
(IIdentifiable resource, ResourceType _) = ConvertResourceObject(data, requirements, state);
3535

‎src/JsonApiDotNetCore/Serialization/Request/Adapters/ResourceIdentityAdapter.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ namespace JsonApiDotNetCore.Serialization.Request.Adapters
1212
/// <summary>
1313
/// Base class for validating and converting objects that represent an identity.
1414
/// </summary>
15-
public abstract class ResourceIdentityAdapter
15+
public abstract class ResourceIdentityAdapter : BaseAdapter
1616
{
1717
private readonly IResourceGraph _resourceGraph;
1818
private readonly IResourceFactory _resourceFactory;

‎src/JsonApiDotNetCore/Serialization/Request/Adapters/ResourceObjectAdapter.cs

+6-5
Original file line numberDiff line numberDiff line change
@@ -133,15 +133,16 @@ private void ConvertRelationships(IDictionary<string, RelationshipObject?>? reso
133133

134134
foreach ((string relationshipName, RelationshipObject? relationshipObject) in resourceObjectRelationships.EmptyIfNull())
135135
{
136-
// [TODO-NRT]: Add tests for downstream failure on `null` relationship value.
137-
ConvertRelationship(relationshipName, relationshipObject?.Data ?? default, resource, resourceType, state);
136+
ConvertRelationship(relationshipName, relationshipObject, resource, resourceType, state);
138137
}
139138
}
140139

141-
private void ConvertRelationship(string relationshipName, SingleOrManyData<ResourceIdentifierObject> relationshipData, IIdentifiable resource,
142-
ResourceType resourceType, RequestAdapterState state)
140+
private void ConvertRelationship(string relationshipName, RelationshipObject? relationshipObject, IIdentifiable resource, ResourceType resourceType,
141+
RequestAdapterState state)
143142
{
144143
using IDisposable _ = state.Position.PushElement(relationshipName);
144+
AssertObjectIsNotNull(relationshipObject, state);
145+
145146
RelationshipAttribute? relationship = resourceType.FindRelationshipByPublicName(relationshipName);
146147

147148
if (relationship == null && _options.AllowUnknownFieldsInRequestBody)
@@ -151,7 +152,7 @@ private void ConvertRelationship(string relationshipName, SingleOrManyData<Resou
151152

152153
AssertIsKnownRelationship(relationship, relationshipName, resourceType, state);
153154

154-
object? rightValue = _relationshipDataAdapter.Convert(relationshipData, relationship, true, state);
155+
object? rightValue = _relationshipDataAdapter.Convert(relationshipObject.Data, relationship, true, state);
155156

156157
relationship.SetValue(resource, rightValue);
157158
state.WritableTargetedFields!.Relationships.Add(relationship);

‎src/JsonApiDotNetCore/Serialization/Request/JsonApiReader.cs

+11-5
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
using Microsoft.AspNetCore.Http.Extensions;
1515
using Microsoft.AspNetCore.WebUtilities;
1616
using Microsoft.Extensions.Logging;
17+
using SysNotNull = System.Diagnostics.CodeAnalysis.NotNullAttribute;
1718

1819
namespace JsonApiDotNetCore.Serialization.Request
1920
{
@@ -83,11 +84,7 @@ private Document DeserializeDocument(string requestBody)
8384

8485
var document = JsonSerializer.Deserialize<Document>(requestBody, _options.SerializerReadOptions);
8586

86-
if (document == null)
87-
{
88-
// [TODO-NRT]: Add tests for incoming body `null`.
89-
throw new InvalidRequestBodyException(_options.IncludeRequestBodyInErrors ? requestBody : null, null, null, null);
90-
}
87+
AssertHasDocument(document, requestBody);
9188

9289
return document;
9390
}
@@ -100,6 +97,15 @@ private Document DeserializeDocument(string requestBody)
10097
}
10198
}
10299

100+
private void AssertHasDocument([SysNotNull] Document? document, string requestBody)
101+
{
102+
if (document == null)
103+
{
104+
throw new InvalidRequestBodyException(_options.IncludeRequestBodyInErrors ? requestBody : null, "Expected an object, instead of 'null'.", null,
105+
null);
106+
}
107+
}
108+
103109
private object? ConvertDocumentToModel(Document document, string requestBody)
104110
{
105111
try

‎test/JsonApiDotNetCoreTests/IntegrationTests/AtomicOperations/Creating/AtomicCreateResourceTests.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -626,7 +626,7 @@ public async Task Cannot_create_resource_for_null_data()
626626

627627
ErrorObject error = responseDocument.Errors[0];
628628
error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
629-
error.Title.Should().Be("Failed to deserialize request body: Expected an object in 'data' element, instead of 'null'.");
629+
error.Title.Should().Be("Failed to deserialize request body: Expected an object, instead of 'null'.");
630630
error.Detail.Should().BeNull();
631631
error.Source.ShouldNotBeNull();
632632
error.Source.Pointer.Should().Be("/atomic:operations[0]/data");
@@ -673,7 +673,7 @@ public async Task Cannot_create_resource_for_array_data()
673673

674674
ErrorObject error = responseDocument.Errors[0];
675675
error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
676-
error.Title.Should().Be("Failed to deserialize request body: Expected an object in 'data' element, instead of an array.");
676+
error.Title.Should().Be("Failed to deserialize request body: Expected an object, instead of an array.");
677677
error.Detail.Should().BeNull();
678678
error.Source.ShouldNotBeNull();
679679
error.Source.Pointer.Should().Be("/atomic:operations[0]/data");

‎test/JsonApiDotNetCoreTests/IntegrationTests/AtomicOperations/Creating/AtomicCreateResourceWithToManyRelationshipTests.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,7 @@ public async Task Cannot_create_with_null_data_in_ManyToMany_relationship()
635635

636636
ErrorObject error = responseDocument.Errors[0];
637637
error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
638-
error.Title.Should().Be("Failed to deserialize request body: Expected an array in 'data' element, instead of 'null'.");
638+
error.Title.Should().Be("Failed to deserialize request body: Expected an array, instead of 'null'.");
639639
error.Detail.Should().BeNull();
640640
error.Source.ShouldNotBeNull();
641641
error.Source.Pointer.Should().Be("/atomic:operations[0]/data/relationships/tracks/data");
@@ -682,7 +682,7 @@ public async Task Cannot_create_with_object_data_in_ManyToMany_relationship()
682682

683683
ErrorObject error = responseDocument.Errors[0];
684684
error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
685-
error.Title.Should().Be("Failed to deserialize request body: Expected an array in 'data' element, instead of an object.");
685+
error.Title.Should().Be("Failed to deserialize request body: Expected an array, instead of an object.");
686686
error.Detail.Should().BeNull();
687687
error.Source.ShouldNotBeNull();
688688
error.Source.Pointer.Should().Be("/atomic:operations[0]/data/relationships/tracks/data");

‎test/JsonApiDotNetCoreTests/IntegrationTests/AtomicOperations/Creating/AtomicCreateResourceWithToOneRelationshipTests.cs

+43-1
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,48 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
270270
});
271271
}
272272

273+
[Fact]
274+
public async Task Cannot_create_for_null_relationship()
275+
{
276+
// Arrange
277+
var requestBody = new
278+
{
279+
atomic__operations = new[]
280+
{
281+
new
282+
{
283+
op = "add",
284+
data = new
285+
{
286+
type = "musicTracks",
287+
relationships = new
288+
{
289+
lyric = (object?)null
290+
}
291+
}
292+
}
293+
}
294+
};
295+
296+
const string route = "/operations";
297+
298+
// Act
299+
(HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecutePostAtomicAsync<Document>(route, requestBody);
300+
301+
// Assert
302+
httpResponse.Should().HaveStatusCode(HttpStatusCode.UnprocessableEntity);
303+
304+
responseDocument.Errors.ShouldHaveCount(1);
305+
306+
ErrorObject error = responseDocument.Errors[0];
307+
error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
308+
error.Title.Should().Be("Failed to deserialize request body: Expected an object, instead of 'null'.");
309+
error.Detail.Should().BeNull();
310+
error.Source.ShouldNotBeNull();
311+
error.Source.Pointer.Should().Be("/atomic:operations[0]/data/relationships/lyric");
312+
error.Meta.ShouldContainKey("requestBody").With(value => value.ShouldNotBeNull().ToString().ShouldNotBeEmpty());
313+
}
314+
273315
[Fact]
274316
public async Task Cannot_create_for_missing_data_in_relationship()
275317
{
@@ -359,7 +401,7 @@ public async Task Cannot_create_for_array_data_in_relationship()
359401

360402
ErrorObject error = responseDocument.Errors[0];
361403
error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
362-
error.Title.Should().Be("Failed to deserialize request body: Expected an object or 'null' in 'data' element, instead of an array.");
404+
error.Title.Should().Be("Failed to deserialize request body: Expected an object or 'null', instead of an array.");
363405
error.Detail.Should().BeNull();
364406
error.Source.ShouldNotBeNull();
365407
error.Source.Pointer.Should().Be("/atomic:operations[0]/data/relationships/lyric/data");

‎test/JsonApiDotNetCoreTests/IntegrationTests/AtomicOperations/Mixed/AtomicRequestBodyTests.cs

+55
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,30 @@ public async Task Cannot_process_for_missing_request_body()
4242
error.Meta.Should().NotContainKey("requestBody");
4343
}
4444

45+
[Fact]
46+
public async Task Cannot_process_for_null_request_body()
47+
{
48+
// Arrange
49+
const string requestBody = "null";
50+
51+
const string route = "/operations";
52+
53+
// Act
54+
(HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecutePostAtomicAsync<Document>(route, requestBody);
55+
56+
// Assert
57+
httpResponse.Should().HaveStatusCode(HttpStatusCode.UnprocessableEntity);
58+
59+
responseDocument.Errors.ShouldHaveCount(1);
60+
61+
ErrorObject error = responseDocument.Errors[0];
62+
error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
63+
error.Title.Should().Be("Failed to deserialize request body: Expected an object, instead of 'null'.");
64+
error.Detail.Should().BeNull();
65+
error.Source.Should().BeNull();
66+
error.Meta.ShouldContainKey("requestBody").With(value => value.ShouldNotBeNull().ToString().ShouldNotBeEmpty());
67+
}
68+
4569
[Fact]
4670
public async Task Cannot_process_for_broken_JSON_request_body()
4771
{
@@ -122,6 +146,37 @@ public async Task Cannot_process_empty_operations_array()
122146
error.Meta.ShouldContainKey("requestBody").With(value => value.ShouldNotBeNull().ToString().ShouldNotBeEmpty());
123147
}
124148

149+
[Fact]
150+
public async Task Cannot_process_null_operation()
151+
{
152+
// Arrange
153+
var requestBody = new
154+
{
155+
atomic__operations = new[]
156+
{
157+
(object?)null
158+
}
159+
};
160+
161+
const string route = "/operations";
162+
163+
// Act
164+
(HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecutePostAtomicAsync<Document>(route, requestBody);
165+
166+
// Assert
167+
httpResponse.Should().HaveStatusCode(HttpStatusCode.UnprocessableEntity);
168+
169+
responseDocument.Errors.ShouldHaveCount(1);
170+
171+
ErrorObject error = responseDocument.Errors[0];
172+
error.StatusCode.Should().Be(HttpStatusCode.UnprocessableEntity);
173+
error.Title.Should().Be("Failed to deserialize request body: Expected an object, instead of 'null'.");
174+
error.Detail.Should().BeNull();
175+
error.Source.ShouldNotBeNull();
176+
error.Source.Pointer.Should().Be("/atomic:operations[0]");
177+
error.Meta.ShouldContainKey("requestBody").With(value => value.ShouldNotBeNull().ToString().ShouldNotBeEmpty());
178+
}
179+
125180
[Fact]
126181
public async Task Cannot_process_for_unknown_operation_code()
127182
{

0 commit comments

Comments
 (0)