Skip to content
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

chore(swagger): Upgrade SpringFox and Swagger #827

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

luispollo
Copy link
Contributor

@luispollo luispollo commented Dec 8, 2020

Upgrades SpringFox, which brings with it a newer/better version of Swagger. Among other things, this fixes the forever-annoying issue with the Swagger UI not properly auto-scrolling and auto-expanding a linked API section.

I haven't been able to test this yet, hence the draft. Something in the updated dependency tree broke loading of one of keel's plugins. 😞 If anyone has some spare cycles and could try this with another service, it would be greatly appreciated.

Heads-up: the new version changes the Swagger UI path from /swagger-ui.html to /swagger-ui/index.html. There's no way to customize that as far as I've seen. Not sure how big of a deal that would be.

@@ -133,13 +129,13 @@ private BasePathProvider(String basePath, String documentationPath) {
}

@Override
protected String applicationPath() {
Copy link
Contributor Author

@luispollo luispollo Dec 8, 2020

Choose a reason for hiding this comment

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

They deprecated AbstractPathProvider in this version, and the docs are not great around path customization, so this is my current guess as to what needs to change, but needs testing. If anyone knows what effect basePath had in the original implementation, please let me know.


@EnableSwagger2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Following the (very light on detail) migration instructions here: https://github.com/springfox/springfox#migrating-from-existing-2x-version

@@ -127,7 +127,7 @@ dependencies {
api("io.mockk:mockk:1.10.0")
api("io.springfox:springfox-swagger-ui:${versions.springfoxSwagger}")
api("io.springfox:springfox-swagger2:${versions.springfoxSwagger}")
api("io.swagger:swagger-annotations:${versions.swagger}")
api("io.swagger.core.v3:swagger-annotations:${versions.swagger}")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This may break services using annotations, I'm not sure yet. Second thing to verify. SpringFox uses different deps for v2 and v3: https://github.com/springfox/springfox/blob/9722b7e3a41208bc3e7c0409aa3e63fc27a66f52/gradle/dependencies.gradle#L61-L68

@robzienert
Copy link
Member

This will need to be tested inside of Gate, too.

@bliu2368
Copy link

Any updates on this? Would solve a Cross-Scripting XSS attack vulnerability that is in kork's current version of swagger-ui https://snyk.io/vuln/maven:io.springfox%3Aspringfox-swagger-ui

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.

3 participants