Skip to content

Commit 0cb58f0

Browse files
aaronburtleCopilotAniruddh25
authored
Return 400 for invalid If-Match values (#3415)
## Why make this change? Closes #3397 ## What is this change? When `DeterminePatchPutSemantics` threw a `DataApiBuilderException` for an invalid `If-Match` value, the exception was thrown before entering `HandleOperation` and the `try-catch` block within. This is where we convert `DataApiBuilderException` into structured JSON error response. We therefore move the `DeterminePatchPutSemantics` call inside of the `try-catch` block within `HandleOperation`, gating the function behind a check of the operation type so that it only applies to `Upsert` and `UpsertIncremental` operations. ## How was this tested? Added a test for `Put` and a test for `Patch` to validate the new behavior. ## Sample Request(s) Any valid `Put` or `Patch` with headers using `If-Match` with a value other than `*` --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
1 parent 5bff17d commit 0cb58f0

3 files changed

Lines changed: 69 additions & 2 deletions

File tree

src/Service.Tests/SqlTests/RestApiTests/Patch/PatchApiTestBase.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,37 @@ await SetupAndRunRestApiTest(
485485
);
486486
}
487487

488+
/// <summary>
489+
/// Tests that a PATCH request with an invalid If-Match header value
490+
/// (anything other than "*") returns a 400 Bad Request response
491+
/// because ETags are not supported.
492+
/// </summary>
493+
[TestMethod]
494+
public virtual async Task PatchOne_Update_InvalidIfMatchHeader_Returns400_Test()
495+
{
496+
Dictionary<string, StringValues> headerDictionary = new();
497+
headerDictionary.Add("If-Match", "\"abc123\"");
498+
string requestBody = @"
499+
{
500+
""title"": ""The Hobbit Returns to The Shire"",
501+
""publisher_id"": 1234
502+
}";
503+
504+
await SetupAndRunRestApiTest(
505+
primaryKeyRoute: "id/1",
506+
queryString: null,
507+
entityNameOrPath: _integrationEntityName,
508+
sqlQuery: string.Empty,
509+
operationType: EntityActionOperation.UpsertIncremental,
510+
headers: new HeaderDictionary(headerDictionary),
511+
requestBody: requestBody,
512+
exceptionExpected: true,
513+
expectedErrorMessage: "Etags not supported, use '*'",
514+
expectedStatusCode: HttpStatusCode.BadRequest,
515+
expectedSubStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest.ToString()
516+
);
517+
}
518+
488519
/// <summary>
489520
/// Test to validate successful execution of PATCH operation which satisfies the database policy for the update operation it resolves into.
490521
/// </summary>

src/Service.Tests/SqlTests/RestApiTests/Put/PutApiTestBase.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,6 +1114,37 @@ await SetupAndRunRestApiTest(
11141114
);
11151115
}
11161116

1117+
/// <summary>
1118+
/// Tests that a PUT request with an invalid If-Match header value
1119+
/// (anything other than "*") returns a 400 Bad Request response
1120+
/// because ETags are not supported.
1121+
/// </summary>
1122+
[TestMethod]
1123+
public virtual async Task PutOne_Update_InvalidIfMatchHeader_Returns400_Test()
1124+
{
1125+
Dictionary<string, StringValues> headerDictionary = new();
1126+
headerDictionary.Add("If-Match", "\"abc123\"");
1127+
string requestBody = @"
1128+
{
1129+
""title"": ""The Return of the King"",
1130+
""publisher_id"": 1234
1131+
}";
1132+
1133+
await SetupAndRunRestApiTest(
1134+
primaryKeyRoute: "id/1",
1135+
queryString: null,
1136+
entityNameOrPath: _integrationEntityName,
1137+
sqlQuery: string.Empty,
1138+
operationType: EntityActionOperation.Upsert,
1139+
headers: new HeaderDictionary(headerDictionary),
1140+
requestBody: requestBody,
1141+
exceptionExpected: true,
1142+
expectedErrorMessage: "Etags not supported, use '*'",
1143+
expectedStatusCode: HttpStatusCode.BadRequest,
1144+
expectedSubStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest.ToString()
1145+
);
1146+
}
1147+
11171148
/// <summary>
11181149
/// Tests that a PUT request with If-Match header (strict update semantics)
11191150
/// still requires a primary key route. When If-Match is present, the operation

src/Service/Controllers/RestController.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ public async Task<IActionResult> Upsert(
161161
{
162162
return await HandleOperation(
163163
route,
164-
DeterminePatchPutSemantics(EntityActionOperation.Upsert));
164+
EntityActionOperation.Upsert);
165165
}
166166

167167
/// <summary>
@@ -181,7 +181,7 @@ public async Task<IActionResult> UpsertIncremental(
181181
{
182182
return await HandleOperation(
183183
route,
184-
DeterminePatchPutSemantics(EntityActionOperation.UpsertIncremental));
184+
EntityActionOperation.UpsertIncremental);
185185
}
186186

187187
/// <summary>
@@ -206,6 +206,11 @@ private async Task<IActionResult> HandleOperation(
206206
{
207207
TelemetryMetricsHelper.IncrementActiveRequests(ApiType.REST);
208208

209+
if (operationType is EntityActionOperation.Upsert or EntityActionOperation.UpsertIncremental)
210+
{
211+
operationType = DeterminePatchPutSemantics(operationType);
212+
}
213+
209214
if (activity is not null)
210215
{
211216
activity.TrackMainControllerActivityStarted(

0 commit comments

Comments
 (0)