[release/2.3] Merge internal commits for May#66762
Merged
Merged
Conversation
RFC9112 updating working to "MUST" close the connection after processing a request with both Content-Length and Transfer-Encoding headers. We are still allowed to process the current request as long as we remove the Content-Length header.
…o ResponseCachingKeyProvider delimiter characters delimiter characters are not validated in the request, meaning users can get the "artificial" response instead of their real request.
These 2 packages don't have direct dependencies on the packages being patched - only transitive. ---- #### AI description (iteration 1) #### PR Classification Bug fix to correct the patch package list for a specific version in `PatchConfig.props`. #### PR Summary Updates patch configuration for `VersionPrefix` `2.3.10` by removing incorrectly included packages from the patch list. - `eng/PatchConfig.props`: Removed `Microsoft.AspNetCore` and `Microsoft.AspNetCore.Mvc.Core` from `PackagesInPatch` for version `2.3.10`. <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot -->
And ship all packages that depend on it ---- #### AI description (iteration 1) #### PR Classification Dependency update to bump Crypto.Xml (System.Security.Cryptography.Xml) to a newer patch version and adjust patch package inclusion for release 2.3.10. #### PR Summary Updates a cryptography XML package version and expands the set of packages included in the 2.3.10 patch configuration. - `build/dependencies.props`: Bump `SystemSecurityCryptographyXmlPackageVersion` from **8.0.2** to **8.0.3**. - `eng/PatchConfig.props`: Expand `PackagesInPatch` for `VersionPrefix == 2.3.10` to include additional ASP.NET Core authentication/data protection/identity/mvc/session/spa packages (plus `Microsoft.Owin.Security.Interop`). <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot -->
Contributor
|
Hi @wtgodbe. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR merges a set of servicing changes into release/2.3, primarily tightening HTTP request handling around Transfer-Encoding + Content-Length (CL+TE) scenarios in Kestrel/HttpSys and hardening ResponseCaching cache-key construction against delimiter injection.
Changes:
- Kestrel + HttpSys: when both
Transfer-EncodingandContent-Lengthare present, preserve the original length asX-Content-Length, removeContent-Length, and (by default) close the connection after responding; add coverage for these behaviors. - ResponseCaching: reject (don’t cache / don’t serve from cache) requests whose key material contains the cache-key delimiter characters; add logging + tests.
- Servicing/build updates: patch packaging config tweaks and a dependency version bump.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Servers/Kestrel/test/FunctionalTests/ChunkedRequestTests.cs | Adds functional coverage ensuring CL+TE requests result in connection close after the response. |
| src/Servers/Kestrel/Core/test/Http1ConnectionTests.cs | Adds unit coverage verifying Content-Length is removed when Transfer-Encoding exists. |
| src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs | Implements CL+TE handling (move CL to X-Content-Length, remove CL, close connection by default). |
| src/Servers/HttpSys/test/FunctionalTests/RequestHeaderTests.cs | Adds functional tests around Transfer-Encoding parsing and CL removal/connection-close behavior. |
| src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs | Ensures disconnect flag is applied when request keep-alive was disabled; minor initialization cleanup. |
| src/Servers/HttpSys/src/RequestProcessing/Response.cs | Uses request keep-alive state as an input to connection lifetime decisions. |
| src/Servers/HttpSys/src/RequestProcessing/Request.cs | Implements CL+TE header normalization and keep-alive suppression (behind an AppContext switch). |
| src/Middleware/ResponseCaching/test/TestUtils.cs | Adds a new logged-message ID used by delimiter-rejection tests. |
| src/Middleware/ResponseCaching/test/ResponseCachingMiddlewareTests.cs | Adds tests validating delimiter rejection in cache lookup/storage paths + logging. |
| src/Middleware/ResponseCaching/test/ResponseCachingKeyProviderTests.cs | Adds tests for delimiter detection and exception behavior. |
| src/Middleware/ResponseCaching/src/ResponseCachingMiddleware.cs | Catches delimiter exceptions during lookup/storage key creation and disables caching for those requests. |
| src/Middleware/ResponseCaching/src/Internal/ResponseCachingKeyProvider.cs | Adds delimiter validation for key components and throws a dedicated exception on invalid input. |
| src/Middleware/ResponseCaching/src/Internal/LoggerExtensions.cs | Adds a debug log message for invalid cache key symbols. |
| src/Middleware/ResponseCaching/src/Internal/CacheKeyDelimiterException.cs | New internal exception used to signal invalid delimiter presence in cache-key components. |
| eng/PatchConfig.props | Adds a comment for 2.3.10 patch config (but contains a version typo). |
| Directory.Build.targets | Adjusts patch packaging selection logic for 2.3.10 (ship everything except Razor). |
| build/sources.props | Minor formatting adjustment in restore sources list. |
| build/dependencies.props | Bumps System.Security.Cryptography.Xml package version (8.0.2 → 8.0.3). |
Comments suppressed due to low confidence (1)
src/Middleware/ResponseCaching/src/ResponseCachingMiddleware.cs:231
- Similarly, if
CreateLookupVaryByKeysthrowsCacheKeyDelimiterException,TryServeFromCacheAsyncreturnsfalseand bypasses theonly-if-cachedcheck later in the method. IfCache-Control: only-if-cachedis present, this should still result in a 504 rather than falling through to the next middleware when no cached response can be served.
try
{
foreach (var varyKey in _keyProvider.CreateLookupVaryByKeys(context))
{
if (await TryServeCachedResponseAsync(context, await _cache.GetAsync(varyKey)))
{
return true;
}
}
}
catch (CacheKeyDelimiterException)
{
_logger.LogRequestContainsInvalidCacheSymbols();
return false;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
BrennanConroy
approved these changes
May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.