Skip to content

⬆️ upgrade swagger-ui to 5.27.1#4218

Open
josemigallas wants to merge 12 commits into
masterfrom
THREESCALE-12255_swagger_ui_27
Open

⬆️ upgrade swagger-ui to 5.27.1#4218
josemigallas wants to merge 12 commits into
masterfrom
THREESCALE-12255_swagger_ui_27

Conversation

@josemigallas
Copy link
Copy Markdown
Contributor

@josemigallas josemigallas commented Feb 5, 2026

THREESCALE-12255: Update swagger-ui to 5.27.1

Latest is 5.31 but 5.28 breaks our implementation. While I figure out why, this is the most recent version we can update to.

Verification steps

This module is used in OAS 3 specs in the following pages:

  • 3scale API Documentation /p/admin/api_docs
  • api_docs with apiconfig/services/:service_id/api_docs/:id/preview
  • dev portal docs page /docs

ℹ️ https://github.com/swagger-api/swagger-ui/releases

@qltysh
Copy link
Copy Markdown

qltysh Bot commented Feb 5, 2026

❌ 17 blocking issues (24 total)

Tool Category Rule Count
eslint Lint Unsafe member access .catch on an any value. 6
eslint Lint Unsafe argument of type any assigned to a parameter of type SchemaProperty. 3
eslint Lint Unsafe call of an any typed value. 2
eslint Lint Unsafe spread of an any value in an array. 2
eslint Lint Unsafe return of an any typed value. 2
eslint Lint Variable name ClearDefaultValuesPlugin must match one of the following formats: camelCase, UPPER_CASE 1
eslint Lint This assertion is unnecessary since it does not change the type of the expression. 1
qlty Structure Function with high complexity (count = 10): clearGeneratedDefaults 5
qlty Structure Function with many returns (count = 7): ClearDefaultValuesPlugin 2

@josemigallas josemigallas requested a review from a team February 6, 2026 10:13
@josemigallas josemigallas self-assigned this Feb 6, 2026
Copy link
Copy Markdown
Contributor

@jlledom jlledom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the problem with 5.28? It's surprising we can upgrade 2 major versions at once but then there's a breaking change in a minor version.

Copy link
Copy Markdown
Contributor

@jlledom jlledom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work fine for me, I found a small bug. In the request body section for POST and PUT requests, the fields are auto-filled wrongly, I think.

Before:
Image

After:

Image

@mayorova
Copy link
Copy Markdown
Contributor

mayorova commented Feb 9, 2026

Seems to work fine for me, I found a small bug. In the request body section for POST and PUT requests, the fields are auto-filled wrongly, I think.

That rings a bell... See Notes for reviewer in #3934

@josemigallas
Copy link
Copy Markdown
Contributor Author

josemigallas commented Feb 9, 2026

What's the problem with 5.28? It's surprising we can upgrade 2 major versions at once but then there's a breaking change in a minor version.

It's TBI (to be investigated). It simply throws an error in the browser console and doesn't render anything.

@github-actions github-actions Bot added the Stale label Mar 14, 2026
@3scale 3scale deleted a comment from github-actions Bot Mar 16, 2026
@github-actions
Copy link
Copy Markdown

This PR is stale because it has not received activity for more than 30 days. Remove stale label or comment or this will be closed in 15 days.

@github-actions github-actions Bot added the Stale label Apr 16, 2026
@jlledom jlledom removed the Stale label Apr 16, 2026
@jlledom
Copy link
Copy Markdown
Contributor

jlledom commented Apr 17, 2026

I've been checking this. I rebased to master and tried to update to 5.32.4, the last release.

I hit the error after upgrading to 5.28.0. After an investigation I found the problem is due to 5.28.0 and later versions depending on React 18 de facto: even when their package.json states they accept React between 16 and 19, the fact is they only accept React 18 and 19 by mistake. People depending on swagger-ui-dist receive the correct React already bundled, but we use swagger-ui which expects React to be installed by us.

Somebody hit the same bug: swagger-api/swagger-ui#10636

And there's a fix already, but not merged: swagger-api/swagger-ui#10642

The fix looks abandoned, it doesn't seem it's going to be merged soon. So we can't rely on that. In order to upgrade to newer versions, we would need:

  • react: 17.0.2 → 18.3.1
  • react-dom: 17.0.2 → 18.3.1
  • react-redux: 7.2.9 → ^9.2.0
  • @wojtekmaj/enzyme-adapter-react-17@cfaester/enzyme-adapter-react-18
  • Add resolutions for swagger-ui/react and swagger-ui/react-dom → 18.3.1
  • Fix tests

I told Claude to do it and I could upgrade to swagger-ui 5.32.4. Everything in porta looked correct. So it can be done. I kept the branch if you are interested.

IMO we should merge this and get swagger-ui 5.27.1. In another PR we can upgrade to React 18 and last swagger-ui release. The issue I mentioned in this comment would still need to be fixed: #4218 (review)

@jlledom jlledom force-pushed the THREESCALE-12255_swagger_ui_27 branch from 9f1e455 to 276e4e4 Compare April 28, 2026 15:14
@jlledom jlledom requested review from akostadinov and mayorova April 29, 2026 08:30
@jlledom
Copy link
Copy Markdown
Contributor

jlledom commented Apr 29, 2026

I took this issue while @josemigallas is out. I made some changes and this is the current status:

  1. swagger-ui upgraded to 5.27.1, we can't go further without upgrading to React 18
  2. swagger-ui pulls minimatch@^10.0.0 which includes a license change. The new license, BlueOak-1.0.0 is approved by OSI and Fedora. I added it to the list of permitted licenses: 4c52779
  3. Newer versions of swagger-ui set a default value for fields in POST and PUT requests, like 0 for numbers or "string" for strings. I was discussing with Claude how to do fix this, considering I know very little about TypeScript or swagger-ui particularities. Finally I opted for writing a plugin that overwrites those default values: 0b22dbb

@jlledom jlledom force-pushed the THREESCALE-12255_swagger_ui_27 branch from ec96dc5 to 4c52779 Compare April 29, 2026 08:44
}
}

const ClearDefaultValuesPlugin: SwaggerUIPlugin = () => {
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.

This plugin seems to be working well for our own API docs (/p/admin/api_docs), but it is not applied to the preview of the API docs in admin portal, and the published versions in the dev portal :(

Image Image

Honestly, this feature is so weird, it's extremely annoying to have to remove this pre-filled "example" values!!!

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.

Right! Here: 48f45bf

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.

Do you think it would be possible to also apply this logic when "Reset" is clicked?...

Screencast.From.2026-05-18.13-11-07.mp4

Copy link
Copy Markdown
Contributor

@jlledom jlledom May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it: 24cd50d

I was annoying Claude until the code was fast enough, however, the resulting code is pretty complicated.

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.

OMG... This code is something... and might be a potential ticking bomb for the next Swagger UI upgrade, maybe?...

Why do we need to upgrade swagger-ui again? 😅

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.

I've been trying to find a simpler way to do this but it's not easy. There are many ways, but all of them imply dirty monkey patching and on-keystroke overhead. The current approach is the one that uses only standard plugins mechanism, doesn't monkey patch and doesn't raises 20-30 function calls per keystroke. But the code is more extensive. I think it's the best option considering the tradeoff,

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.

I see...

But again - do we need to upgrade? What are we gaining?
We can't go to the latest version (at least not for now). Is the difference between 5.12.3 and 5.27.1 worth it?

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.

I assume it's always good to keep the gems as much up to date as possible. To reduce tech debt and to get security fixes.

Since the complicated code is in a self-contained and tested plugin that only affects a small issue and not functionality, I think it's OK to keep it. The worst thing that could happen is that we aren't able to migrate this plugin to newer versions, but in that case the only effect would be having those annoying values back to forms.

I don't think swagger-ui maintainers are going to listen to community and fix it. What's our alternative then? never upgrading is not an option neither, because that would make us vulnerable.

According to Claude those are the CVE's fixed between 5.12.3 and 5.27.1 (Haven't verified myself):

Here are the security fixes in swagger-ui between v5.12.3 and v5.27.1 that affect the npm package (not Docker-only):

  High Severity

  ┌─────────┬──────────────────────────────────────────────────────────────────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ Version │                               CVE                                │                                                    Description                                                     │
  ├─────────┼──────────────────────────────────────────────────────────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ v5.20.0 │ CVE-2024-39338 (https://nvd.nist.gov/vuln/detail/CVE-2024-39338) │ SSRF in Axios — path-relative URLs processed as protocol-relative, allowing request redirection                    │
  ├─────────┼──────────────────────────────────────────────────────────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ v5.20.1 │ CVE-2025-27152 (https://nvd.nist.gov/vuln/detail/CVE-2025-27152) │ SSRF + credential leak in Axios — absolute URLs bypass baseURL, leaking credentials to attacker-controlled servers │
  ├─────────┼──────────────────────────────────────────────────────────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ v5.22.0 │ Multiple                                                         │ Axios updated to 1.9.0 to cover all known vulnerabilities                                                          │
  └─────────┴──────────────────────────────────────────────────────────────────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

  Medium Severity
  
  ┌─────────┬──────────────────────────────────────────────────────────────────┬───────────────────────────────────────────────────────────────────┐
  │ Version │                               CVE                                │                            Description                            │
  ├─────────┼──────────────────────────────────────────────────────────────────┼───────────────────────────────────────────────────────────────────┤
  │ v5.20.0 │ CVE-2024-47764 (https://nvd.nist.gov/vuln/detail/CVE-2024-47764) │ Cookie injection — out-of-bounds chars in cookie name/path/domain │
  ├─────────┼──────────────────────────────────────────────────────────────────┼───────────────────────────────────────────────────────────────────┤
  │ v5.24.0 │ None                                                             │ ReDoS — crafted pattern in schema could hang the browser          │
  └─────────┴──────────────────────────────────────────────────────────────────┴───────────────────────────────────────────────────────────────────┘

  Low Severity
  
  ┌─────────┬──────────────────────────────────────────────────────────────────┬───────────────────────────────────────────────┐
  │ Version │                              Issue                               │                  Description                  │
  ├─────────┼──────────────────────────────────────────────────────────────────┼───────────────────────────────────────────────┤
  │ v5.17.4 │ PR #9909 (https://github.com/swagger-api/swagger-ui/pull/9909)   │ patch-package incorrectly in production deps  │
  ├─────────┼──────────────────────────────────────────────────────────────────┼───────────────────────────────────────────────┤
  │ v5.27.0 │ PR #10528 (https://github.com/swagger-api/swagger-ui/pull/10528) │ Relative path sanitization fix in sanitizeUrl │
  └─────────┴──────────────────────────────────────────────────────────────────┴───────────────────────────────────────────────┘

  Unresolved in v5.27.1

  - CVE-2024-53382 (Medium) — PrismJS DOM Clobbering XSS, blocked by upstream react-syntax-highlighter dependency chain

  The most critical ones are the two Axios SSRF vulnerabilities fixed in v5.20.0 and v5.20.1. Any version >= v5.22.0 covers all the high-severity fixes.

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.

@jlledom and what do you think about this: 0ac596b

It's not the same as your solution - because this one doesn't care if there are actual "examples" in the specification, but as we saw in swagger-api/swagger-ui#5776 - auto-filling with values, even when they are explicitly set in the "examples", in many cases (I'd say in 100% of cases 😒 ) are not desired.

So, I would just not set anything in the inputs and that's it.

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.

Haven't tested, but if we don't have to worry about preserving existing examples, then of course the plugin can be much simpler. If that works fine and test passes, fine for me.

Your commit implements the getSampleSchema way. That approach has the advantage of being common for init and reset secenarios, but the problem is that method is called multiple times per keystroke, but if emptySampleFromSchema is faster than the original method, then performance will improve in fact.

The alternative is to wrap the JsonSchemaForm component with dispatchInitialValue: false, which is what this commit did: 9d01a34

That's even simpler but only works for the init scenario, if we want the reset scenario to behave that way without intercepting all keystrokes, we need to wrap the selectDefaultRequestBodyValue, which is what 24cd50d does.

In all cases, not worrying about the example values makes it much simpler so I'll let you decide, you might want to try the JsonSchemaForm+selectDefaultRequestBodyValue approach as well.

But summarizing: fine for me 👍

josemigallas and others added 7 commits May 18, 2026 09:56
Co-Authored-By: Claude <noreply@anthropic.com>
Newer swagger-ui versions auto-fill form fields with generated
defaults like "string" for string types or 0 for integers. This
was not the previous behavior, where fields were left empty with
the parameter name shown as an HTML placeholder.

Add ClearDefaultValuesPlugin which wraps the JsonSchemaForm
component and sets dispatchInitialValue to false, preventing
the auto-fill on initial render. The plugin is extracted into
its own module and applied across all three API docs screens:
provider admin, per-service, and developer portal.

Co-Authored-By: Claude <noreply@anthropic.com>
@jlledom jlledom force-pushed the THREESCALE-12255_swagger_ui_27 branch from 48f45bf to 9d01a34 Compare May 18, 2026 08:30
@jlledom jlledom requested a review from mayorova May 18, 2026 08:39
@akostadinov
Copy link
Copy Markdown
Contributor

akostadinov commented May 18, 2026

@jlledom , great work, glad you decided to adopt this one!

Could you review the qlty (eslint) report and see what makes sense and what doesn't?

Also Bob suggested a new plugin test:

// test/javascript/src/ActiveDocs/ClearDefaultValuesPlugin.spec.ts              
describe('ClearDefaultValuesPlugin', () => {                                    
  it('prevents initial default value dispatch', () => {                         
    // Test that dispatchInitialValue is set to false                           
  });                                                                           
                                                                                
  it('maintains behavior after reset', () => {                                  
    // Test reset button interaction                                            
  });                                                                           
});                                                                             

jlledom added 5 commits May 19, 2026 17:57
Replace the fn.getSampleSchema override with two targeted overrides to
eliminate per-keystroke overhead. The previous implementation wrapped
getSampleSchema which was called ~20 times per keystroke as swagger-ui
recomputes initialValue on every render.

New approach combines:
- JsonSchemaForm wrapper that sets dispatchInitialValue=false for
  primitive fields without explicit examples/defaults (handles initial
  render)
- selectDefaultRequestBodyValue selector wrapper that clears generated
  defaults from the Reset button value (handles form reset)

Neither fires per keystroke since JsonSchemaForm runs at mount and the
selector is only called by onResetClick.

Assisted-by: Claude Code
Add comprehensive test coverage for the optimized plugin including:
- JsonSchemaForm wrapper behavior with Immutable.js schema objects
- Conditional dispatchInitialValue based on schema examples/defaults
- selectDefaultRequestBodyValue selector wrapper
- clearGeneratedDefaults helper function

Tests verify that generated defaults ("string", 0, true) are cleared
while explicit examples and defaults from the spec are preserved.

Assisted-by: Claude Code
Add Cucumber feature tests that verify the ClearDefaultValuesPlugin
behavior in the browser using the Swagger UI interface. Tests cover
both initial form field values and Reset button behavior.

The feature ensures that form fields for properties without explicit
examples/defaults remain empty while fields with explicit values show
those values correctly.

Includes a new test fixture (user-api-3.0.json) and supporting step
definitions for checking request body field values.

Assisted-by: Claude Code
Remove unused JsonSchemaFormProperties import from ThreeScaleApiDocs.ts
since it's no longer referenced in the file.
Add comprehensive inline documentation to clarify the plugin's two-override
strategy and implementation details for code reviewers. Comments explain:

- The purpose of each interface, constant, and helper function
- The distinction between plain-object and Immutable.js schema handling
- Why each override runs when it does (mount vs Reset)
- Content-type filtering rationale (form-encoded vs JSON)
- The wrapSelectors calling convention and parameter usage

This makes the optimization approach more understandable without requiring
knowledge of previous implementation attempts or swagger-ui internals.

Assisted-by: Claude Code
@jlledom
Copy link
Copy Markdown
Contributor

jlledom commented May 19, 2026

Also Bob suggested a new plugin test:

Here: 78dc76b

And here: 2b9107c

@jlledom
Copy link
Copy Markdown
Contributor

jlledom commented May 19, 2026

Could you review the qlty (eslint) report and see what makes sense and what doesn't?

I don't think any of those is worth the time.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants