Skip to content

Conversation

@bkoelman
Copy link
Contributor

Pull Request

The issue or feature being addressed

Fix for #3283 (comment): do not read/write fractional numbers in the current culture, which depends on OS-level settings, potentially resulting in unparsable JSON.

Details on the issue fix or feature implementation

According to https://json-schema.org/understanding-json-schema/reference/numeric#number, exponents are allowed, which is why I combined the flags:

[Flags]
public enum NumberStyles
{
    Number = AllowThousands | AllowDecimalPoint | AllowTrailingSign | Integer,
    Float = AllowExponent | AllowDecimalPoint | Integer
}

@bkoelman bkoelman mentioned this pull request May 25, 2025
13 tasks
@martincostello
Copy link
Collaborator

Thanks for the fix.

I'm away from a proper computer for the next few days, otherwise I'd do this myself - would you be able to add/update a test somewhere to use a non en-US culture that would hit this bug so we can catch any regression in the future please?

@martincostello martincostello added this to the Future milestone May 26, 2025
@bkoelman
Copy link
Contributor Author

I considered that for a moment, but couldn't find the right place to add such a test. There's no urgency to get this in, I don't mind if you take it from here once you have time.

@martincostello martincostello merged commit 5a0d266 into domaindrivendev:dotnet-vnext Jun 1, 2025
13 of 14 checks passed
@martincostello
Copy link
Collaborator

Test added in #3431 which will be merged into #3283 shortly.

@martincostello
Copy link
Collaborator

The test flushed out a bug with the values not round-tripping correctly. Fixed with 9e6dedf.

@bkoelman bkoelman deleted the fix-culture-dotnet10 branch June 1, 2025 20:26
@martincostello martincostello modified the milestones: Future, v10.0.0 Jun 3, 2025
martincostello added a commit that referenced this pull request Nov 11, 2025
* Support .NET 10

- Add support for ASP.NET Core 10 and Microsoft.OpenApi v2.
- Add opt-in support for OpenAPI 3.1 when targeting `net10.0`.

* Fix test

Fix broken test.

* Remove obsolete method

Remove obsolete method.

* Update to ASP.NET Core 10 preview 2

Update to preview 2 of ASP.NET Core 10, which includes Microsoft.OpenApi 2.0.0-preview7.

* Add TODO

Add TODO to use `SerializeAs()`.

* Partially fix security requirement issue

Use `AddComponent()` to ensure references work correctly.

* Fix test

Fix test by passing the current `OpenApiDocument` to the filter contexts so it can be used to resolve the references.

* Fix test

Fix broken test.

* Fix type tests

Account for flags enum.

* Fix missing descriptions

Fix missing descriptions in .NET 10.

* Refactor fix

Restore the sample projects to how they were, and instead adjust the annotation filters to consider parameters and properties for records.

* Fix package reference

Fix missed rename in merge.

* Fix xUnit1051 warnings

Fix xUnit1051 warnings

* Update to ASP.NET Core 10 preview 3 (#3361)

Update to preview 3 of ASP.NET Core 10.

* Update NuGet packages

Update NuGet packages for .NET 10 preview 3.

* Link to issue

Update Skip with link to GitHub issue.

* Fix typo

"respository" ➡️ "repository".

* Fix tag serialization

Add tags to the document before adding them to the operations as a workaround for microsoft/OpenAPI.NET#2319.

* Fix merge

Fix bad merge with default branch.

* Add issue reference

Add comment with link to issue.

* Remove TODO

Based on tests passing, I think it is correct.

* Fix formatting

Adjust formatting to match what it would be without the pragma.

* Prepare for .NET 10 preview 4 (#3388)

Cherry-pick changes from #3386 to prepare for ASP.NET Core 10 preview 4 and Microsoft.OpenApi 2.0.0-preview.17.

* Fix nullability for null schema Type

Fix bitmasks for adding `JsonSchemaType.Null` if the `Type` is null.

Resolves #3387.

* Re-enable MyGet

Re-enable MyGet on the off-chance that it was a transient issue and has self-resolved.

* Fix rebase

Fix changes after merge from main.

* Skip failing tets

Skip failing tests for now - re-visit once ASP.NET Core 10 preview.4 lets us update to Microsoft.OpenApi 2.0.0-preview.17.

* Update MyGet publish URL

See if the issue is specific to the old V2 push endpoint.

* Re-disable MyGet publishing

Using the v3 feed didn't fix it.

* Update to Microsoft.OpenApi 2.0.0 preview 17

Update Microsoft.OpenApi to version 2.0.0-preview.17 using the latest daily build of ASP.NET Core 10 preview 4.

* Fix API baselines

Update API baselines for change from `OperationType` to `HttpMethod `.

* Skip failing tests

.NET 8 and 9 TestFirst integration tests are failing, so skip for now.

* Use SerializeAs

Use the new `SerializeAs()` method to resolve TODOs.

* Fix build

Use the right writer.

* Apply suggestions from code review

* Fix build

Fix build for official build of .NET 10 preview 4.

* Disable failing tests

Disable tests failing due to yet-to-be-logged issue with the OpenAPI document being truncated/not flushed.

* Fix MergeWith

Fix incorrect file name.

* Publish packages

Publish NuGet packages to the workflow even if tests fail.

* Fix tests

Fix tests due to error in test setup fixture caused by changes in microsoft/OpenAPI.NET#2325.

* Remove workaround

Remove workaround needed for previous preview versions.

* Re-enable tests

Re-enable three skipped tests that are no longer failing.

* Re-enable MyGet publishing

Re-enable publishing to MyGet.

* Apply review suggestions

Apply some of the code-review suggestions. More to follow.

* Workaround VS issue

Workaround issue building the solution in Visual Studio.

* Fix culture-insensitive handling of numbers with [Range] (#3426)

Fix incorrect handling of culture-sensitivity for number ranges.

* Refactor method

Rename constant and move culture to variable.

* Fix range parsing

- Use the correct culture to parse `[Range]`.
- Update snapshots.

* Sort AnyOf and OneOf

Address code review comment.

* Update issue number

Point to latest issue number.

* Refactor [Range] handling

Refactor how `[Range]` is handled to match how ASP.NET Core 10 does.

* Add test cases for `[Range]`

Add various test cases for the behaviour of `[Range]` applying changes to `OpenApiSchema` instances.

* Refactor method to avoid round-trip

Refactor method to avoid redundant round-trip parse/format when the `Maximum` and `Minimum` properties are not strings.

* Tweak comment

Fix grammar.

* Fix comment

Add another missing word.

* Fix conversion

Convert to `decimal` not `double`.

* Update snapshot

Update snapshot after merge.

* Update .NET SDK to 10.0.100-preview.5.25277.114 (#3451)

* Update .NET SDK

Update .NET SDK to version 10.0.100-preview.5.25277.114.

---
updated-dependencies:
- dependency-name: Microsoft.NET.Sdk
  dependency-version: 10.0.100-preview.5.25277.114
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* Bump NuGet packages

Update NuGet packages for .NET 10 preview 5.

---------

Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Martin Costello <[email protected]>

* Bump Microsoft.OpenApi

Bump Microsoft.OpenApi for .NET 10 preview 5.

* Fix build

Update `OpenApiAny` to `JsonNodeExtension`.

* Fix line endings

Fix-up line endings.

* Validate TypedResults is supported for ASP.NET Core 10 (#3455)

Co-authored-by: Javier García de la Noceda Argüelles <[email protected]>

* Support [ProducesResponseType], [Produces], and [ProducesDefaultResponseType] (#3456)

Add support for populating the description for an OpenAPI operation from `[ProducesResponseType]`, `[Produces]`, and/or `[ProducesDefaultResponseType]`.

Resolves #3099.

* Remove workarounds

Remove workarounds from earlier previews.

* Apply suggestions from code review

* Fix build

Fix build after merge.

* Fix package version

Update from preview 1 to preview 5.

* Update PublicAPI baselines

Move renames from Shipped to Unshipped files.

* Update _SdkTasksTFM

Update the value of `_SdkTasksTFM` for .NET 10.

* Update to ASP.NET Core 10 preview.6 (#3479)

Update to ASP.NET Core 10 preview 6 and Microsoft.OpenApi 2.0.0-preview.29.

* Update Microsoft.OpenApi to 2.0.0

Update to the stable release of Microsoft.OpenApi 2.0.0.

* Remove WithOpenApi usage

`WithOpenApi()` will be deprecated in .NET 10 preview 7.

See aspnet/Announcements#519.

* Remove redundant references

Remove redundant references to `Microsoft.AspNetCore.OpenApi`.

* Add UI tests for SwaggerUI and Redoc

Add end-to-end UI tests for SwaggerUI and Redoc to help validate upstream changes.

Resolves #3514.

* Update allowed licenses

Update licenses for Microsoft.OpenApi 2.0.0.

* Update to ASP.NET Core 10 preview 7

Update to preview 7 of ASP.NET Core 10.

* Remove workaround

Remove workaround for resolved issue.

* Bump Microsoft.OpenApi to 2.1.0

- Bump Microsoft.OpenApi to 2.1.0 in test projects.
- Pin to 2.0.0 in shipping packages.

* Verify OpenAPI documents

Verify there are no errors or warnings.

* Update snapshots

Update snapshot baselines.

* Apply suggestions from code review

Bump version to match other TFMs.

* Update to .NET 10 RC1 (#3556)

- Update to release candidate 1 of .NET 10.
- Fix or suppress `ASPDEPR004` and `ASPDEPR008` warnings.

* Fix deprecation warnings

Fix `ASPDEPR004` and `ASPDEPR008` warnings.

* Fix whitespace

Fix-up formatting and package versions.

* Resolve TODO

NSwag now supports .NET 10.

* Bump Duende.IdentityServer

Update Duende.IdentityServer for .NET 10.

* Simplify reference

Remove duplication by defining the reference once.

* Update documentation for v10 and OpenAPI 3.1 (#3585)

- Update the READMEs for changes in v10.
- Update most references to Swagger to OpenAPI.
- Add migration guide for v9 to v10.
- Add the version of Microsoft.OpenApi included to the version table.
- Support for generating code snippets in the documentation from code that is checked to compile.

* Fix build

Fix bad merge.

* Update Microsoft.OpenApi

Bump to v2.3.4.

* Bump Microsoft.OpenApi

Bump Microsoft.OpenApi to 2.3.5.

* Update to .NET 10 RC2

Update to Release Candidate 2 of .NET 10.

* Bump Microsoft.OpenApi

Bump Microsoft.OpenApi to 2.3.6.

* Update Microsoft.OpenApi

Update to 2.3.7.

* Bump Microsoft.OpenApi

Update to 2.3.8.

* Initialized Security property (#3635)

Avoid `NullReferenceException` in synchronous code path.

* Bump Microsoft.OpenApi

Bump Microsoft.OpenApi to 2.3.9.

* Bump Microsoft.OpenApi

Update Microsoft.OpenApi package version to 2.3.0.

* Bump Microsoft.OpenApi

Update Microsoft.OpenApi package version to 2.3.0.

* Update migration guide

- Call out some additional required changes.
- Add specific package versions.
- Link to the Microsoft.AspNetCore.OpenApi NuGet package.

* Add upgrade tip

Added a recommendation to upgrade to Swashbuckle.AspNetCore v9.0.6 before migrating to v10.

* Update to .NET 10 GA

Update to stable release of .NET 10.

---------

Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Bart Koelman <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Javier García de la Noceda Argüelles <[email protected]>
Co-authored-by: Javier García de la Noceda Argüelles <[email protected]>
Co-authored-by: Cesar Roux dit Buisson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants