Swagger UI should work without Node.js polyfills - rewrite generateCodeVerifier to avoid transitive dependency on Buffer (Node.js API)

[teodorlu]

This PR rewrites generateCodeVerifier to not require the "randombytes" package.

Motivation and Context

As of today, I cannot use swagger-ui-react in my Vite.js application without polyfills. When I try to build, I error out on a missing Buffer in safe-buffer. src/core/utils.js depends on randombytes, which depends on safebuffer. safebuffer uses Buffer - which is a Node.js API, and requires polyfilling outside of Node.js.

I'd like to avoid having to polyfill.

Additional changes may be required to make swagger-ui-react work out of the box with a non-polyfilled application, but this is a start. I'm making this as a PR to make a specific suggestion -- and hear whether avoiding polyfills is something you're interested in.

There are some references to randombytes in model-example.jsx, but that looks like example code to me.

How Has This Been Tested?

1. I've tested the function manually in a separate Node.js REPL
2. The unit test for generateCodeVerifier is still passing
3. I've added a unit test that checks that the output from generateCodeVerifier is a valid base64 string.

My PR contains...

• [ ] No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
• [x] Dependency changes (any modification to dependencies in package.json)
  • I've removed the dependency on randombytes from src/core/utils.js
  • src/core/components/model-example.jsx still uses randombytes. That looks like example code to me. I'm hoping it's possible to replace the sample code with something else, so that we can remove the dependency on randombytes. Suggestions welcome from maintainers that are more familiar with the Swagger UI codebase than I.
  • I haven't touched randombytes in package.json (yet)
• [ ] Bug fixes (non-breaking change which fixes an issue)
• [ ] Improvements (misc. changes to existing features)
• [ ] Features (non-breaking change which adds functionality)

My changes...

• [ ] are breaking changes to a public API (config options, System API, major UI change, etc).
• [ ] are breaking changes to a private API (Redux, component props, utility functions, etc.).
• [ ] are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
• [x] are not breaking changes.

Documentation

• [x] My changes do not require a change to the project documentation.
• [ ] My changes require a change to the project documentation.
• [ ] If yes to above: I have updated the documentation accordingly.

Automated tests

• [ ] My changes can not or do not need to be tested.
• [x] My changes can and should be tested by unit and/or integration tests.
• [x] If yes to above: I have added tests to cover my changes.
• [ ] If yes to above: I have taken care to cover edge cases in my tests.
• [x] All new and existing tests passed.

[teodorlu]

I added a unittest that ensures that we don't throw when decoding base64 - which passes.

[tim-lai]

@teodorlu Thanks for the PR! SwaggerUI had a recent patch update that changed the build system to webpack@5. One of the benefits to this change was to remove extraneous Node.js dependencies and polyfills. However, the caveat is that Node.js polyfills are no longer included by default by webpack@5. This is true for any project based on webpack@5, not just SwaggerUI.

Buffer and process/browser are the two known required Node.js polyfills, and SwaggerUI webpack configuration specifically loads them as a plugin here. There are also known fallbacks and aliases specified here.

All that said, I think that it can't be guaranteed that SwaggerUI could and would remain free of any Node.js polyfills or fallbacks moving forward. So while I'm open to replacing randombytes with a different library (but not within SwaggerUI code directly), which I also think has been added and removed multiple times over time within SwaggerUI, if it's possible, it might be simpler to add the required dependencies to your project.

I'm not familiar with Vite.js or its requirements, but I can say that I've been able to add dependencies to CreateReactApp@5-based projects. CreateReactApp@5 was released a couple months ago, and is also built with webpack@5.

I hope this helps. Let me know, thanks.

[teodorlu]

Thanks for taking the time to give a solid response.

It looks like polyfills in Swagger UI is a bigger piece of work than I imagined. If we merged this PR, released and I tried again, we'd probably just hit another polyfill issue.

---

A first question is whether Swagger UI should be supported from a Vite.js application. If yes, how?

I decided to migrate my application from react-scripts 4 to Vite for build performance. I saw build times decrease by a factor of about 10. Under the hood, Vite relies on esbuild rather than webpack. So something that works out of the box with Webpack doesn't necessarily work with Vite. And you have to think about backwards compatibility with a large user base too.

You'll also have to think about testing, and if you decide to support Vite, I guess you'll want CI checks in place to ensure that Swagger UI keeps on working with Vite.

I think a better place to start is a minimal working example of my build issues with Swagger UI on Vite. Then perhaps some docs for how to configure the correct polyfills. I'll see what I can do.

Thanks again -- Swagger UI is of great help to me and my team!

Teodor

[teodorlu]

Here's a minimal example that reproduces the load issues for SwaggerUI on a Vite app - with stacktrace pointing to randombytes: https://github.com/teodorlu/swagger-ui-on-vite

I'm going to see what I can do on my end with polyfills. I'm still not super faimilar with Node.js javascript development, so I have some things to learn. I'll report back if I have any meaningful things to contribute to docs.

[tim-lai]

@teodorlu While we wouldn't directly support Vite.js at this time, I would happy to merge in additional documentation for Vite.js users.

[char0n]

Hi guys,

I've looked into the Vite.js briefly and here are my observations:

• Vite is based on rullup
• Vite is also based on esbuild
• Vite allows to be configured to overcame the issues @teodorlu sees

It's just another build systems that does the same thing but in different way. @teodorlu I've invested couple of minutes trying to amend your vite config to support swagger-ui-react. Here is the PR that fixes your problems: https://github.com/teodorlu/swagger-ui-on-vite/pull/1

Note: IMHO we can close this PR. Every build system has it's own intricacies and it doesn't really make sense to try to compensate for all them in swagger-ui codebase.

[teodorlu]

I agree that Swagger UI needs to keep its scope narrow enough to maintain. Supporting everything in the world doesn't work.

Yet - I'd argue that removing dependencies is a good thing. If we don't really need the dependency on "buffer" and it causes problems, why keep it? From a security perspective, less dependencies means smaller attack surface, and fewer places for vulnerabilities to sneak in.

Caveat: I haven't worked through removing the dependency to uncover whether that produces more problems. Meaning that this PR isn't done (yet). Landing it and removing the dependency on "buffer" would probably require some effort from Swagger UI team members. And I'd guess they've got plenty of things to do!

[char0n]

Yet - I'd argue that removing dependencies is a good thing. If we don't really need the dependency on "buffer" and it causes problems, why keep it? From a security perspective, less dependencies means smaller attack surface, and fewer places for vulnerabilities to sneak in.

Sure, it's hard to oppose this argument - so I do agree. If we can replace vendor library with a native JavaScript alternative or replace the library with simple custom implementation (let's do it).

Caveat: I haven't worked through removing the dependency to uncover whether that produces more problems. Meaning that this PR isn't done (yet). Landing it and removing the dependency on "buffer" would probably require some effort from Swagger UI team members. And I'd guess they've got plenty of things to do!

Right, go ahead with your PR and let's see where we get.

[teodorlu]

@char0n - thanks for the response.

Possible further steps:

1. Find other "randombytes" call sites and propose alternatives
2. Try to build Swagger UI NPM packages - and see if we discover more problems.

I have a few other things I need to do as well at work - but I'll see what I can do. It would help us if we can remove some vulnerable Swagger UI dependencies, and fixing that upstream (here) is better for everyone.

[BeMoreDog]

Hi 🙂 We're getting the exact same problem in our Vue 2 webpack 5 app with versions newer than 4.5.2. Adding the buffer polyfill to the webpack config in vue.config.js doesn't fix the issue, so if you can remove it without issues, that'd be great!

[teodorlu]

Hi @BeMoreDog! I haven't had the time to move this further.

In the meantime, here's a snippet from our main.tsx which works around the issue. Perhaps a temporary fix for you too :)

// Polyfill the Node.js Buffer API for use in Swagger UI's transitive dependencies
import { Buffer } from "buffer";
declare global {
  interface Window {
    Buffer: any;
  }
}
window.Buffer = Buffer;

[char0n]

I've produces https://github.com/swagger-api/swagger-ui/pull/7946 that deals with Node.js polyfills and other problems in main SwaggerUI fragment. If you continue with this PR, please pay attention also to webpack config in the PR which needs to be compensated after dependency changes.