feat(server/v2): Add Swagger UI support for server/v2 #23092
Conversation
crStiv
requested review from
julienrbrt,
hieuvubk,
kocubinski and
a team
as code owners
📝 WalkthroughWalkthroughThe pull request introduces a new Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
C:server/v2
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
server/v2/api/swagger/config.go(1 hunks)server/v2/api/swagger/handler.go(1 hunks)server/v2/server.go(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
server/v2/api/swagger/handler.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
server/v2/api/swagger/config.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
server/v2/server.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🪛 golangci-lint (1.62.2)
server/v2/api/swagger/handler.go
44-44: undefined: time
(typecheck)
server/v2/api/swagger/config.go
24-24: undefined: fmt
(typecheck)
server/v2/server.go
78-78: undefined: http
(typecheck)
88-88: undefined: http
(typecheck)
251-251: s.config.API undefined (type ServerConfig has no field or method API)
(typecheck)
🔇 Additional comments (5)
server/v2/api/swagger/config.go (2)
5-11: Struct fields look good
Your Config struct is straightforward and clearly documents each field. This aligns well with Go style guidelines.
13-19: Clarity in default values
Returning Enable: false and Path: "/swagger" as defaults appears consistent with the code’s intended usage. This ensures that Swagger UI is turned off by default unless explicitly enabled.
server/v2/api/swagger/handler.go (1)
12-46: Handler logic and MIME handling
Your approach to handle file serving, MIME type detection, and fallback to index.html is solid. The usage of path.Clean is sensible for preventing tricky path manipulations. Once the time package is imported, http.ServeContent is a good fit for robust file serving.
🧰 Tools
🪛 golangci-lint (1.62.2)
44-44: undefined: time
(typecheck)
server/v2/server.go (2)
Line range hint 78-88: HTTP router usage
Defining router *http.ServeMux and assigning it with http.NewServeMux() is sensible for isolating routes. Confirm the design pattern for hooking in additional routes from other components if needed.
🧰 Tools
🪛 golangci-lint (1.62.2)
78-78: undefined: http
(typecheck)
249-263: Potential config mismatch
Line 251 references s.config.API.Swagger, but the static analysis suggests that ServerConfig has no field API. Verify your ServerConfig type to ensure it holds the API field or reference the correct nested structure.
🧰 Tools
🪛 golangci-lint (1.62.2)
251-251: s.config.API undefined (type ServerConfig has no field or method API)
(typecheck)
server/v2/api/swagger/config.go
Outdated
| // Validate validates the configuration | ||
| func (c Config) Validate() error { | ||
| if c.Path == "" { | ||
| return fmt.Errorf("swagger path cannot be empty") | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Ensure error formatting is enabled
While the validation logic is correct, remember that fmt.Errorf("swagger path cannot be empty") requires "fmt" to be imported (as noted above). Once the import is added, this code works as intended.
🧰 Tools
🪛 golangci-lint (1.62.2)
24-24: undefined: fmt
(typecheck)
server/v2/api/swagger/config.go
Outdated
| package swagger | ||
|
|
||
| import "github.com/cosmos/cosmos-sdk/server/v2/config" | ||
|
|
There was a problem hiding this comment.
Package import oversight
Currently, you're importing "github.com/cosmos/cosmos-sdk/server/v2/config" but using fmt.Errorf in the code without importing "fmt". There's no direct usage of the imported "github.com/cosmos/cosmos-sdk/server/v2/config" in this snippet. If it's needed in other parts of the file, that's fine. Otherwise, consider adding the missing "fmt" import or removing unused imports.
package swagger
-import "github.com/cosmos/cosmos-sdk/server/v2/config"
+import (
+ "fmt"
+ "github.com/cosmos/cosmos-sdk/server/v2/config"
+)
Committable suggestion skipped: line range outside the PR's diff.
server/v2/api/swagger/handler.go
Outdated
| import ( | ||
| "net/http" | ||
| "path" | ||
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "github.com/rakyll/statik/fs" | ||
| ) |
There was a problem hiding this comment.
Missing time import
You're calling time.Time{} in this file but haven't imported "time". Add the missing import to avoid compilation issues.
import (
"net/http"
"path"
"path/filepath"
"strings"
"github.com/rakyll/statik/fs"
+ "time"
)Committable suggestion skipped: line range outside the PR's diff.
server/v2/server.go
Outdated
| "github.com/cosmos/cosmos-sdk/server/v2/api/swagger" | ||
| "github.com/pelletier/go-toml/v2" | ||
| "github.com/spf13/cobra" | ||
| "github.com/spf13/pflag" | ||
|
|
||
| "cosmossdk.io/core/transaction" | ||
| "cosmossdk.io/log" | ||
| "github.com/rakyll/statik/fs" |
There was a problem hiding this comment.
Check for missing imports
You reference swagger and statik/fs but also rely on http.ServeMux later (lines 78, 88) without importing "net/http". Ensure all required imports are present to avoid compilation errors.
import (
"context"
"errors"
"fmt"
"os"
"path/filepath"
"strings"
+ "net/http"
"github.com/cosmos/cosmos-sdk/server/v2/api/swagger"
"github.com/pelletier/go-toml/v2"
"github.com/spf13/cobra"Committable suggestion skipped: line range outside the PR's diff.
server/v2/server.go
Outdated
There was a problem hiding this comment.
Hi, thanks for tackling this. The wiring shouldn't be done in the main server, but api/swagger should be its own server component. Check out the other servers.
There was a problem hiding this comment.
Hi, thanks for tackling this. The wiring shouldn't be done in the main server, but api/swagger should be its own server component. Check out the other servers.
oh you're absolutely right, gonna fix it soon!
There was a problem hiding this comment.
@julienrbrt sorry for mess in commits, I had to redo some things but now everything should be fine
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
server/v2/api/swagger/config.go (1)
4-4: Remove unused imports.The import
"cosmossdk.io/core/server"is not needed and can be safely removed to adhere to Go’s best practices on unused imports.Here’s a suggested fix:
import ( - "cosmossdk.io/core/server" )🧰 Tools
🪛 golangci-lint (1.62.2)
4-4: "cosmossdk.io/core/server" imported and not used
(typecheck)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
server/v2/api/swagger/config.go(1 hunks)server/v2/api/swagger/server.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
server/v2/api/swagger/config.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
server/v2/api/swagger/server.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🪛 golangci-lint (1.62.2)
server/v2/api/swagger/config.go
4-4: "cosmossdk.io/core/server" imported and not used
(typecheck)
server/v2/api/swagger/server.go
31-31: srv.Config undefined (type *Server[T] has no field or method Config, but does have field config)
(typecheck)
40-40: undefined: NewSwaggerHandler
(typecheck)
server/v2/api/swagger/server.go
Outdated
| srv.config = serverCfg | ||
|
|
||
| mux := http.NewServeMux() | ||
| mux.Handle(srv.config.Path, NewSwaggerHandler()) |
There was a problem hiding this comment.
Handle the undefined NewSwaggerHandler reference.
The function NewSwaggerHandler() is not defined in the reviewed code, nor is it imported from any package. Ensure that NewSwaggerHandler, or whichever handler function you’re calling, is properly defined and imported to prevent a runtime error or undefined symbol.
Would you like assistance creating a NewSwaggerHandler() function?
🧰 Tools
🪛 golangci-lint (1.62.2)
40-40: undefined: NewSwaggerHandler
(typecheck)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| w.Header().Set("Last-Modified", time.Now().UTC().Format(http.TimeFormat)) | ||
|
|
||
| // Serve the file | ||
| http.ServeContent(w, r, urlPath, time.Now(), file.(io.ReadSeeker)) |
Check warning
Code scanning / CodeQL
Calling the system time Warning
julienrbrt
changed the title
server/v2/api/swagger/handler.go
Outdated
| ) | ||
|
|
||
| // Handler returns an HTTP handler for Swagger UI | ||
| func Handler() http.Handler { |
There was a problem hiding this comment.
nit: let's not expose this and just use swaggerHandler{} in the handler registration
There was a problem hiding this comment.
The swagger ui is passed nowhere, so this server will actually do nothing.
We should let the user pass its chain swagger ui / file at server instantiation.
For instance, a user could pass the default SDK swagger config (docs.SwaggerUI):
Line 19 in b6150ec
|
Additionally, let's wire this server in simapp/v2 so we can test it out and be shown to users how it is wired. |
@julienrbrt hey Julien, I've added some changes, is it what you asked for? |
Yes great, if you can create a doc.go showing how to wire this server/v2 and update simapp/v2 to wire it, then it looks good to me. |
|
@julienrbrt made an update, is this doc pretty clear to understand? |
|
@julienrbrt like this? |
|
Yes, like that great work! Could you update the doc.go again? For the handler I think we can remove some headers, they aren't all necessary, let's keep the strict minimum. |
|
Additionally can you run |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
server/v2/api/swagger/handler.go (1)
52-72: Consider using mime.TypeByExtension for more robust content type detection.While the current implementation covers common file types, using the standard library's mime package would provide more comprehensive coverage.
// getContentType returns the content-type for a file extension func getContentType(ext string) string { + if mimeType := mime.TypeByExtension(ext); mimeType != "" { + return mimeType + } + // Fallback to current implementation for specific overrides switch strings.ToLower(ext) { case ".html": return "text/html"
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
server/v2/api/swagger/handler.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
server/v2/api/swagger/handler.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (2)
server/v2/api/swagger/handler.go (2)
1-10: LGTM! Package and imports are well-organized.The package name follows Go conventions, and all imports are necessary and properly used.
11-13: Keep the handler struct unexported.The struct is correctly unexported as
swaggerHandler, following the previous review suggestion.
server/v2/api/swagger/handler.go
Outdated
| // Process the path | ||
| urlPath := strings.TrimPrefix(r.URL.Path, "/swagger") | ||
| if urlPath == "" || urlPath == "/" { | ||
| urlPath = "/index.html" | ||
| } | ||
|
|
||
| // Open the file | ||
| file, err := h.swaggerFS.Open(urlPath) | ||
| if err != nil { | ||
| http.Error(w, "File not found", http.StatusNotFound) | ||
| return | ||
| } |
There was a problem hiding this comment.
Add path sanitization to prevent directory traversal.
The current path handling could be vulnerable to directory traversal attacks. Consider using filepath.Clean and additional validation.
// Process the path
urlPath := strings.TrimPrefix(r.URL.Path, "/swagger")
+urlPath = filepath.Clean(urlPath)
+if strings.Contains(urlPath, "..") {
+ http.Error(w, "Invalid path", http.StatusBadRequest)
+ return
+}
if urlPath == "" || urlPath == "/" {
urlPath = "/index.html"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Process the path | |
| urlPath := strings.TrimPrefix(r.URL.Path, "/swagger") | |
| if urlPath == "" || urlPath == "/" { | |
| urlPath = "/index.html" | |
| } | |
| // Open the file | |
| file, err := h.swaggerFS.Open(urlPath) | |
| if err != nil { | |
| http.Error(w, "File not found", http.StatusNotFound) | |
| return | |
| } | |
| // Process the path | |
| urlPath := strings.TrimPrefix(r.URL.Path, "/swagger") | |
| urlPath = filepath.Clean(urlPath) | |
| if strings.Contains(urlPath, "..") { | |
| http.Error(w, "Invalid path", http.StatusBadRequest) | |
| return | |
| } | |
| if urlPath == "" || urlPath == "/" { | |
| urlPath = "/index.html" | |
| } | |
| // Open the file | |
| file, err := h.swaggerFS.Open(urlPath) | |
| if err != nil { | |
| http.Error(w, "File not found", http.StatusNotFound) | |
| return | |
| } |
server/v2/api/swagger/handler.go
Outdated
| // Set caching headers | ||
| w.Header().Set("Cache-Control", "public, max-age=31536000") | ||
| w.Header().Set("Last-Modified", time.Now().UTC().Format(http.TimeFormat)) | ||
|
|
||
| // Serve the file | ||
| http.ServeContent(w, r, urlPath, time.Now(), file.(io.ReadSeeker)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize caching implementation and add security headers.
- Use a constant time for Last-Modified to ensure deterministic behavior
- Add security headers to protect against common web vulnerabilities
// Set caching headers
w.Header().Set("Cache-Control", "public, max-age=31536000")
-w.Header().Set("Last-Modified", time.Now().UTC().Format(http.TimeFormat))
+// Use a fixed timestamp for deterministic behavior
+w.Header().Set("Last-Modified", "Mon, 01 Jan 2024 00:00:00 GMT")
+
+// Add security headers
+w.Header().Set("X-Content-Type-Options", "nosniff")
+w.Header().Set("X-Frame-Options", "DENY")
+w.Header().Set("Content-Security-Policy", "default-src 'self'")
-http.ServeContent(w, r, urlPath, time.Now(), file.(io.ReadSeeker))
+http.ServeContent(w, r, urlPath, time.Unix(0, 0), file.(io.ReadSeeker))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Set caching headers | |
| w.Header().Set("Cache-Control", "public, max-age=31536000") | |
| w.Header().Set("Last-Modified", time.Now().UTC().Format(http.TimeFormat)) | |
| // Serve the file | |
| http.ServeContent(w, r, urlPath, time.Now(), file.(io.ReadSeeker)) | |
| // Set caching headers | |
| w.Header().Set("Cache-Control", "public, max-age=31536000") | |
| // Use a fixed timestamp for deterministic behavior | |
| w.Header().Set("Last-Modified", "Mon, 01 Jan 2024 00:00:00 GMT") | |
| // Add security headers | |
| w.Header().Set("X-Content-Type-Options", "nosniff") | |
| w.Header().Set("X-Frame-Options", "DENY") | |
| w.Header().Set("Content-Security-Policy", "default-src 'self'") | |
| // Serve the file | |
| http.ServeContent(w, r, urlPath, time.Unix(0, 0), file.(io.ReadSeeker)) |
|
@julienrbrt I've made things:
If anything else, I'm open to do it |
|
still checks are red |
| @@ -163,6 +166,18 @@ func InitRootCmd[T transaction.Tx]( | |||
| } | |||
| registerGRPCGatewayRoutes[T](deps, grpcgatewayServer) | |||
|
|
|||
| // Create Swagger server | |||
| swaggerServer, err := swaggerv2.New[T]( | |||
| logger.With(log.ModuleKey, "swagger"), | |||
There was a problem hiding this comment.
We can just pass the litter here, the key is added in the constrictor
simapp/v2/simdv2/cmd/commands.go
Outdated
| @@ -38,6 +38,8 @@ import ( | |||
| genutilcli "github.com/cosmos/cosmos-sdk/x/genutil/client/cli" | |||
| genutiltypes "github.com/cosmos/cosmos-sdk/x/genutil/types" | |||
| v2 "github.com/cosmos/cosmos-sdk/x/genutil/v2/cli" | |||
| "cosmossdk.io/client/docs" | |||
There was a problem hiding this comment.
This import doesn't exist and the ordering is wrong (Make lint-fix should have fixed that.)
|
|
||
| // Create Swagger server | ||
| swaggerServer, err := swaggerv2.New[T]( | ||
| logger.With(log.ModuleKey, "swagger"), |
server/v2/api/swagger/config.go
Outdated
| return &Config{ | ||
| Enable: true, | ||
| Address: "localhost:8080", | ||
| Path: "/swagger/", |
There was a problem hiding this comment.
Why do we need a path? I think keeping /swagger by default is great.
server/v2/api/swagger/config.go
Outdated
|
|
||
| // Config defines the configuration for the Swagger UI server | ||
| type Config struct { | ||
| Enable bool `toml:"enable" mapstructure:"enable"` |
There was a problem hiding this comment.
Let's add toml comment annotation
server/v2/api/swagger/config.go
Outdated
| func DefaultConfig() *Config { | ||
| return &Config{ | ||
| Enable: true, | ||
| Address: "localhost:8080", |
There was a problem hiding this comment.
Let's use something else than 8080 by default
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
server/v2/api/swagger/config.go (2)
3-8: Remove unused importThe
"cosmossdk.io/core/server"package is imported but not used in this file.import ( "fmt" "net/http" - - "cosmossdk.io/core/server" )
12-20: Enhance struct documentationWhile the struct is well-documented, consider adding more details about:
- Expected format for the Address field (e.g., "host:port")
- Default values
- Usage examples
server/v2/api/swagger/handler.go (2)
24-38: Enhance path validationWhile the current path validation is good, consider adding:
- Maximum path length check
- Validation for allowed characters
// Process and validate the path urlPath := strings.TrimPrefix(r.URL.Path, "/swagger") + +// Check path length +if len(urlPath) > 255 { + http.Error(w, "Path too long", http.StatusBadRequest) + return +} + +// Check for allowed characters +if strings.ContainsAny(urlPath, "<>:\"\\|?*") { + http.Error(w, "Invalid characters in path", http.StatusBadRequest) + return +}
62-81: Add default content typeConsider adding a default content type for unknown extensions to ensure proper content handling.
func getContentType(ext string) string { switch strings.ToLower(ext) { // ... existing cases ... default: - return "" + return "application/octet-stream" } }
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
server/v2/api/swagger/config.go(1 hunks)server/v2/api/swagger/doc.go(1 hunks)server/v2/api/swagger/handler.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- server/v2/api/swagger/doc.go
🧰 Additional context used
📓 Path-based instructions (2)
server/v2/api/swagger/config.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
server/v2/api/swagger/handler.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🪛 golangci-lint (1.62.2)
server/v2/api/swagger/config.go
1-1: : # cosmossdk.io/server/v2/api/swagger
api/swagger/server.go:51:27: srv.config.Path undefined (type *Config has no field or method Path)
api/swagger/config.go:7:5: "cosmossdk.io/core/server" imported and not used
(typecheck)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (5)
server/v2/api/swagger/config.go (2)
22-28: LGTM! Good choice of default portThe default configuration uses port 8090, which is a good choice as it avoids conflicts with commonly used ports.
30-41: LGTM! Proper validation logicThe validation logic correctly checks for required fields when Swagger UI is enabled.
server/v2/api/swagger/handler.go (3)
11-13: LGTM! Clean handler implementationThe handler struct is minimal and follows good design practices.
53-58:⚠️ Potential issueUse deterministic timestamps
Using
time.Now()can lead to non-deterministic behavior. Consider using a fixed timestamp for caching headers.// Set caching headers w.Header().Set("Cache-Control", "public, max-age=31536000") -w.Header().Set("Last-Modified", time.Now().UTC().Format(http.TimeFormat)) +// Use a fixed timestamp for deterministic behavior +w.Header().Set("Last-Modified", "Mon, 01 Jan 2024 00:00:00 GMT") -http.ServeContent(w, r, urlPath, time.Now(), file.(io.ReadSeeker)) +http.ServeContent(w, r, urlPath, time.Unix(0, 0), file.(io.ReadSeeker))Likely invalid or redundant comment.
16-22: 🛠️ Refactor suggestionStrengthen CORS security
The current CORS configuration is too permissive for production use. Consider:
- Restricting allowed origins
- Adding additional security headers
// Set minimal CORS headers -w.Header().Set("Access-Control-Allow-Origin", "*") +w.Header().Set("Access-Control-Allow-Origin", r.Header.Get("Origin")) w.Header().Set("Access-Control-Allow-Methods", "GET") +w.Header().Set("X-Content-Type-Options", "nosniff") +w.Header().Set("X-Frame-Options", "DENY")Likely invalid or redundant comment.
|
@julienrbrt I ran a linter, there are 0 problems but there is a text in terminal like this linting module cosmossdk.io/server/v2/stf [2025-01-07T17:19:55+00:00] |
|
Hey, this is because of the build failure. You need to make sure that the app builds. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
server/v2/api/swagger/handler.go (2)
47-54: Add caching headers and use deterministic timestamps.The current implementation:
- Lacks caching headers for static files.
- Uses non-deterministic
time.Now()which could cause issues in tests.Apply this diff:
// Set the content-type ext := filepath.Ext(urlPath) if ct := getContentType(ext); ct != "" { w.Header().Set("Content-Type", ct) } + // Add caching headers for static files + w.Header().Set("Cache-Control", "public, max-age=3600") + w.Header().Set("ETag", fmt.Sprintf("W/\"%x\"", sha256.Sum256([]byte(urlPath)))) + // Serve the file - http.ServeContent(w, r, urlPath, time.Now(), file.(io.ReadSeeker)) + http.ServeContent(w, r, urlPath, time.Unix(0, 0), file.(io.ReadSeeker))
58-77: Enhance content type detection.Consider these improvements:
- Use
mime.TypeByExtensionas a fallback for unknown extensions.- Add support for additional Swagger UI file types (yaml, ico).
Apply this diff:
+import "mime" func getContentType(ext string) string { - switch strings.ToLower(ext) { - case ".html": - return "text/html" - case ".css": - return "text/css" - case ".js": - return "application/javascript" - case ".json": - return "application/json" - case ".png": - return "image/png" - case ".jpg", ".jpeg": - return "image/jpeg" - case ".svg": - return "image/svg+xml" - default: - return "" + // Handle known types first + knownTypes := map[string]string{ + ".html": "text/html", + ".css": "text/css", + ".js": "application/javascript", + ".json": "application/json", + ".yaml": "application/yaml", + ".yml": "application/yaml", + ".png": "image/png", + ".jpg": "image/jpeg", + ".jpeg": "image/jpeg", + ".svg": "image/svg+xml", + ".ico": "image/x-icon", } + + if ct, ok := knownTypes[strings.ToLower(ext)]; ok { + return ct + } + + // Fallback to standard mime package + if ct := mime.TypeByExtension(ext); ct != "" { + return ct + } + + return "" }simapp/v2/simdv2/cmd/commands.go (2)
18-18: Fix import ordering.The import statement for
swaggerpackage is causing linting issues.Apply this diff to fix the import ordering:
- swaggerv2 "cosmossdk.io/server/v2/api/swagger" "cosmossdk.io/server/v2/api/telemetry" "cosmossdk.io/server/v2/cometbft" serverstore "cosmossdk.io/server/v2/store" + swaggerv2 "cosmossdk.io/server/v2/api/swagger"🧰 Tools
🪛 golangci-lint (1.62.2)
18-18: could not import cosmossdk.io/server/v2/api/swagger (-: # cosmossdk.io/server/v2/api/swagger
../../server/v2/api/swagger/config.go:7:5: "cosmossdk.io/core/server" imported and not used)(typecheck)
169-175: Simplify logger creation.The logger key is already added in the constructor, no need to add it here.
Apply this diff:
- swaggerServer, err := swaggerv2.New[T]( - logger.With(log.ModuleKey, "swagger"), - deps.GlobalConfig, - ) + swaggerServer, err := swaggerv2.New[T](logger, deps.GlobalConfig)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
server/v2/api/swagger/doc.go(1 hunks)server/v2/api/swagger/handler.go(1 hunks)server/v2/api/swagger/server.go(1 hunks)simapp/v2/app.go(1 hunks)simapp/v2/simdv2/cmd/commands.go(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- server/v2/api/swagger/doc.go
- server/v2/api/swagger/server.go
🧰 Additional context used
📓 Path-based instructions (3)
server/v2/api/swagger/handler.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
simapp/v2/app.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
simapp/v2/simdv2/cmd/commands.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🪛 golangci-lint (1.62.2)
simapp/v2/simdv2/cmd/commands.go
18-18: could not import cosmossdk.io/server/v2/api/swagger (-: # cosmossdk.io/server/v2/api/swagger
../../server/v2/api/swagger/config.go:7:5: "cosmossdk.io/core/server" imported and not used)
(typecheck)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (1)
server/v2/api/swagger/handler.go (1)
11-13: LGTM!The struct design follows good encapsulation principles by being unexported and using the appropriate
http.FileSysteminterface for serving static files.
server/v2/api/swagger/handler.go
Outdated
| func (h *swaggerHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { | ||
| // Set minimal CORS headers | ||
| w.Header().Set("Access-Control-Allow-Origin", "*") | ||
| w.Header().Set("Access-Control-Allow-Methods", "GET") | ||
|
|
||
| if r.Method == http.MethodOptions { | ||
| return | ||
| } | ||
|
|
||
| // Process and validate the path | ||
| urlPath := strings.TrimPrefix(r.URL.Path, "/swagger") | ||
| if urlPath == "" || urlPath == "/" { | ||
| urlPath = "/index.html" | ||
| } | ||
|
|
||
| // Basic path validation | ||
| if strings.Contains(urlPath, "..") || strings.Contains(urlPath, "//") { | ||
| http.Error(w, "Invalid path", http.StatusBadRequest) | ||
| return | ||
| } | ||
|
|
||
| // Clean the path | ||
| urlPath = filepath.Clean(urlPath) | ||
|
|
||
| // Open the file | ||
| file, err := h.swaggerFS.Open(urlPath) | ||
| if err != nil { | ||
| http.Error(w, "File not found", http.StatusNotFound) | ||
| return | ||
| } | ||
| defer file.Close() | ||
|
|
||
| // Set the content-type | ||
| ext := filepath.Ext(urlPath) | ||
| if ct := getContentType(ext); ct != "" { | ||
| w.Header().Set("Content-Type", ct) | ||
| } | ||
|
|
||
| // Serve the file | ||
| http.ServeContent(w, r, urlPath, time.Now(), file.(io.ReadSeeker)) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Strengthen path validation and add security headers.
While basic path validation exists, it could be enhanced:
- Use
filepath.Cleanbefore validation to prevent bypass attempts. - Add security headers to protect against common web vulnerabilities.
Apply this diff:
func (h *swaggerHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// Set minimal CORS headers
w.Header().Set("Access-Control-Allow-Origin", "*")
w.Header().Set("Access-Control-Allow-Methods", "GET")
+ // Add security headers
+ w.Header().Set("X-Content-Type-Options", "nosniff")
+ w.Header().Set("X-Frame-Options", "DENY")
+ w.Header().Set("Content-Security-Policy", "default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'")
if r.Method == http.MethodOptions {
return
}
// Process and validate the path
urlPath := strings.TrimPrefix(r.URL.Path, "/swagger")
+ urlPath = filepath.Clean(urlPath)
+
+ // Validate path before any operations
+ if strings.Contains(urlPath, "..") || strings.Contains(urlPath, "//") || strings.Contains(urlPath, "\x00") {
http.Error(w, "Invalid path", http.StatusBadRequest)
return
}
-
- // Clean the path
- urlPath = filepath.Clean(urlPath)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (h *swaggerHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |
| // Set minimal CORS headers | |
| w.Header().Set("Access-Control-Allow-Origin", "*") | |
| w.Header().Set("Access-Control-Allow-Methods", "GET") | |
| if r.Method == http.MethodOptions { | |
| return | |
| } | |
| // Process and validate the path | |
| urlPath := strings.TrimPrefix(r.URL.Path, "/swagger") | |
| if urlPath == "" || urlPath == "/" { | |
| urlPath = "/index.html" | |
| } | |
| // Basic path validation | |
| if strings.Contains(urlPath, "..") || strings.Contains(urlPath, "//") { | |
| http.Error(w, "Invalid path", http.StatusBadRequest) | |
| return | |
| } | |
| // Clean the path | |
| urlPath = filepath.Clean(urlPath) | |
| // Open the file | |
| file, err := h.swaggerFS.Open(urlPath) | |
| if err != nil { | |
| http.Error(w, "File not found", http.StatusNotFound) | |
| return | |
| } | |
| defer file.Close() | |
| // Set the content-type | |
| ext := filepath.Ext(urlPath) | |
| if ct := getContentType(ext); ct != "" { | |
| w.Header().Set("Content-Type", ct) | |
| } | |
| // Serve the file | |
| http.ServeContent(w, r, urlPath, time.Now(), file.(io.ReadSeeker)) | |
| } | |
| func (h *swaggerHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { | |
| // Set minimal CORS headers | |
| w.Header().Set("Access-Control-Allow-Origin", "*") | |
| w.Header().Set("Access-Control-Allow-Methods", "GET") | |
| // Add security headers | |
| w.Header().Set("X-Content-Type-Options", "nosniff") | |
| w.Header().Set("X-Frame-Options", "DENY") | |
| w.Header().Set("Content-Security-Policy", "default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'") | |
| if r.Method == http.MethodOptions { | |
| return | |
| } | |
| // Process and validate the path | |
| urlPath := strings.TrimPrefix(r.URL.Path, "/swagger") | |
| urlPath = filepath.Clean(urlPath) | |
| // Validate path before any operations | |
| if strings.Contains(urlPath, "..") || strings.Contains(urlPath, "//") || strings.Contains(urlPath, "\x00") { | |
| http.Error(w, "Invalid path", http.StatusBadRequest) | |
| return | |
| } | |
| if urlPath == "" || urlPath == "/" { | |
| urlPath = "/index.html" | |
| } | |
| // Open the file | |
| file, err := h.swaggerFS.Open(urlPath) | |
| if err != nil { | |
| http.Error(w, "File not found", http.StatusNotFound) | |
| return | |
| } | |
| defer file.Close() | |
| // Set the content-type | |
| ext := filepath.Ext(urlPath) | |
| if ct := getContentType(ext); ct != "" { | |
| w.Header().Set("Content-Type", ct) | |
| } | |
| // Serve the file | |
| http.ServeContent(w, r, urlPath, time.Now(), file.(io.ReadSeeker)) | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
server/v2/api/swagger/handler.go (2)
29-42: Strengthen path validation and improve error messaging.While the path validation is good, it could be enhanced:
- Add validation for null bytes and other malicious characters
- Provide more descriptive error messages
- Consider logging invalid path attempts
// Process and validate the path urlPath := strings.TrimPrefix(r.URL.Path, "/swagger") + +// Log the requested path for debugging +h.logger.Debug("swagger request path", "path", urlPath) + if urlPath == "" || urlPath == "/" { urlPath = "/index.html" } // Clean the path before validation urlPath = filepath.Clean(urlPath) // Validate path before any operations -if strings.Contains(urlPath, "..") || strings.Contains(urlPath, "//") || strings.Contains(urlPath, "\\") { - http.Error(w, "Invalid path", http.StatusBadRequest) +if strings.Contains(urlPath, "..") || strings.Contains(urlPath, "//") || + strings.Contains(urlPath, "\\") || strings.Contains(urlPath, "\x00") { + msg := "Invalid path: contains forbidden characters or sequences" + h.logger.Warn("invalid swagger path attempt", "path", urlPath) + http.Error(w, msg, http.StatusBadRequest) return }
62-82: Enhance content type mapping.Consider adding support for additional common file types and using standard MIME types:
func getContentType(ext string) string { switch strings.ToLower(ext) { case ".html": - return "text/html" + return "text/html; charset=utf-8" case ".css": - return "text/css" + return "text/css; charset=utf-8" case ".js": - return "application/javascript" + return "application/javascript; charset=utf-8" case ".json": - return "application/json" + return "application/json; charset=utf-8" case ".png": return "image/png" case ".jpg", ".jpeg": return "image/jpeg" case ".svg": - return "image/svg+xml" + return "image/svg+xml; charset=utf-8" + case ".ico": + return "image/x-icon" + case ".woff": + return "font/woff" + case ".woff2": + return "font/woff2" default: return "" } }server/v2/api/swagger/server.go (1)
71-81: Improve configuration handling with validation.The configuration retrieval should include validation when creating a new config:
func (s *Server[T]) Config() any { if s.config == nil { cfg := DefaultConfig() for _, opt := range s.cfgOptions { opt(cfg) } + // Validate the configuration + if err := cfg.Validate(); err != nil { + s.logger.Error("invalid swagger configuration", "error", err) + return DefaultConfig() + } return cfg } return s.config }
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
server/v2/api/swagger/handler.go(1 hunks)server/v2/api/swagger/server.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
server/v2/api/swagger/server.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
server/v2/api/swagger/handler.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
| // Set minimal CORS headers | ||
| w.Header().Set("Access-Control-Allow-Origin", "*") | ||
| w.Header().Set("Access-Control-Allow-Methods", "GET") | ||
|
|
||
| // Add security headers | ||
| w.Header().Set("X-Content-Type-Options", "nosniff") | ||
| w.Header().Set("X-Frame-Options", "DENY") | ||
| w.Header().Set("Content-Security-Policy", "default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance CORS and security headers configuration.
The current CORS configuration is too permissive. Consider:
- Restricting CORS to specific origins instead of "*"
- Adding additional security headers for better protection
// Set minimal CORS headers
-w.Header().Set("Access-Control-Allow-Origin", "*")
+// TODO: Make this configurable via Config struct
+w.Header().Set("Access-Control-Allow-Origin", "https://api.example.com")
w.Header().Set("Access-Control-Allow-Methods", "GET")
+w.Header().Set("Access-Control-Allow-Headers", "Content-Type")
// Add security headers
w.Header().Set("X-Content-Type-Options", "nosniff")
w.Header().Set("X-Frame-Options", "DENY")
w.Header().Set("Content-Security-Policy", "default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'")
+w.Header().Set("Strict-Transport-Security", "max-age=31536000; includeSubDomains")
+w.Header().Set("Referrer-Policy", "strict-origin-when-cross-origin")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Set minimal CORS headers | |
| w.Header().Set("Access-Control-Allow-Origin", "*") | |
| w.Header().Set("Access-Control-Allow-Methods", "GET") | |
| // Add security headers | |
| w.Header().Set("X-Content-Type-Options", "nosniff") | |
| w.Header().Set("X-Frame-Options", "DENY") | |
| w.Header().Set("Content-Security-Policy", "default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'") | |
| // Set minimal CORS headers | |
| // TODO: Make this configurable via Config struct | |
| w.Header().Set("Access-Control-Allow-Origin", "https://api.example.com") | |
| w.Header().Set("Access-Control-Allow-Methods", "GET") | |
| w.Header().Set("Access-Control-Allow-Headers", "Content-Type") | |
| // Add security headers | |
| w.Header().Set("X-Content-Type-Options", "nosniff") | |
| w.Header().Set("X-Frame-Options", "DENY") | |
| w.Header().Set("Content-Security-Policy", "default-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'") | |
| w.Header().Set("Strict-Transport-Security", "max-age=31536000; includeSubDomains") | |
| w.Header().Set("Referrer-Policy", "strict-origin-when-cross-origin") |
| // Open the file | ||
| file, err := h.swaggerFS.Open(urlPath) | ||
| if err != nil { | ||
| http.Error(w, "File not found", http.StatusNotFound) | ||
| return | ||
| } | ||
| defer file.Close() | ||
|
|
||
| // Set the content-type | ||
| ext := filepath.Ext(urlPath) | ||
| if ct := getContentType(ext); ct != "" { | ||
| w.Header().Set("Content-Type", ct) | ||
| } | ||
|
|
||
| // Serve the file | ||
| http.ServeContent(w, r, urlPath, time.Now(), file.(io.ReadSeeker)) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve file serving and error handling.
The current implementation has two issues:
- Using
time.Now()inServeContentis non-deterministic - Error handling could be more informative
// Open the file
file, err := h.swaggerFS.Open(urlPath)
if err != nil {
- http.Error(w, "File not found", http.StatusNotFound)
+ msg := fmt.Sprintf("Swagger file not found: %s", urlPath)
+ h.logger.Debug("swagger file not found", "path", urlPath, "error", err)
+ http.Error(w, msg, http.StatusNotFound)
return
}
defer file.Close()
// Set the content-type
ext := filepath.Ext(urlPath)
if ct := getContentType(ext); ct != "" {
w.Header().Set("Content-Type", ct)
}
// Serve the file
-http.ServeContent(w, r, urlPath, time.Now(), file.(io.ReadSeeker))
+// Use a fixed timestamp for deterministic behavior
+http.ServeContent(w, r, urlPath, time.Unix(0, 0), file.(io.ReadSeeker))Committable suggestion skipped: line range outside the PR's diff.
| // Start starts the server | ||
| func (s *Server[T]) Start(ctx context.Context) error { | ||
| if !s.config.Enable { | ||
| s.logger.Info(fmt.Sprintf("%s server is disabled via config", s.Name())) | ||
| return nil | ||
| } | ||
|
|
||
| s.logger.Info("starting swagger server...", "address", s.config.Address) | ||
| if err := s.server.ListenAndServe(); err != nil && err != http.ErrServerClosed { | ||
| return fmt.Errorf("failed to start swagger server: %w", err) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // Stop stops the server | ||
| func (s *Server[T]) Stop(ctx context.Context) error { | ||
| if !s.config.Enable { | ||
| return nil | ||
| } | ||
|
|
||
| s.logger.Info("stopping swagger server...", "address", s.config.Address) | ||
| return s.server.Shutdown(ctx) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance server lifecycle handling with context and graceful shutdown.
The server lifecycle management should:
- Use context in Start method
- Add graceful shutdown timeout
- Improve error handling
func (s *Server[T]) Start(ctx context.Context) error {
if !s.config.Enable {
s.logger.Info(fmt.Sprintf("%s server is disabled via config", s.Name()))
return nil
}
s.logger.Info("starting swagger server...", "address", s.config.Address)
- if err := s.server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
- return fmt.Errorf("failed to start swagger server: %w", err)
+
+ // Use errChan to handle server errors
+ errChan := make(chan error, 1)
+ go func() {
+ if err := s.server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
+ errChan <- fmt.Errorf("failed to start swagger server: %w", err)
+ }
+ }()
+
+ // Wait for context cancellation or server error
+ select {
+ case <-ctx.Done():
+ return s.Stop(context.Background())
+ case err := <-errChan:
+ return err
}
return nil
}
func (s *Server[T]) Stop(ctx context.Context) error {
if !s.config.Enable {
return nil
}
s.logger.Info("stopping swagger server...", "address", s.config.Address)
- return s.server.Shutdown(ctx)
+
+ // Create a timeout context for shutdown
+ shutdownCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
+ defer cancel()
+
+ if err := s.server.Shutdown(shutdownCtx); err != nil {
+ s.logger.Error("failed to gracefully shutdown swagger server", "error", err)
+ return fmt.Errorf("failed to gracefully shutdown swagger server: %w", err)
+ }
+
+ return nil
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Start starts the server | |
| func (s *Server[T]) Start(ctx context.Context) error { | |
| if !s.config.Enable { | |
| s.logger.Info(fmt.Sprintf("%s server is disabled via config", s.Name())) | |
| return nil | |
| } | |
| s.logger.Info("starting swagger server...", "address", s.config.Address) | |
| if err := s.server.ListenAndServe(); err != nil && err != http.ErrServerClosed { | |
| return fmt.Errorf("failed to start swagger server: %w", err) | |
| } | |
| return nil | |
| } | |
| // Stop stops the server | |
| func (s *Server[T]) Stop(ctx context.Context) error { | |
| if !s.config.Enable { | |
| return nil | |
| } | |
| s.logger.Info("stopping swagger server...", "address", s.config.Address) | |
| return s.server.Shutdown(ctx) | |
| } | |
| // Start starts the server | |
| func (s *Server[T]) Start(ctx context.Context) error { | |
| if !s.config.Enable { | |
| s.logger.Info(fmt.Sprintf("%s server is disabled via config", s.Name())) | |
| return nil | |
| } | |
| s.logger.Info("starting swagger server...", "address", s.config.Address) | |
| // Use errChan to handle server errors | |
| errChan := make(chan error, 1) | |
| go func() { | |
| if err := s.server.ListenAndServe(); err != nil && err != http.ErrServerClosed { | |
| errChan <- fmt.Errorf("failed to start swagger server: %w", err) | |
| } | |
| }() | |
| // Wait for context cancellation or server error | |
| select { | |
| case <-ctx.Done(): | |
| return s.Stop(context.Background()) | |
| case err := <-errChan: | |
| return err | |
| } | |
| return nil | |
| } | |
| // Stop stops the server | |
| func (s *Server[T]) Stop(ctx context.Context) error { | |
| if !s.config.Enable { | |
| return nil | |
| } | |
| s.logger.Info("stopping swagger server...", "address", s.config.Address) | |
| // Create a timeout context for shutdown | |
| shutdownCtx, cancel := context.WithTimeout(ctx, 30*time.Second) | |
| defer cancel() | |
| if err := s.server.Shutdown(shutdownCtx); err != nil { | |
| s.logger.Error("failed to gracefully shutdown swagger server", "error", err) | |
| return fmt.Errorf("failed to gracefully shutdown swagger server: %w", err) | |
| } | |
| return nil | |
| } |
| // Server represents a Swagger UI server | ||
| type Server[T transaction.Tx] struct { | ||
| logger log.Logger | ||
| config *Config | ||
| cfgOptions []CfgOption | ||
| server *http.Server | ||
| } | ||
|
|
||
| // New creates a new Swagger UI server | ||
| func New[T transaction.Tx]( | ||
| logger log.Logger, | ||
| cfg server.ConfigMap, | ||
| cfgOptions ...CfgOption, | ||
| ) (*Server[T], error) { | ||
| srv := &Server[T]{ | ||
| logger: logger.With(log.ModuleKey, ServerName), | ||
| cfgOptions: cfgOptions, | ||
| } | ||
|
|
||
| serverCfg := DefaultConfig() | ||
| if len(cfg) > 0 { | ||
| if err := serverv2.UnmarshalSubConfig(cfg, ServerName, serverCfg); err != nil { | ||
| return nil, fmt.Errorf("failed to unmarshal config: %w", err) | ||
| } | ||
| } | ||
| for _, opt := range cfgOptions { | ||
| opt(serverCfg) | ||
| } | ||
| srv.config = serverCfg | ||
|
|
||
| if err := srv.config.Validate(); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| mux := http.NewServeMux() | ||
| mux.Handle("/swagger", &swaggerHandler{ | ||
| swaggerFS: srv.config.SwaggerUI, | ||
| }) | ||
|
|
||
| srv.server = &http.Server{ | ||
| Addr: srv.config.Address, | ||
| Handler: mux, | ||
| } | ||
|
|
||
| return srv, nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance server initialization with timeouts and improved error handling.
The server initialization should include:
- HTTP server timeouts for better security
- More detailed error handling
- Graceful shutdown configuration
srv.server = &http.Server{
Addr: srv.config.Address,
Handler: mux,
+ // Add timeouts for better security
+ ReadTimeout: 15 * time.Second,
+ ReadHeaderTimeout: 5 * time.Second,
+ WriteTimeout: 15 * time.Second,
+ IdleTimeout: 60 * time.Second,
+ MaxHeaderBytes: 1 << 20, // 1MB
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Server represents a Swagger UI server | |
| type Server[T transaction.Tx] struct { | |
| logger log.Logger | |
| config *Config | |
| cfgOptions []CfgOption | |
| server *http.Server | |
| } | |
| // New creates a new Swagger UI server | |
| func New[T transaction.Tx]( | |
| logger log.Logger, | |
| cfg server.ConfigMap, | |
| cfgOptions ...CfgOption, | |
| ) (*Server[T], error) { | |
| srv := &Server[T]{ | |
| logger: logger.With(log.ModuleKey, ServerName), | |
| cfgOptions: cfgOptions, | |
| } | |
| serverCfg := DefaultConfig() | |
| if len(cfg) > 0 { | |
| if err := serverv2.UnmarshalSubConfig(cfg, ServerName, serverCfg); err != nil { | |
| return nil, fmt.Errorf("failed to unmarshal config: %w", err) | |
| } | |
| } | |
| for _, opt := range cfgOptions { | |
| opt(serverCfg) | |
| } | |
| srv.config = serverCfg | |
| if err := srv.config.Validate(); err != nil { | |
| return nil, err | |
| } | |
| mux := http.NewServeMux() | |
| mux.Handle("/swagger", &swaggerHandler{ | |
| swaggerFS: srv.config.SwaggerUI, | |
| }) | |
| srv.server = &http.Server{ | |
| Addr: srv.config.Address, | |
| Handler: mux, | |
| } | |
| return srv, nil | |
| } | |
| // Server represents a Swagger UI server | |
| type Server[T transaction.Tx] struct { | |
| logger log.Logger | |
| config *Config | |
| cfgOptions []CfgOption | |
| server *http.Server | |
| } | |
| // New creates a new Swagger UI server | |
| func New[T transaction.Tx]( | |
| logger log.Logger, | |
| cfg server.ConfigMap, | |
| cfgOptions ...CfgOption, | |
| ) (*Server[T], error) { | |
| srv := &Server[T]{ | |
| logger: logger.With(log.ModuleKey, ServerName), | |
| cfgOptions: cfgOptions, | |
| } | |
| serverCfg := DefaultConfig() | |
| if len(cfg) > 0 { | |
| if err := serverv2.UnmarshalSubConfig(cfg, ServerName, serverCfg); err != nil { | |
| return nil, fmt.Errorf("failed to unmarshal config: %w", err) | |
| } | |
| } | |
| for _, opt := range cfgOptions { | |
| opt(serverCfg) | |
| } | |
| srv.config = serverCfg | |
| if err := srv.config.Validate(); err != nil { | |
| return nil, err | |
| } | |
| mux := http.NewServeMux() | |
| mux.Handle("/swagger", &swaggerHandler{ | |
| swaggerFS: srv.config.SwaggerUI, | |
| }) | |
| srv.server = &http.Server{ | |
| Addr: srv.config.Address, | |
| Handler: mux, | |
| // Add timeouts for better security | |
| ReadTimeout: 15 * time.Second, | |
| ReadHeaderTimeout: 5 * time.Second, | |
| WriteTimeout: 15 * time.Second, | |
| IdleTimeout: 60 * time.Second, | |
| MaxHeaderBytes: 1 << 20, // 1MB | |
| } | |
| return srv, nil | |
| } |
Description
This PR adds Swagger UI support to the server/v2 package, providing an easy way to serve and interact with API documentation. The implementation includes a configurable handler for serving Swagger UI files and proper configuration options.
Closes: #23020
Key changes:
Author Checklist
I have...
[x] included the correct type prefix (feat) in the PR title
[x] confirmed ! in the type prefix if API or client breaking change (not needed)
[x] targeted the correct branch (main)
[x] provided a link to the relevant issue (#23020)
[x] reviewed "Files changed" and left comments if necessary
[x] included the necessary unit and integration tests
[x] added a changelog entry to CHANGELOG.md:
[x] updated the relevant documentation or specification
[x] confirmed all CI checks have passed
Reviewers Checklist
[ ] confirmed the correct type prefix in the PR title
[ ] confirmed all author checklist items have been addressed
[ ] reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage
Changed files:
Summary by CodeRabbit
New Features
Improvements
Technical Enhancements