Skip to content

Commit df7376d

Browse files
CopilotjmprieurbgavrilMS
authored
Limit error response reading in DownstreamApi to avoid performance issues with large payloads (#3532)
* Initial plan * Add error response size limiting in DownstreamApi Co-authored-by: jmprieur <[email protected]> * Add unit tests for error response size limiting Co-authored-by: jmprieur <[email protected]> * Add cancellation token support and revert changes to deprecated files Co-authored-by: jmprieur <[email protected]> * Propagate cancellation token through DeserializeOutputAsync call chain Co-authored-by: jmprieur <[email protected]> * Use streaming to read error responses for better performance Co-authored-by: bgavrilMS <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: jmprieur <[email protected]> Co-authored-by: bgavrilMS <[email protected]> Co-authored-by: Jean-Marc Prieur <[email protected]>
1 parent 252e8c6 commit df7376d

File tree

12 files changed

+413
-298
lines changed

12 files changed

+413
-298
lines changed

‎src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.HttpMethods.cs‎

Lines changed: 262 additions & 262 deletions
Large diffs are not rendered by default.

‎src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.HttpMethods.tt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,18 +119,18 @@ namespace Microsoft.Identity.Web
119119
}
120120
if (response.IsSuccessStatusCode == false)
121121
{
122-
errorResponseContent = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
123122
errorStatusCode = (int)response.StatusCode;
123+
errorResponseContent = await ReadErrorResponseContentAsync(response, cancellationToken).ConfigureAwait(false);
124124
}
125125
response.EnsureSuccessStatusCode();
126126
<# }
127127
if (hasOutput && framework == "net8")
128128
{ #>
129-
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions, outputJsonTypeInfo).ConfigureAwait(false);
129+
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions, outputJsonTypeInfo, cancellationToken).ConfigureAwait(false);
130130
<# }
131131
else if (hasOutput)
132132
{#>
133-
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions).ConfigureAwait(false);
133+
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions, cancellationToken).ConfigureAwait(false);
134134
<# } #>
135135
}
136136
catch(Exception ex) when (

‎src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.cs‎

Lines changed: 56 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ public Task<HttpResponseMessage> CallApiForAppAsync(
125125
effectiveInput?.Dispose();
126126
}
127127

128-
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions).ConfigureAwait(false);
128+
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions, cancellationToken).ConfigureAwait(false);
129129
}
130130

131131
/// <inheritdoc/>
@@ -151,7 +151,7 @@ public Task<HttpResponseMessage> CallApiForAppAsync(
151151
effectiveInput?.Dispose();
152152
}
153153

154-
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions).ConfigureAwait(false);
154+
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions, cancellationToken).ConfigureAwait(false);
155155
}
156156

157157
/// <inheritdoc/>
@@ -168,7 +168,7 @@ public Task<HttpResponseMessage> CallApiForAppAsync(
168168
HttpResponseMessage response = await CallApiInternalAsync(serviceName, effectiveOptions, true,
169169
null, null, cancellationToken).ConfigureAwait(false);
170170

171-
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions).ConfigureAwait(false);
171+
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions, cancellationToken).ConfigureAwait(false);
172172
}
173173

174174
/// <inheritdoc/>
@@ -185,7 +185,7 @@ public Task<HttpResponseMessage> CallApiForAppAsync(
185185
DownstreamApiOptions effectiveOptions = MergeOptions(serviceName, downstreamApiOptionsOverride);
186186
HttpResponseMessage response = await CallApiInternalAsync(serviceName, effectiveOptions, false,
187187
null, user, cancellationToken).ConfigureAwait(false);
188-
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions).ConfigureAwait(false);
188+
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions, cancellationToken).ConfigureAwait(false);
189189
}
190190

191191
#if NET8_0_OR_GREATER
@@ -212,7 +212,7 @@ public Task<HttpResponseMessage> CallApiForAppAsync(
212212
effectiveInput?.Dispose();
213213
}
214214

215-
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions, outputJsonTypeInfo).ConfigureAwait(false);
215+
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions, outputJsonTypeInfo, cancellationToken).ConfigureAwait(false);
216216
}
217217

218218
/// <inheritdoc/>
@@ -227,7 +227,7 @@ public Task<HttpResponseMessage> CallApiForAppAsync(
227227
DownstreamApiOptions effectiveOptions = MergeOptions(serviceName, downstreamApiOptionsOverride);
228228
HttpResponseMessage response = await CallApiInternalAsync(serviceName, effectiveOptions, false,
229229
null, user, cancellationToken).ConfigureAwait(false);
230-
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions, outputJsonTypeInfo).ConfigureAwait(false);
230+
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions, outputJsonTypeInfo, cancellationToken).ConfigureAwait(false);
231231
}
232232

233233
/// <inheritdoc/>
@@ -251,7 +251,7 @@ public Task<HttpResponseMessage> CallApiForAppAsync(
251251
effectiveInput?.Dispose();
252252
}
253253

254-
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions, outputJsonTypeInfo).ConfigureAwait(false);
254+
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions, outputJsonTypeInfo, cancellationToken).ConfigureAwait(false);
255255
}
256256

257257
/// <inheritdoc/>
@@ -266,7 +266,7 @@ public Task<HttpResponseMessage> CallApiForAppAsync(
266266
HttpResponseMessage response = await CallApiInternalAsync(serviceName, effectiveOptions, true,
267267
null, null, cancellationToken).ConfigureAwait(false);
268268

269-
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions, outputJsonTypeInfo).ConfigureAwait(false);
269+
return await DeserializeOutputAsync<TOutput>(response, effectiveOptions, outputJsonTypeInfo, cancellationToken).ConfigureAwait(false);
270270
}
271271

272272
internal static HttpContent? SerializeInput<TInput>(TInput input, DownstreamApiOptions effectiveOptions, JsonTypeInfo<TInput> inputJsonTypeInfo)
@@ -299,10 +299,10 @@ public Task<HttpResponseMessage> CallApiForAppAsync(
299299
return httpContent;
300300
}
301301

302-
internal static async Task<TOutput?> DeserializeOutputAsync<TOutput>(HttpResponseMessage response, DownstreamApiOptions effectiveOptions, JsonTypeInfo<TOutput> outputJsonTypeInfo)
302+
internal static async Task<TOutput?> DeserializeOutputAsync<TOutput>(HttpResponseMessage response, DownstreamApiOptions effectiveOptions, JsonTypeInfo<TOutput> outputJsonTypeInfo, CancellationToken cancellationToken = default)
303303
where TOutput : class
304304
{
305-
return await DeserializeOutputImplAsync<TOutput>(response, effectiveOptions, outputJsonTypeInfo);
305+
return await DeserializeOutputImplAsync<TOutput>(response, effectiveOptions, outputJsonTypeInfo, cancellationToken);
306306
}
307307
#endif
308308

@@ -396,7 +396,7 @@ public Task<HttpResponseMessage> CallApiForAppAsync(
396396
[RequiresUnreferencedCode("Calls JsonSerializer.Serialize<TInput>")]
397397
[RequiresDynamicCode("Calls JsonSerializer.Serialize<TInput>")]
398398
#endif
399-
internal static async Task<TOutput?> DeserializeOutputAsync<TOutput>(HttpResponseMessage response, DownstreamApiOptions effectiveOptions)
399+
internal static async Task<TOutput?> DeserializeOutputAsync<TOutput>(HttpResponseMessage response, DownstreamApiOptions effectiveOptions, CancellationToken cancellationToken = default)
400400
where TOutput : class
401401
{
402402
try
@@ -405,7 +405,7 @@ public Task<HttpResponseMessage> CallApiForAppAsync(
405405
}
406406
catch
407407
{
408-
string error = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
408+
string error = await ReadErrorResponseContentAsync(response, cancellationToken).ConfigureAwait(false);
409409

410410
#if NET5_0_OR_GREATER
411411
throw new HttpRequestException($"{(int)response.StatusCode} {response.StatusCode} {error}", null, response.StatusCode);
@@ -447,7 +447,7 @@ public Task<HttpResponseMessage> CallApiForAppAsync(
447447
}
448448
}
449449

450-
private static async Task<TOutput?> DeserializeOutputImplAsync<TOutput>(HttpResponseMessage response, DownstreamApiOptions effectiveOptions, JsonTypeInfo<TOutput> outputJsonTypeInfo)
450+
private static async Task<TOutput?> DeserializeOutputImplAsync<TOutput>(HttpResponseMessage response, DownstreamApiOptions effectiveOptions, JsonTypeInfo<TOutput> outputJsonTypeInfo, CancellationToken cancellationToken = default)
451451
where TOutput : class
452452
{
453453
try
@@ -456,7 +456,7 @@ public Task<HttpResponseMessage> CallApiForAppAsync(
456456
}
457457
catch
458458
{
459-
string error = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
459+
string error = await ReadErrorResponseContentAsync(response, cancellationToken).ConfigureAwait(false);
460460

461461
#if NET5_0_OR_GREATER
462462
throw new HttpRequestException($"{(int)response.StatusCode} {response.StatusCode} {error}", null, response.StatusCode);
@@ -645,7 +645,46 @@ private static void AddCallerSDKTelemetry(DownstreamApiOptions effectiveOptions)
645645
CallerSDKDetails["caller-sdk-id"];
646646
effectiveOptions.AcquireTokenOptions.ExtraQueryParameters["caller-sdk-ver"] =
647647
CallerSDKDetails["caller-sdk-ver"];
648-
}
649-
}
650-
}
648+
}
649+
}
650+
651+
/// <summary>
652+
/// Safely reads error response content with size limits to avoid performance issues with large payloads.
653+
/// </summary>
654+
/// <param name="response">The HTTP response message.</param>
655+
/// <param name="cancellationToken">Cancellation token.</param>
656+
/// <returns>The error response content, truncated if necessary.</returns>
657+
internal static async Task<string> ReadErrorResponseContentAsync(HttpResponseMessage response, CancellationToken cancellationToken = default)
658+
{
659+
const int maxErrorContentLength = 4096;
660+
661+
long? contentLength = response.Content.Headers.ContentLength;
662+
663+
if (contentLength.HasValue && contentLength.Value > maxErrorContentLength)
664+
{
665+
return $"[Error response too large: {contentLength.Value} bytes, not captured]";
666+
}
667+
668+
// Use streaming to read only up to maxErrorContentLength to avoid loading entire response into memory
669+
#if NET5_0_OR_GREATER
670+
using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false);
671+
#else
672+
using var stream = await response.Content.ReadAsStreamAsync().ConfigureAwait(false);
673+
#endif
674+
using var reader = new StreamReader(stream);
675+
676+
char[] buffer = new char[maxErrorContentLength];
677+
int readCount = await reader.ReadBlockAsync(buffer, 0, maxErrorContentLength).ConfigureAwait(false);
678+
679+
string errorResponseContent = new string(buffer, 0, readCount);
680+
681+
// Check if there's more content that was truncated
682+
if (readCount == maxErrorContentLength && reader.Peek() != -1)
683+
{
684+
errorResponseContent += "... (truncated)";
685+
}
686+
687+
return errorResponseContent;
688+
}
689+
}
651690
}

‎src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net462/InternalAPI.Unshipped.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
11
#nullable enable
2+
static Microsoft.Identity.Web.DownstreamApi.DeserializeOutputAsync<TOutput>(System.Net.Http.HttpResponseMessage! response, Microsoft.Identity.Abstractions.DownstreamApiOptions! effectiveOptions, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<TOutput?>!
23
static Microsoft.Identity.Web.DownstreamApi.Logger.HttpRequestError(Microsoft.Extensions.Logging.ILogger! logger, string! ServiceName, string! BaseUrl, string! RelativePath, int statusCode, string! responseContent, System.Exception? ex) -> void
4+
static Microsoft.Identity.Web.DownstreamApi.ReadErrorResponseContentAsync(System.Net.Http.HttpResponseMessage! response, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<string!>!

‎src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net472/InternalAPI.Unshipped.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
11
#nullable enable
2+
static Microsoft.Identity.Web.DownstreamApi.DeserializeOutputAsync<TOutput>(System.Net.Http.HttpResponseMessage! response, Microsoft.Identity.Abstractions.DownstreamApiOptions! effectiveOptions, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<TOutput?>!
23
static Microsoft.Identity.Web.DownstreamApi.Logger.HttpRequestError(Microsoft.Extensions.Logging.ILogger! logger, string! ServiceName, string! BaseUrl, string! RelativePath, int statusCode, string! responseContent, System.Exception? ex) -> void
4+
static Microsoft.Identity.Web.DownstreamApi.ReadErrorResponseContentAsync(System.Net.Http.HttpResponseMessage! response, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<string!>!

‎src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net8.0/InternalAPI.Unshipped.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,5 @@
11
#nullable enable
2+
static Microsoft.Identity.Web.DownstreamApi.DeserializeOutputAsync<TOutput>(System.Net.Http.HttpResponseMessage! response, Microsoft.Identity.Abstractions.DownstreamApiOptions! effectiveOptions, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<TOutput?>!
3+
static Microsoft.Identity.Web.DownstreamApi.DeserializeOutputAsync<TOutput>(System.Net.Http.HttpResponseMessage! response, Microsoft.Identity.Abstractions.DownstreamApiOptions! effectiveOptions, System.Text.Json.Serialization.Metadata.JsonTypeInfo<TOutput!>! outputJsonTypeInfo, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<TOutput?>!
24
static Microsoft.Identity.Web.DownstreamApi.Logger.HttpRequestError(Microsoft.Extensions.Logging.ILogger! logger, string! ServiceName, string! BaseUrl, string! RelativePath, int statusCode, string! responseContent, System.Exception? ex) -> void
5+
static Microsoft.Identity.Web.DownstreamApi.ReadErrorResponseContentAsync(System.Net.Http.HttpResponseMessage! response, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<string!>!

‎src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/net9.0/InternalAPI.Unshipped.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,5 @@
11
#nullable enable
2+
static Microsoft.Identity.Web.DownstreamApi.DeserializeOutputAsync<TOutput>(System.Net.Http.HttpResponseMessage! response, Microsoft.Identity.Abstractions.DownstreamApiOptions! effectiveOptions, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<TOutput?>!
3+
static Microsoft.Identity.Web.DownstreamApi.DeserializeOutputAsync<TOutput>(System.Net.Http.HttpResponseMessage! response, Microsoft.Identity.Abstractions.DownstreamApiOptions! effectiveOptions, System.Text.Json.Serialization.Metadata.JsonTypeInfo<TOutput!>! outputJsonTypeInfo, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<TOutput?>!
24
static Microsoft.Identity.Web.DownstreamApi.Logger.HttpRequestError(Microsoft.Extensions.Logging.ILogger! logger, string! ServiceName, string! BaseUrl, string! RelativePath, int statusCode, string! responseContent, System.Exception? ex) -> void
5+
static Microsoft.Identity.Web.DownstreamApi.ReadErrorResponseContentAsync(System.Net.Http.HttpResponseMessage! response, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<string!>!

‎src/Microsoft.Identity.Web.DownstreamApi/PublicAPI/netstandard2.0/InternalAPI.Unshipped.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
11
#nullable enable
2+
static Microsoft.Identity.Web.DownstreamApi.DeserializeOutputAsync<TOutput>(System.Net.Http.HttpResponseMessage! response, Microsoft.Identity.Abstractions.DownstreamApiOptions! effectiveOptions, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<TOutput?>!
23
static Microsoft.Identity.Web.DownstreamApi.Logger.HttpRequestError(Microsoft.Extensions.Logging.ILogger! logger, string! ServiceName, string! BaseUrl, string! RelativePath, int statusCode, string! responseContent, System.Exception? ex) -> void
4+
static Microsoft.Identity.Web.DownstreamApi.ReadErrorResponseContentAsync(System.Net.Http.HttpResponseMessage! response, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<string!>!

‎src/Microsoft.Identity.Web.Sidecar/OpenAPI/Microsoft.Identity.Web.Sidecar.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@
7474
{
7575
"name": "AgentUsername",
7676
"in": "query",
77-
"description": "The username (UPN) of the agent.",
77+
"description": "The username (UPN) of the user agent identity.",
7878
"schema": {
7979
"type": "string"
8080
}
@@ -272,7 +272,7 @@
272272
{
273273
"name": "AgentUsername",
274274
"in": "query",
275-
"description": "The username (UPN) of the agent.",
275+
"description": "The username (UPN) of the user agent identity.",
276276
"schema": {
277277
"type": "string"
278278
}
@@ -478,7 +478,7 @@
478478
{
479479
"name": "AgentUserId",
480480
"in": "query",
481-
"description": "The Object ID of the agent (OID).",
481+
"description": "The Object ID of the user agent identity (OID).",
482482
"schema": {
483483
"type": "string"
484484
}
@@ -676,7 +676,7 @@
676676
{
677677
"name": "AgentUserId",
678678
"in": "query",
679-
"description": "The Object ID of the agent (OID).",
679+
"description": "The Object ID of the user agent identity (OID).",
680680
"schema": {
681681
"type": "string"
682682
}

0 commit comments

Comments
 (0)