Skip to content

Dependencies: Hold Microsoft.OpenApi at 2.9.0 on the v17 line (do not auto-bump) - #23455

Closed
AndyButland wants to merge 1 commit into
v17/devfrom
v17/task/update-microsoft-openapi
Closed

Dependencies: Hold Microsoft.OpenApi at 2.9.0 on the v17 line (do not auto-bump)#23455
AndyButland wants to merge 1 commit into
v17/devfrom
v17/task/update-microsoft-openapi

Conversation

@AndyButland

@AndyButland AndyButland commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Description

Keeps Microsoft.OpenApi held at 2.9.0 on the v17 line and documents why, so the routine dependency-update process (umb-update-server-dependencies-for-minor) does not bump it. Targets v17/dev.

This PR started life as the deferred 2.11.0 upgrade + Delivery API contract regeneration (the follow-up promised in #23453). On review, that upgrade is being kept back rather than merged — see below — so the PR now only adds a HOLD comment to the pin (central + Umbraco.Web.UI inline). Comment-only; no version change.

Why hold

Microsoft.OpenApi 2.10.0+ reworked how nullable schemas serialize to OpenAPI 3.0, regressing the Delivery API 3.0 contract (which the v17 line generates via Swashbuckle):

  • type-less nullable schemas — the open property dictionaries ApiElement.Properties / ApiMedia.Properties (IDictionary<string, object?>) — emit "enum": [null], which in JSON-Schema terms constrains every value to null, instead of "nullable": true;
  • nullable $refs wrapped in oneOf (media focalPoint, coordinates) lose the "nullable": true marker entirely, so the contract advertises them as non-nullable even though they are nullable at runtime (ImageFocalPoint?, ImageCropCoordinates?).

Both mislead downstream client generation/validation. Upstream this is a deliberate OpenAPI 3.0.3 spec-compliance change (nullable only applies when type is present in the same schema object) and is tracked at microsoft/OpenAPI.NET#2967. As of now there is no fixed 2.x release (2.11.0 is the latest).

No security cost to holding: 2.9.0 is itself the patched version for CVE-2026-49451 / GHSA-v5pm-xwqc-g5wc. The 18.x line is unaffected — it generates OpenAPI 3.1 via Microsoft.AspNetCore.OpenApi, which doesn't exercise the changed 3.0 path.

Change

When to revisit

Once the 2.x track ships a fix (watch #2967), do the 2.11.0+ upgrade and regenerate the Delivery API contract in a fresh PR. If a security advisory forces Microsoft.OpenApi off 2.9.0 before then, a Swashbuckle schema-filter workaround will be needed to restore the 3.0 nullable output.

Testing

Solution should build and CI checks pass. Comment-only change; no functional impact.

Copilot AI review requested due to automatic review settings July 22, 2026 09:26
@claude

claude Bot commented Jul 22, 2026

Copy link
Copy Markdown

Claude finished @AndyButland's task in 3m 24s —— View job


PR Review

Target: origin/v17/dev · Based on commit: 9fe0c5eb · Skipped: 0 noise files out of 3 total

Bumps Microsoft.OpenApi from 2.9.0 to 2.11.0 (central pin + Umbraco.Web.UI inline) and regenerates the Delivery API OpenAPI 3.0 contract snapshot to reflect the library's changed nullable serialization behavior.

  • Other changes: Two behavioral changes in the emitted Delivery API contract: (1) open property dictionaries serialized as "enum": [null] instead of "nullable": true; (2) focalPoint and coordinates schemas no longer carry "nullable": true.

Important

  • tests/Umbraco.Tests.Integration/Umbraco.Core/DeliveryApi/OpenApiContractTest.cs:1420 and :1500: focalPoint and coordinates drop "nullable": true from the contract. Both are genuinely nullable at runtime — ImageFocalPoint? on IApiMediaWithCrops and ImageCropCoordinates? on ImageCrop. Any client generated from the new contract would treat these fields as non-nullable, risking null reference failures at runtime when the API legitimately returns null (e.g. a media item with no focal point set). This is already called out in the PR description as requiring explicit sign-off; confirming the analysis here. If the regression is acceptable for 17.7, the generated-client risk should be documented in the release notes so downstream authors know to add null guards.

Suggestions

  • tests/Umbraco.Tests.Integration/Umbraco.Core/DeliveryApi/OpenApiContractTest.cs:1412 and :1411: "enum": [null] is not strictly equivalent to "nullable": true on a typeless schema under OpenAPI 3.0. The former restricts additional-property values to null only; the latter allows any JSON value with null permitted. In practice the open property bags hold real content values (strings, objects, etc.), so a strict code generator may misinterpret the new schema. The PR description calls this cosmetic — that may be true for popular generators (NSwag, Kiota) that ignore over-constrained enums — but worth filing an upstream issue with microsoft/OpenAPI.NET if one doesn't exist, so the 3.0 writer correctly emits {} or an anyOf for a nullable-any schema rather than enum: [null].

Approved with Suggestions for improvement

Good to go, but please carefully consider the importance of the suggestions.

The key concern — focalPoint/coordinates nullability loss — is already identified and framed as a conscious sign-off decision. The implementation is correct and consistent (both the central pin and the inline override are updated to 2.11.0). The enum: [null] point is a low-priority upstream tracking item, not a blocker.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 upgrades Microsoft.OpenApi to 2.11.0 on the v17 line and updates the committed Delivery API OpenAPI 3.0 contract expectation so OpenApiContractTest matches the newly generated output.

Changes:

  • Bump the centrally managed Microsoft.OpenApi version 2.9.0 → 2.11.0.
  • Keep Umbraco.Web.UI’s explicit Microsoft.OpenApi reference aligned at 2.11.0 (it opts out of central package management).
  • Regenerate the Delivery API OpenAPI 3.0 contract string used by OpenApiContractTest, reflecting nullable-schema serialization changes in 2.11.0.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
Directory.Packages.props Updates the central Microsoft.OpenApi pin to 2.11.0.
src/Umbraco.Web.UI/Umbraco.Web.UI.csproj Updates the explicit Microsoft.OpenApi reference to 2.11.0 for the project that doesn’t use central package management.
tests/Umbraco.Tests.Integration/Umbraco.Core/DeliveryApi/OpenApiContractTest.cs Updates the expected OpenAPI 3.0 contract output to match 2.11.0 generation (notably around nullable serialization).

Comment on lines 1278 to 1282
"additionalProperties": {
"nullable": true
"enum": [
null
]
}
Comment on lines +1419 to +1423
@@ -1416,8 +1420,7 @@
{
"$ref": "#/components/schemas/ImageFocalPointModel"
}
],
"nullable": true
]
Microsoft.OpenApi 2.10.0+ reworked OpenAPI 3.0 nullability serialization
(regression confirmed in 2.11.0): nullable, type-less schemas emit
"enum": [null] and nullable oneOf $refs drop "nullable" entirely, which
corrupts the generated Delivery API 3.0 contract (fails OpenApiContractTest)
and misleads client code generation. The v17 line generates OpenAPI 3.0 via
Swashbuckle, so it is affected; the v18 line uses Microsoft.AspNetCore.OpenApi
(3.1) and is not.

Add a HOLD comment to the pin (central + Umbraco.Web.UI inline) so the routine
dependency-update process does not bump it, referencing the upstream tracking
issue microsoft/OpenAPI.NET#2967. 2.9.0 is already
patched for CVE-2026-49451, so holding carries no security cost. Comment-only;
no version change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@AndyButland
AndyButland force-pushed the v17/task/update-microsoft-openapi branch from 9fe0c5e to 595fab9 Compare July 22, 2026 09:54
@AndyButland AndyButland changed the title Dependencies: Update Microsoft.OpenApi to 2.11.0 and regenerate the Delivery API contract Dependencies: Hold Microsoft.OpenApi at 2.9.0 on the v17 line (do not auto-bump) Jul 22, 2026
@AndyButland

Copy link
Copy Markdown
Contributor Author

Hold comment is now added to #23453, due to go into release/17.6 and hence v17/dev. So we don't need this any more.

@sonarqubecloud

Copy link
Copy Markdown

@AndyButland
AndyButland deleted the v17/task/update-microsoft-openapi branch July 22, 2026 10:29
@Youssef1313

Copy link
Copy Markdown

@AndyButland Can you please clarify more the issue you're facing?

The enum: [ null ] should be produced if you only and strictly have a schema like new OpenApiSchema { Type = JsonSchemaType.Null }.

Note that the Object Model reflects OpenAPI 3.2 semantics, which is mostly JSON Schema DRAFT 2020-12. So, in 3.2 (and DRAFT 2020-12), the above schema maps to:

{
  "type": "null"
}

Such schema literally means that only null is allowed, and so enum: [ null ] is correct way to represent the same semantics in OpenAPI 3.0.

@AndyButland

Copy link
Copy Markdown
Contributor Author

I've prepared a standalone repository that shows the difference in output we are seeing @Youssef1313, you can find it here: https://github.com/AndyButland/OpenApiNullableRepro

It surfaced here at Umbraco as a failure in an integration test where we compare the actual output with an expected one, and that flagged the changes that this smaller reproduction shows.

@Youssef1313

Copy link
Copy Markdown

@AndyButland My question isn't about showing the output difference. The output difference that was introduced is intentional.

The question is, why do you think the output is wrong, and if there are any tools you feed the output to which broke you (e.g, do you feed the document to some client generator, and it started producing something invalid? Can I have more details on that?)

@Youssef1313

Copy link
Copy Markdown

@AndyButland fwiw, the implementation of SetNullable on Swashbuckle itself was buggy and wrong. This appears to have gotten fixed in domaindrivendev/Swashbuckle.AspNetCore#3976

@AndyButland

Copy link
Copy Markdown
Contributor Author

OK, that I don't have. We have this API for customers to use when building headless application using content managed in Umbraco. We don't actively use it ourselves, but we do have integration tests verifying that the output is as we expect, and we found this discrepancy when upgrading to Microsoft.OpenApi 2.11. It may well be harmless for all clients, but at least for now, we've held on 2.9 to avoid any unexpected changes for a client relying on the previous output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants