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

examples/axum-utoipa-bindings: the swagger ui cannot be shown properly when loading /swagger-ui #1152

Closed
masnagam opened this issue Oct 19, 2024 · 5 comments · Fixed by #1155

Comments

@masnagam
Copy link
Contributor

masnagam commented Oct 19, 2024

before a985d8c, the swagger ui can be shown when loading /swagger-ui.
the request for /swagger-ui is redirected to /swagger-ui/ and the assets of the swagger ui are loaded properly.

a985d8c breaks this behavior and the request for /swagger-ui is not redirected and urls of the assets are wrong.
because the url of the swagger ui is /swagger-ui and relative urls of the assets are resolved with /.

i think that it's better to revert a985d8c on the master branch. then fix #1042 properly.

the simplest solution is panic!() when SwaggerUi::new() is called with /.
i think that this is acceptable for most people because mounting the swagger ui to / is rare.

another solution is adding the route for redirect only when the path does not end with /.

diff --git a/utoipa-swagger-ui/src/axum.rs b/utoipa-swagger-ui/src/axum.rs
index 22a3179..7b76c7a 100644
--- a/utoipa-swagger-ui/src/axum.rs
+++ b/utoipa-swagger-ui/src/axum.rs
@@ -45,10 +45,20 @@ where
         let handler = routing::get(serve_swagger_ui).layer(Extension(Arc::new(config)));
         let path: &str = swagger_ui.path.as_ref();

-        router
-            .route(path, handler.clone())
-            .route(&format!("{}/", path), handler.clone())
-            .route(&format!("{}/*rest", path), handler)
+        if path.ends_with('/') {
+            router
+                .route(path, handler.clone())
+                .route(&format!("{}*rest", path), handler)
+        } else {
+            let slash_path = format!("{}/", path);
+            router
+                .route(
+                    path,
+                    routing::get(|| async move { axum::response::Redirect::to(&slash_path) }),
+                )
+                .route(&format!("{}/", path), handler.clone())
+                .route(&format!("{}/*rest", path), handler)
+        }
     }
 }

in this case, a request for /swagger-ui won't be redirected if SwaggerUi::new() is called with /swagger-ui/.
but i think that this is what the code is intended to do.

there is another solution.
currently, assets are specified with relative urls.
so, using absolute urls for the assets in the html document of the swagger ui can solve this issue without the redirect.
but, this kind of change affects servers other than axum and you may have to pay more cost for testing than other solutions.

i prefer the second solution, but you can choose one of any solutions.

@shunkakinoki
Copy link

+1 having the same issue here

@shunkakinoki
Copy link

Added a quick revert here #1153

@shunkakinoki
Copy link

Can confirm the issue of the assets being not /swagger-ui/ but from / is fixed - would love to know how to proceed w/ this!

@juhaku
Copy link
Owner

juhaku commented Oct 20, 2024

Right, I'd then just fix this directly, because reverting back to the original, is not feasible either it will again break the behavior the original commit was meant to fix.

another solution is adding the route for redirect only when the path does not end with /.

I would be interested to see whether this would fix the issue for both use cases.

@masnagam
Copy link
Contributor Author

masnagam commented Oct 21, 2024

i've created a PR (#1155) based on the patch in my previous comment.
in this PR, the limitation described in my previous comment has been solved.
i've confirmed that ./scripts/test.sh utoipa-swagger-ui passes.

without my change, mount_onto_path_ends_with_slash() and mount_onto_path_not_end_with_slash() fail due to this issue.

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 a pull request may close this issue.

3 participants