Skip to content

Commit a108db4

Browse files
authored
Fix content type error format (#5207)
Change the content type error message when using post search to be an operation outcome like other standard error messages.
1 parent ffb2148 commit a108db4

File tree

6 files changed

+68
-26
lines changed

6 files changed

+68
-26
lines changed

src/Microsoft.Health.Fhir.Api/Features/Routing/SearchPostReroutingMiddleware.cs

Lines changed: 57 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,58 +3,96 @@
33
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
44
// -------------------------------------------------------------------------------------------------
55

6+
using System;
67
using System.Collections.Generic;
78
using System.Collections.Specialized;
89
using System.Linq;
910
using System.Net;
1011
using System.Threading.Tasks;
1112
using System.Web;
1213
using EnsureThat;
14+
using Hl7.Fhir.Model;
1315
using Microsoft.AspNetCore.Http;
16+
using Microsoft.Extensions.Logging;
1417
using Microsoft.Extensions.Primitives;
18+
using Microsoft.Health.Fhir.Api.Features.ActionResults;
19+
using Microsoft.Health.Fhir.Core.Extensions;
1520
using Microsoft.Health.Fhir.Core.Features.Routing;
1621

1722
namespace Microsoft.Health.Fhir.Api.Features.Routing
1823
{
1924
public class SearchPostReroutingMiddleware
2025
{
2126
private readonly RequestDelegate _next;
27+
private readonly ILogger<SearchPostReroutingMiddleware> _logger;
2228

23-
public SearchPostReroutingMiddleware(RequestDelegate next)
29+
public SearchPostReroutingMiddleware(RequestDelegate next, ILogger<SearchPostReroutingMiddleware> logger)
2430
{
2531
EnsureArg.IsNotNull(next);
2632
_next = next;
33+
_logger = logger;
2734
}
2835

2936
public async Task Invoke(HttpContext context)
3037
{
3138
var request = context.Request;
3239

33-
if (request != null
34-
&& request.Method == "POST"
35-
&& request.Path.Value.EndsWith(KnownRoutes.Search, System.StringComparison.OrdinalIgnoreCase))
40+
try
3641
{
37-
if (request.ContentType is null || request.HasFormContentType)
42+
if (request != null
43+
&& request.Method == "POST"
44+
&& request.Path.Value.EndsWith(KnownRoutes.Search, System.StringComparison.OrdinalIgnoreCase))
3845
{
39-
if (request.HasFormContentType)
46+
if (request.ContentType is null || request.HasFormContentType)
4047
{
41-
var mergedPairs = GetUniqueFormAndQueryStringKeyValues(HttpUtility.ParseQueryString(request.QueryString.ToString()), request.Form);
42-
request.Query = mergedPairs;
48+
_logger.LogInformation("Rerouting POST to GET with query parameters from form body.");
49+
50+
if (request.HasFormContentType)
51+
{
52+
var mergedPairs = GetUniqueFormAndQueryStringKeyValues(HttpUtility.ParseQueryString(request.QueryString.ToString()), request.Form);
53+
request.Query = mergedPairs;
54+
}
55+
56+
request.ContentType = null;
57+
request.Form = null;
58+
request.Path = request.Path.Value.Substring(0, request.Path.Value.Length - KnownRoutes.Search.Length);
59+
request.Method = "GET";
4360
}
61+
else
62+
{
63+
_logger.LogDebug("Rejecting POST with invalid Content-Type.");
4464

45-
request.ContentType = null;
46-
request.Form = null;
47-
request.Path = request.Path.Value.Substring(0, request.Path.Value.Length - KnownRoutes.Search.Length);
48-
request.Method = "GET";
49-
}
50-
else
51-
{
52-
context.Response.Clear();
53-
context.Response.StatusCode = (int)HttpStatusCode.BadRequest;
54-
await context.Response.WriteAsync(Api.Resources.ContentTypeFormUrlEncodedExpected);
55-
return;
65+
context.Response.Clear();
66+
context.Response.StatusCode = (int)HttpStatusCode.BadRequest;
67+
68+
var operationOutcome = new OperationOutcome
69+
{
70+
Id = Guid.NewGuid().ToString(),
71+
Issue = new List<OperationOutcome.IssueComponent>()
72+
{
73+
new OperationOutcome.IssueComponent()
74+
{
75+
Severity = OperationOutcome.IssueSeverity.Error,
76+
Code = OperationOutcome.IssueType.Invalid,
77+
Diagnostics = Api.Resources.ContentTypeFormUrlEncodedExpected,
78+
},
79+
},
80+
Meta = new Meta()
81+
{
82+
LastUpdated = Clock.UtcNow,
83+
},
84+
};
85+
86+
await context.Response.WriteAsJsonAsync(operationOutcome);
87+
return;
88+
}
5689
}
5790
}
91+
catch (Exception ex)
92+
{
93+
_logger.LogError(ex, "Error occurred while rerouting POST search to GET.");
94+
throw;
95+
}
5896

5997
await _next.Invoke(context);
6098
}

src/Microsoft.Health.Fhir.Azure.UnitTests/Api/SearchPostReroutingMiddlewareTests.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Net.Http;
99
using System.Threading.Tasks;
1010
using Microsoft.AspNetCore.Http;
11+
using Microsoft.Extensions.Logging.Abstractions;
1112
using Microsoft.Health.Fhir.Api;
1213
using Microsoft.Health.Fhir.Api.Features.Routing;
1314
using Microsoft.Health.Fhir.Tests.Common;
@@ -30,7 +31,7 @@ public SearchPostReroutingMiddlewareTests()
3031
{
3132
_httpContext = new DefaultHttpContext();
3233
_requestDelegate = Substitute.For<RequestDelegate>();
33-
_middleware = new SearchPostReroutingMiddleware(_requestDelegate);
34+
_middleware = new SearchPostReroutingMiddleware(_requestDelegate, new NullLogger<SearchPostReroutingMiddleware>());
3435
}
3536

3637
[Theory]
@@ -64,7 +65,9 @@ public async Task GivenSearchRequestViaPost_WhenContentTypeIsSpecified_InvalidCo
6465
_httpContext.Response.Body.Position = 0;
6566
using var reader = new StreamReader(_httpContext.Response.Body);
6667
var body = reader.ReadToEnd();
67-
Assert.Equal(ApiResources.ContentTypeFormUrlEncodedExpected, body);
68+
var expectedString = ApiResources.ContentTypeFormUrlEncodedExpected.Replace("\"", "\\\"");
69+
expectedString = $"\"{expectedString}\"";
70+
Assert.Contains(expectedString, body);
6871
Assert.Equal((int)HttpStatusCode.BadRequest, _httpContext.Response.StatusCode);
6972
}
7073
}

src/Microsoft.Health.Fhir.Shared.Api/Features/Exceptions/BaseExceptionMiddleware.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,8 @@ public async Task Invoke(HttpContext context)
114114

115115
doesOperationOutcomeHaveError = true;
116116

117+
_logger.LogError(exception, "An unhandled exception occurred while processing the request");
118+
117119
await ExecuteResultAsync(context, result);
118120
}
119121
finally

src/Microsoft.Health.Fhir.Shared.Api/Microsoft.Health.Fhir.Shared.Api.projitems

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
<Compile Include="$(MSBuildThisFileDirectory)Features\ActionConstraints\ConditionalConstraintAttribute.cs" />
2929
<Compile Include="$(MSBuildThisFileDirectory)Features\ActionResults\FhirResult.cs" />
3030
<Compile Include="$(MSBuildThisFileDirectory)Features\ActionResults\MemberMatchResult.cs" />
31-
<Compile Include="$(MSBuildThisFileDirectory)Features\ActionResults\OperationOutcomeResult.cs" />
3231
<Compile Include="$(MSBuildThisFileDirectory)Features\ActionResults\OperationSmartConfigurationResult.cs" />
3332
<Compile Include="$(MSBuildThisFileDirectory)Features\ActionResults\OperationVersionsResult.cs" />
3433
<Compile Include="$(MSBuildThisFileDirectory)Features\Filters\Metrics\ActionExecutedStatistics.cs" />

src/Microsoft.Health.Fhir.SqlServer/Features/Storage/SqlRetry/SqlRetryService.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -422,19 +422,19 @@ public async Task<SqlConnection> GetConnection(ISqlConnectionBuilder sqlConnecti
422422
{
423423
SqlConnection conn;
424424
var sw = Stopwatch.StartNew();
425-
var logSB = new StringBuilder("Long running retrieve SQL connection");
425+
var logSB = new StringBuilder("Long running retrieve SQL connection. ");
426426
var isReadOnlyConnection = isReadOnly ? "read-only " : string.Empty;
427427

428428
if (!isReadOnly || !_coreFeatureConfiguration.SupportsSqlReplicas)
429429
{
430-
logSB.AppendLine("Not read only");
430+
logSB.AppendLine("Not read only. ");
431431
conn = await sqlConnectionBuilder.GetSqlConnectionAsync(false, applicationName);
432432
}
433433
else
434434
{
435-
logSB.AppendLine("Checking read only");
435+
logSB.AppendLine("Checking read only. ");
436436
var replicaTrafficRatio = GetReplicaTrafficRatio(sqlConnectionBuilder, logger);
437-
logSB.AppendLine($"Got replica traffic ratio in {sw.Elapsed.TotalSeconds} seconds. Ratio is {replicaTrafficRatio}");
437+
logSB.AppendLine($"Got replica traffic ratio in {sw.Elapsed.TotalSeconds} seconds. Ratio is {replicaTrafficRatio}. ");
438438

439439
if (replicaTrafficRatio < 0.5) // it does not make sense to use replica less than master at all
440440
{

0 commit comments

Comments
 (0)