-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[release/9.0] [OpenAPI] Use invariant culture for TextWriter #62239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Ensure OpenAPI documents are written to a culture-invariant `TextWriter` implementation. Resolves dotnet#60628.
|
@martincostello I updated the issue with the servicing template in case you'd like to review it and mark the PR as ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses a culture-invariant formatting issue in OpenAPI document generation. When hosting applications under non-English cultures (like fr-FR), the OpenAPI serialization was producing incorrect JSON with locale-specific number formatting (e.g., using commas instead of dots for decimals).
- Introduces a new constructor for
Utf8BufferTextWriterthat accepts anIFormatProvider - Updates the OpenAPI endpoint handler to use
CultureInfo.InvariantCulturefor serialization - Refactors test approach from direct document serialization to HTTP endpoint testing
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Utf8BufferTextWriter.cs | Adds constructor overload accepting IFormatProvider for culture-invariant formatting |
| OpenApiEndpointRouteBuilderExtensions.cs | Updates OpenAPI serialization to use invariant culture and creates new writer instance instead of pooled one |
| OpenApiDocumentIntegrationTests.cs | Simplifies tests to use HTTP client instead of direct document service |
| Program.cs | Adds middleware to set French culture for testing culture-invariant behavior |
| TestController.cs | Adds test endpoint with float property to verify decimal formatting |
| Multiple snapshot files | Updates test snapshots with new server URL and endpoint entries |
|
@captainsafia Thanks, done. I wanted to make sure you were happy with the changes/approach first before trying to guide it through shiproom. |
captainsafia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
servers change to snapshots is expected since we move from resolving the document via the service to via the endpoint.
New test scenario matches what we have in main.
|
@captainsafia tactics approved but asked the usual question about whether we scanned for variants (other instances)? |
|
Holding this for October at this point |
|
/azp run |
Updated [Microsoft.AspNetCore.Mvc.Testing](https://github.com/dotnet/aspnetcore) from 9.0.9 to 9.0.11. <details> <summary>Release notes</summary> _Sourced from [Microsoft.AspNetCore.Mvc.Testing's releases](https://github.com/dotnet/aspnetcore/releases)._ ## 9.0.11 [Release](https://github.com/dotnet/core/releases/tag/v9.0.11) ## What's Changed * Update branding to 9.0.11 by @vseanreesermsft in dotnet/aspnetcore#63950 * [release/9.0] Update dependencies from dotnet/extensions by @dotnet-maestro[bot] in dotnet/aspnetcore#63677 * [release/9.0] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/aspnetcore#63678 * [release/9.0] (deps): Bump src/submodules/googletest from `eb2d85e` to `9706f75` by @dependabot[bot] in dotnet/aspnetcore#63894 * [release/9.0] Fixed devtools url used for debug with chrome and edge by @github-actions[bot] in dotnet/aspnetcore#61948 * [release/9.0] (http2): Lower WINDOWS_UPDATE received on (half)closed stream to stream abortion by @DeagleGross in dotnet/aspnetcore#63934 * [release/9.0] Re-quarantine ServerRoutingTest.NavigationLock_OverlappingNavigationsCancelExistingNavigations_HistoryNavigation by @github-actions[bot] in dotnet/aspnetcore#63956 * [release/9.0] Fix nginx install on mac, linux by @wtgodbe in dotnet/aspnetcore#63966 * [Hot Reload] Do not attempt to apply empty deltas. by @tmat in dotnet/aspnetcore#63979 * Merging internal commits for release/9.0 by @vseanreesermsft in dotnet/aspnetcore#64036 * Revert log level severity for unknown proxy in ForwardedHeadersMiddleware by @BrennanConroy in dotnet/aspnetcore#64091 * Set timeoutInMinutes to 0 for Windows build job by @vseanreesermsft in dotnet/aspnetcore#64126 **Full Changelog**: dotnet/aspnetcore@v9.0.10...v9.0.11 ## 9.0.10 [Release](https://github.com/dotnet/core/releases/tag/v9.0.10) ## What's Changed * Update branding to 9.0.10 by @vseanreesermsft in dotnet/aspnetcore#63510 * [9.0] Make duplicate deb/rpm packages so we can sign them with the new PMC key by @jkoritzinsky in dotnet/aspnetcore#63249 * [release/9.0] Extend Unofficial 1ES template in IdentityModel nightly tests job by @github-actions[bot] in dotnet/aspnetcore#63465 * [release/9.0] (deps): Bump src/submodules/googletest from `373af2e` to `eb2d85e` by @dependabot[bot] in dotnet/aspnetcore#63501 * [release/9.0] Quarantine ResponseBody_WriteContentLength_PassedThrough by @wtgodbe in dotnet/aspnetcore#63533 * [release/9.0] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/aspnetcore#63304 * [release/9.0] [OpenAPI] Use invariant culture for TextWriter by @martincostello in dotnet/aspnetcore#62239 * [release/9.0] Update dependencies from dotnet/extensions by @dotnet-maestro[bot] in dotnet/aspnetcore#63303 * Unquarantine `RadioButtonGetsResetAfterSubmittingEnhancedForm` by @ilonatommy in dotnet/aspnetcore#63556 * [release/9.0] Update dependencies from dotnet/extensions by @dotnet-maestro[bot] in dotnet/aspnetcore#63577 * Merging internal commits for release/9.0 by @vseanreesermsft in dotnet/aspnetcore#63604 * [release/9.0] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/aspnetcore#63648 * backport(9.0): Fix runtime architecture detection logic in ANCM. by @DeagleGross in dotnet/aspnetcore#63707 **Full Changelog**: dotnet/aspnetcore@v9.0.9...v9.0.10 Commits viewable in [compare view](dotnet/aspnetcore@v9.0.9...v9.0.11). </details> Updated [Microsoft.AspNetCore.OpenApi](https://github.com/dotnet/aspnetcore) from 9.0.9 to 9.0.11. <details> <summary>Release notes</summary> _Sourced from [Microsoft.AspNetCore.OpenApi's releases](https://github.com/dotnet/aspnetcore/releases)._ ## 9.0.11 [Release](https://github.com/dotnet/core/releases/tag/v9.0.11) ## What's Changed * Update branding to 9.0.11 by @vseanreesermsft in dotnet/aspnetcore#63950 * [release/9.0] Update dependencies from dotnet/extensions by @dotnet-maestro[bot] in dotnet/aspnetcore#63677 * [release/9.0] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/aspnetcore#63678 * [release/9.0] (deps): Bump src/submodules/googletest from `eb2d85e` to `9706f75` by @dependabot[bot] in dotnet/aspnetcore#63894 * [release/9.0] Fixed devtools url used for debug with chrome and edge by @github-actions[bot] in dotnet/aspnetcore#61948 * [release/9.0] (http2): Lower WINDOWS_UPDATE received on (half)closed stream to stream abortion by @DeagleGross in dotnet/aspnetcore#63934 * [release/9.0] Re-quarantine ServerRoutingTest.NavigationLock_OverlappingNavigationsCancelExistingNavigations_HistoryNavigation by @github-actions[bot] in dotnet/aspnetcore#63956 * [release/9.0] Fix nginx install on mac, linux by @wtgodbe in dotnet/aspnetcore#63966 * [Hot Reload] Do not attempt to apply empty deltas. by @tmat in dotnet/aspnetcore#63979 * Merging internal commits for release/9.0 by @vseanreesermsft in dotnet/aspnetcore#64036 * Revert log level severity for unknown proxy in ForwardedHeadersMiddleware by @BrennanConroy in dotnet/aspnetcore#64091 * Set timeoutInMinutes to 0 for Windows build job by @vseanreesermsft in dotnet/aspnetcore#64126 **Full Changelog**: dotnet/aspnetcore@v9.0.10...v9.0.11 ## 9.0.10 [Release](https://github.com/dotnet/core/releases/tag/v9.0.10) ## What's Changed * Update branding to 9.0.10 by @vseanreesermsft in dotnet/aspnetcore#63510 * [9.0] Make duplicate deb/rpm packages so we can sign them with the new PMC key by @jkoritzinsky in dotnet/aspnetcore#63249 * [release/9.0] Extend Unofficial 1ES template in IdentityModel nightly tests job by @github-actions[bot] in dotnet/aspnetcore#63465 * [release/9.0] (deps): Bump src/submodules/googletest from `373af2e` to `eb2d85e` by @dependabot[bot] in dotnet/aspnetcore#63501 * [release/9.0] Quarantine ResponseBody_WriteContentLength_PassedThrough by @wtgodbe in dotnet/aspnetcore#63533 * [release/9.0] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/aspnetcore#63304 * [release/9.0] [OpenAPI] Use invariant culture for TextWriter by @martincostello in dotnet/aspnetcore#62239 * [release/9.0] Update dependencies from dotnet/extensions by @dotnet-maestro[bot] in dotnet/aspnetcore#63303 * Unquarantine `RadioButtonGetsResetAfterSubmittingEnhancedForm` by @ilonatommy in dotnet/aspnetcore#63556 * [release/9.0] Update dependencies from dotnet/extensions by @dotnet-maestro[bot] in dotnet/aspnetcore#63577 * Merging internal commits for release/9.0 by @vseanreesermsft in dotnet/aspnetcore#63604 * [release/9.0] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/aspnetcore#63648 * backport(9.0): Fix runtime architecture detection logic in ANCM. by @DeagleGross in dotnet/aspnetcore#63707 **Full Changelog**: dotnet/aspnetcore@v9.0.9...v9.0.10 Commits viewable in [compare view](dotnet/aspnetcore@v9.0.9...v9.0.11). </details> Updated [Microsoft.NET.Test.Sdk](https://github.com/microsoft/vstest) from 18.0.0 to 18.0.1. <details> <summary>Release notes</summary> _Sourced from [Microsoft.NET.Test.Sdk's releases](https://github.com/microsoft/vstest/releases)._ ## 18.0.1 ## What's Changed Fixing an issue with loading covrun64.dll on systems that have .NET 10 SDK installed: https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/10.0/code-coverage-dynamic-native-instrumentation * Disable DynamicNative instrumentation by default by @nohwnd in microsoft/vstest#15298 * Update MicrosoftInternalCodeCoveragePackageVersion to 18.0.6 by @nohwnd in microsoft/vstest#15312 ### Internal changes * Update VersionPrefix to 18.0.1 by @nohwnd in microsoft/vstest#15301 * Update build tools to 17.8.43 by @nohwnd in microsoft/vstest#15305 **Full Changelog**: microsoft/vstest@v18.0.0...v18.0.1 Commits viewable in [compare view](microsoft/vstest@v18.0.0...v18.0.1). </details> Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore <dependency name> major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore <dependency name> minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore <dependency name>` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore <dependency name>` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore <dependency name> <ignore condition>` will remove the ignore condition of the specified dependency and ignore conditions </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Use culture-invariant TextWriter implementation for OpenAPI.
Port of #62193 for .NET 9.
Description
To avoid any accidental changes to SignalR's use of
Utf8BufferTextWriterI've removed the use of the writer pooling for OpenAPI and instead a new instance is created every time.Fixes #60628
Customer Impact
Developers run into this issue when hosting under non-en cultures, but no workarounds are available.
Regression?
Risk
Change is isolated to the OpenAPI serialization path and uses an existing overload; no impact on runtime behavior or other subsystems.
Verification
Packaging changes reviewed?