Skip to content

Commit 37c03ec

Browse files
bkoelmanbkoelman
committed
Return better error for whitespace relationship name in request URL
1 parent 2cd4190 commit 37c03ec

File tree

10 files changed

+263
-27
lines changed

10 files changed

+263
-27
lines changed

‎src/JsonApiDotNetCore/Controllers/BaseJsonApiController.cs‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -134,15 +134,16 @@ public virtual async Task<IActionResult> GetAsync([DisallowNull] TId id, Cancell
134134
/// GET /articles/1/revisions HTTP/1.1
135135
/// ]]></code>
136136
/// </summary>
137-
public virtual async Task<IActionResult> GetSecondaryAsync([DisallowNull] TId id, string relationshipName, CancellationToken cancellationToken)
137+
public virtual async Task<IActionResult> GetSecondaryAsync([DisallowNull] TId id, [PreserveEmptyString] string relationshipName,
138+
CancellationToken cancellationToken)
138139
{
139140
_traceWriter.LogMethodStart(new
140141
{
141142
id,
142143
relationshipName
143144
});
144145

145-
ArgumentGuard.NotNullNorEmpty(relationshipName);
146+
ArgumentGuard.NotNull(relationshipName);
146147

147148
if (_getSecondary == null)
148149
{
@@ -163,15 +164,16 @@ public virtual async Task<IActionResult> GetSecondaryAsync([DisallowNull] TId id
163164
/// GET /articles/1/relationships/revisions HTTP/1.1
164165
/// ]]></code>
165166
/// </summary>
166-
public virtual async Task<IActionResult> GetRelationshipAsync([DisallowNull] TId id, string relationshipName, CancellationToken cancellationToken)
167+
public virtual async Task<IActionResult> GetRelationshipAsync([DisallowNull] TId id, [PreserveEmptyString] string relationshipName,
168+
CancellationToken cancellationToken)
167169
{
168170
_traceWriter.LogMethodStart(new
169171
{
170172
id,
171173
relationshipName
172174
});
173175

174-
ArgumentGuard.NotNullNorEmpty(relationshipName);
176+
ArgumentGuard.NotNull(relationshipName);
175177

176178
if (_getRelationship == null)
177179
{
@@ -247,7 +249,7 @@ private string GetLocationUrl(string resourceId)
247249
/// <param name="cancellationToken">
248250
/// Propagates notification that request handling should be canceled.
249251
/// </param>
250-
public virtual async Task<IActionResult> PostRelationshipAsync([DisallowNull] TId id, string relationshipName,
252+
public virtual async Task<IActionResult> PostRelationshipAsync([DisallowNull] TId id, [PreserveEmptyString] string relationshipName,
251253
[FromBody] ISet<IIdentifiable> rightResourceIds, CancellationToken cancellationToken)
252254
{
253255
_traceWriter.LogMethodStart(new
@@ -257,7 +259,7 @@ public virtual async Task<IActionResult> PostRelationshipAsync([DisallowNull] TI
257259
rightResourceIds
258260
});
259261

260-
ArgumentGuard.NotNullNorEmpty(relationshipName);
262+
ArgumentGuard.NotNull(relationshipName);
261263
ArgumentGuard.NotNull(rightResourceIds);
262264

263265
if (_addToRelationship == null)
@@ -322,8 +324,8 @@ public virtual async Task<IActionResult> PatchAsync([DisallowNull] TId id, [From
322324
/// <param name="cancellationToken">
323325
/// Propagates notification that request handling should be canceled.
324326
/// </param>
325-
public virtual async Task<IActionResult> PatchRelationshipAsync([DisallowNull] TId id, string relationshipName, [FromBody] object? rightValue,
326-
CancellationToken cancellationToken)
327+
public virtual async Task<IActionResult> PatchRelationshipAsync([DisallowNull] TId id, [PreserveEmptyString] string relationshipName,
328+
[FromBody] object? rightValue, CancellationToken cancellationToken)
327329
{
328330
_traceWriter.LogMethodStart(new
329331
{
@@ -332,7 +334,7 @@ public virtual async Task<IActionResult> PatchRelationshipAsync([DisallowNull] T
332334
rightValue
333335
});
334336

335-
ArgumentGuard.NotNullNorEmpty(relationshipName);
337+
ArgumentGuard.NotNull(relationshipName);
336338

337339
if (_setRelationship == null)
338340
{
@@ -383,7 +385,7 @@ public virtual async Task<IActionResult> DeleteAsync([DisallowNull] TId id, Canc
383385
/// <param name="cancellationToken">
384386
/// Propagates notification that request handling should be canceled.
385387
/// </param>
386-
public virtual async Task<IActionResult> DeleteRelationshipAsync([DisallowNull] TId id, string relationshipName,
388+
public virtual async Task<IActionResult> DeleteRelationshipAsync([DisallowNull] TId id, [PreserveEmptyString] string relationshipName,
387389
[FromBody] ISet<IIdentifiable> rightResourceIds, CancellationToken cancellationToken)
388390
{
389391
_traceWriter.LogMethodStart(new
@@ -393,7 +395,7 @@ public virtual async Task<IActionResult> DeleteRelationshipAsync([DisallowNull]
393395
rightResourceIds
394396
});
395397

396-
ArgumentGuard.NotNullNorEmpty(relationshipName);
398+
ArgumentGuard.NotNull(relationshipName);
397399
ArgumentGuard.NotNull(rightResourceIds);
398400

399401
if (_removeFromRelationship == null)

‎src/JsonApiDotNetCore/Controllers/PreserveEmptyStringAttribute.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
using System.ComponentModel.DataAnnotations;
2+
using JetBrains.Annotations;
3+
4+
namespace JsonApiDotNetCore.Controllers;
5+
6+
[PublicAPI]
7+
[AttributeUsage(AttributeTargets.Parameter)]
8+
public sealed class PreserveEmptyStringAttribute : DisplayFormatAttribute
9+
{
10+
public PreserveEmptyStringAttribute()
11+
{
12+
// Workaround for https://github.com/dotnet/aspnetcore/issues/29948#issuecomment-1898216682
13+
ConvertEmptyStringToNull = false;
14+
}
15+
}

‎src/JsonApiDotNetCore/Services/JsonApiResourceService.cs‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ public virtual async Task<TResource> GetAsync([DisallowNull] TId id, Cancellatio
109109

110110
using IDisposable _ = CodeTimingSessionManager.Current.Measure("Service - Get secondary resource(s)");
111111

112+
ArgumentGuard.NotNull(relationshipName);
112113
AssertPrimaryResourceTypeInJsonApiRequestIsNotNull(_request.PrimaryResourceType);
113114
AssertHasRelationship(_request.Relationship, relationshipName);
114115

@@ -146,10 +147,9 @@ public virtual async Task<TResource> GetAsync([DisallowNull] TId id, Cancellatio
146147
relationshipName
147148
});
148149

149-
ArgumentGuard.NotNullNorEmpty(relationshipName);
150-
151150
using IDisposable _ = CodeTimingSessionManager.Current.Measure("Service - Get relationship");
152151

152+
ArgumentGuard.NotNull(relationshipName);
153153
AssertPrimaryResourceTypeInJsonApiRequestIsNotNull(_request.PrimaryResourceType);
154154
AssertHasRelationship(_request.Relationship, relationshipName);
155155

@@ -350,11 +350,10 @@ public virtual async Task AddToToManyRelationshipAsync([DisallowNull] TId leftId
350350
rightResourceIds
351351
});
352352

353-
ArgumentGuard.NotNullNorEmpty(relationshipName);
354-
ArgumentGuard.NotNull(rightResourceIds);
355-
356353
using IDisposable _ = CodeTimingSessionManager.Current.Measure("Service - Add to to-many relationship");
357354

355+
ArgumentGuard.NotNull(relationshipName);
356+
ArgumentGuard.NotNull(rightResourceIds);
358357
AssertHasRelationship(_request.Relationship, relationshipName);
359358

360359
TResource? resourceFromDatabase = null;
@@ -501,10 +500,9 @@ public virtual async Task SetRelationshipAsync([DisallowNull] TId leftId, string
501500
rightValue
502501
});
503502

504-
ArgumentGuard.NotNullNorEmpty(relationshipName);
505-
506503
using IDisposable _ = CodeTimingSessionManager.Current.Measure("Service - Set relationship");
507504

505+
ArgumentGuard.NotNull(relationshipName);
508506
AssertHasRelationship(_request.Relationship, relationshipName);
509507

510508
object? effectiveRightValue = _request.Relationship.RightType.IsPartOfTypeHierarchy()
@@ -573,11 +571,10 @@ public virtual async Task RemoveFromToManyRelationshipAsync([DisallowNull] TId l
573571
rightResourceIds
574572
});
575573

576-
ArgumentGuard.NotNullNorEmpty(relationshipName);
577-
ArgumentGuard.NotNull(rightResourceIds);
578-
579574
using IDisposable _ = CodeTimingSessionManager.Current.Measure("Repository - Remove from to-many relationship");
580575

576+
ArgumentGuard.NotNull(relationshipName);
577+
ArgumentGuard.NotNull(rightResourceIds);
581578
AssertHasRelationship(_request.Relationship, relationshipName);
582579
var hasManyRelationship = (HasManyAttribute)_request.Relationship;
583580

‎test/JsonApiDotNetCoreTests/IntegrationTests/IdObfuscation/ObfuscatedIdentifiableController.cs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,16 @@ public Task<IActionResult> GetAsync([Required] string id, CancellationToken canc
3131
}
3232

3333
[HttpGet("{id}/{relationshipName}")]
34-
public Task<IActionResult> GetSecondaryAsync([Required] string id, [Required] string relationshipName, CancellationToken cancellationToken)
34+
public Task<IActionResult> GetSecondaryAsync([Required] string id, [Required] [PreserveEmptyString] string relationshipName,
35+
CancellationToken cancellationToken)
3536
{
3637
int idValue = _codec.Decode(id);
3738
return base.GetSecondaryAsync(idValue, relationshipName, cancellationToken);
3839
}
3940

4041
[HttpGet("{id}/relationships/{relationshipName}")]
41-
public Task<IActionResult> GetRelationshipAsync([Required] string id, [Required] string relationshipName, CancellationToken cancellationToken)
42+
public Task<IActionResult> GetRelationshipAsync([Required] string id, [Required] [PreserveEmptyString] string relationshipName,
43+
CancellationToken cancellationToken)
4244
{
4345
int idValue = _codec.Decode(id);
4446
return base.GetRelationshipAsync(idValue, relationshipName, cancellationToken);
@@ -51,7 +53,7 @@ public override Task<IActionResult> PostAsync([FromBody] [Required] TResource re
5153
}
5254

5355
[HttpPost("{id}/relationships/{relationshipName}")]
54-
public Task<IActionResult> PostRelationshipAsync([Required] string id, [Required] string relationshipName,
56+
public Task<IActionResult> PostRelationshipAsync([Required] string id, [Required] [PreserveEmptyString] string relationshipName,
5557
[FromBody] [Required] ISet<IIdentifiable> rightResourceIds, CancellationToken cancellationToken)
5658
{
5759
int idValue = _codec.Decode(id);
@@ -67,8 +69,8 @@ public Task<IActionResult> PatchAsync([Required] string id, [FromBody] [Required
6769

6870
[HttpPatch("{id}/relationships/{relationshipName}")]
6971
// Parameter `[Required] object? rightValue` makes Swashbuckle generate the OpenAPI request body as required. We don't actually validate ModelState, so it doesn't hurt.
70-
public Task<IActionResult> PatchRelationshipAsync([Required] string id, [Required] string relationshipName, [FromBody] [Required] object? rightValue,
71-
CancellationToken cancellationToken)
72+
public Task<IActionResult> PatchRelationshipAsync([Required] string id, [Required] [PreserveEmptyString] string relationshipName,
73+
[FromBody] [Required] object? rightValue, CancellationToken cancellationToken)
7274
{
7375
int idValue = _codec.Decode(id);
7476
return base.PatchRelationshipAsync(idValue, relationshipName, rightValue, cancellationToken);
@@ -82,7 +84,7 @@ public Task<IActionResult> DeleteAsync([Required] string id, CancellationToken c
8284
}
8385

8486
[HttpDelete("{id}/relationships/{relationshipName}")]
85-
public Task<IActionResult> DeleteRelationshipAsync([Required] string id, [Required] string relationshipName,
87+
public Task<IActionResult> DeleteRelationshipAsync([Required] string id, [Required] [PreserveEmptyString] string relationshipName,
8688
[FromBody] [Required] ISet<IIdentifiable> rightResourceIds, CancellationToken cancellationToken)
8789
{
8890
int idValue = _codec.Decode(id);

‎test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Fetching/FetchRelationshipTests.cs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,4 +243,31 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
243243
error.Title.Should().Be("The requested relationship does not exist.");
244244
error.Detail.Should().Be($"Resource of type 'workItems' does not contain a relationship named '{Unknown.Relationship}'.");
245245
}
246+
247+
[Fact]
248+
public async Task Cannot_get_relationship_for_whitespace_relationship_name()
249+
{
250+
WorkItem workItem = _fakers.WorkItem.GenerateOne();
251+
252+
await _testContext.RunOnDatabaseAsync(async dbContext =>
253+
{
254+
dbContext.WorkItems.Add(workItem);
255+
await dbContext.SaveChangesAsync();
256+
});
257+
258+
string route = $"/workItems/{workItem.StringId}/relationships/%20%20";
259+
260+
// Act
261+
(HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync<Document>(route);
262+
263+
// Assert
264+
httpResponse.ShouldHaveStatusCode(HttpStatusCode.NotFound);
265+
266+
responseDocument.Errors.ShouldHaveCount(1);
267+
268+
ErrorObject error = responseDocument.Errors[0];
269+
error.StatusCode.Should().Be(HttpStatusCode.NotFound);
270+
error.Title.Should().Be("The requested relationship does not exist.");
271+
error.Detail.Should().Be("Resource of type 'workItems' does not contain a relationship named ' '.");
272+
}
246273
}

‎test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Fetching/FetchResourceTests.cs‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,4 +376,32 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
376376
error.Title.Should().Be("The requested relationship does not exist.");
377377
error.Detail.Should().Be($"Resource of type 'workItems' does not contain a relationship named '{Unknown.Relationship}'.");
378378
}
379+
380+
[Fact]
381+
public async Task Cannot_get_secondary_resource_for_whitespace_relationship_name()
382+
{
383+
// Arrange
384+
WorkItem workItem = _fakers.WorkItem.GenerateOne();
385+
386+
await _testContext.RunOnDatabaseAsync(async dbContext =>
387+
{
388+
dbContext.WorkItems.Add(workItem);
389+
await dbContext.SaveChangesAsync();
390+
});
391+
392+
string route = $"/workItems/{workItem.StringId}/%20";
393+
394+
// Act
395+
(HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync<Document>(route);
396+
397+
// Assert
398+
httpResponse.ShouldHaveStatusCode(HttpStatusCode.NotFound);
399+
400+
responseDocument.Errors.ShouldHaveCount(1);
401+
402+
ErrorObject error = responseDocument.Errors[0];
403+
error.StatusCode.Should().Be(HttpStatusCode.NotFound);
404+
error.Title.Should().Be("The requested relationship does not exist.");
405+
error.Detail.Should().Be("Resource of type 'workItems' does not contain a relationship named ' '.");
406+
}
379407
}

‎test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Updating/Relationships/AddToToManyRelationshipTests.cs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -601,6 +601,48 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
601601
error.Meta.Should().NotContainKey("requestBody");
602602
}
603603

604+
[Fact]
605+
public async Task Cannot_add_to_whitespace_relationship_in_url()
606+
{
607+
// Arrange
608+
WorkItem existingWorkItem = _fakers.WorkItem.GenerateOne();
609+
610+
await _testContext.RunOnDatabaseAsync(async dbContext =>
611+
{
612+
dbContext.WorkItems.Add(existingWorkItem);
613+
await dbContext.SaveChangesAsync();
614+
});
615+
616+
var requestBody = new
617+
{
618+
data = new[]
619+
{
620+
new
621+
{
622+
type = "userAccounts",
623+
id = Unknown.StringId.For<UserAccount, long>()
624+
}
625+
}
626+
};
627+
628+
string route = $"/workItems/{existingWorkItem.StringId}/relationships/%20%20";
629+
630+
// Act
631+
(HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecutePostAsync<Document>(route, requestBody);
632+
633+
// Assert
634+
httpResponse.ShouldHaveStatusCode(HttpStatusCode.NotFound);
635+
636+
responseDocument.Errors.ShouldHaveCount(1);
637+
638+
ErrorObject error = responseDocument.Errors[0];
639+
error.StatusCode.Should().Be(HttpStatusCode.NotFound);
640+
error.Title.Should().Be("The requested relationship does not exist.");
641+
error.Detail.Should().Be("Resource of type 'workItems' does not contain a relationship named ' '.");
642+
error.Source.Should().BeNull();
643+
error.Meta.Should().NotContainKey("requestBody");
644+
}
645+
604646
[Fact]
605647
public async Task Cannot_add_for_relationship_mismatch_between_url_and_body()
606648
{

‎test/JsonApiDotNetCoreTests/IntegrationTests/ReadWrite/Updating/Relationships/RemoveFromToManyRelationshipTests.cs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,48 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
714714
error.Meta.Should().NotContainKey("requestBody");
715715
}
716716

717+
[Fact]
718+
public async Task Cannot_remove_from_whitespace_relationship_in_url()
719+
{
720+
// Arrange
721+
WorkItem existingWorkItem = _fakers.WorkItem.GenerateOne();
722+
723+
await _testContext.RunOnDatabaseAsync(async dbContext =>
724+
{
725+
dbContext.WorkItems.Add(existingWorkItem);
726+
await dbContext.SaveChangesAsync();
727+
});
728+
729+
var requestBody = new
730+
{
731+
data = new[]
732+
{
733+
new
734+
{
735+
type = "userAccounts",
736+
id = Unknown.StringId.For<UserAccount, long>()
737+
}
738+
}
739+
};
740+
741+
string route = $"/workItems/{existingWorkItem.StringId}/relationships/%20";
742+
743+
// Act
744+
(HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteDeleteAsync<Document>(route, requestBody);
745+
746+
// Assert
747+
httpResponse.ShouldHaveStatusCode(HttpStatusCode.NotFound);
748+
749+
responseDocument.Errors.ShouldHaveCount(1);
750+
751+
ErrorObject error = responseDocument.Errors[0];
752+
error.StatusCode.Should().Be(HttpStatusCode.NotFound);
753+
error.Title.Should().Be("The requested relationship does not exist.");
754+
error.Detail.Should().Be("Resource of type 'workItems' does not contain a relationship named ' '.");
755+
error.Source.Should().BeNull();
756+
error.Meta.Should().NotContainKey("requestBody");
757+
}
758+
717759
[Fact]
718760
public async Task Cannot_remove_for_relationship_mismatch_between_url_and_body()
719761
{

0 commit comments

Comments
 (0)