🐛 fix: Embedded struct parsing #3478
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test_Ctx_QueryParser_EmbeddedStruct
participant Parser as QueryParser
participant Struct as Person/A Structs
Test->>Parser: Call QueryParser(p)
Parser->>Struct: Inspect fields (including embedded)
Parser->>Struct: Parse query tags and values
Struct->>Parser: Return parsed fields (including Products slice)
Parser->>Test: Return parsed struct
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (11)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 10
🔭 Outside diff range comments (1)
middleware/session/session.go (1)
112-123:⚠️ Potential issueCritical: Incorrect lock type in Destroy method.
The
Destroymethod usesRLockbut performs write operations (storage deletion anddelSession). This can lead to race conditions when multiple goroutines callDestroyconcurrently.Replace the read lock with a write lock:
-s.mu.RLock() -defer s.mu.RUnlock() +s.mu.Lock() +defer s.mu.Unlock()
🧹 Nitpick comments (39)
docs/guide/routing.md (1)
148-149: Nit: Improve caution note clarity and grammar
The phrase “Constraints aren’t validation for parameters.” is awkward—consider rephrasing to “Constraints do not serve as validation for parameters.” for better readability.Makefile (1)
50-55: Consider improving dependency installation.The generate target installs tools each time it runs. Consider using a separate setup target or checking if tools are already installed to improve performance.
+## setup: 🔧 Install development dependencies +.PHONY: setup +setup: + go install github.com/tinylib/msgp@latest + go install github.com/vburenin/ifacemaker@975a95966976eeb2d4365a7fb236e274c54da64c + ## generate: ⚡️ Generate msgp && interface implementations .PHONY: generate -generate: - go install github.com/tinylib/msgp@latest - go install github.com/vburenin/ifacemaker@975a95966976eeb2d4365a7fb236e274c54da64c +generate: setup go generate ./...AGENTS.md (1)
5-5: Fix grammar issue.The static analysis tool correctly identified a missing comma after the conjunctive adverb "Otherwise".
-> **Note for the AI agent:** Before running the startup script, verify that all dependencies and CLI tools are already available. If a `vendor/` directory exists **or** `go list -m -mod=readonly all` succeeds (indicating modules are cached), you may skip the entire startup script—including all `go mod` commands and `go install` steps. Otherwise run the script as listed below. +> **Note for the AI agent:** Before running the startup script, verify that all dependencies and CLI tools are already available. If a `vendor/` directory exists **or** `go list -m -mod=readonly all` succeeds (indicating modules are cached), you may skip the entire startup script—including all `go mod` commands and `go install` steps. Otherwise, run the script as listed below.🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...o modcommands andgo install` steps. Otherwise run the script as listed below. --- #...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
.github/README_es.md (1)
125-126: Add alt text for benchmark images
For accessibility and to satisfy Markdown lint rules (MD045), include descriptivealtattributes on the<img>tags, e.g.:<img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="Fiber plaintext benchmark" width="49%"> <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="Fiber JSON benchmark" width="49%">.github/README_zh-CN.md (1)
127-128: Add alt text for benchmark images
To improve accessibility and satisfy Markdown lint rules (MD045), please includealtattributes on these<img>tags, for example:<img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="Fiber 明文基准测试" width="49%"> <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="Fiber JSON 基准测试" width="49%">🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
127-127: Images should have alternate text (alt text)
null(MD045, no-alt-text)
128-128: Images should have alternate text (alt text)
null(MD045, no-alt-text)
.github/README_ru.md (1)
125-126: Add descriptive alt text to images for accessibility.The new
<img>tags lackaltattributes, which triggers MD045 (no-alt-text) and harms accessibility. Please include meaningfulalttext for each benchmark image.- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="Fiber plaintext benchmark" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="Fiber JSON benchmark" width="49%">.github/README_ko.md (1)
125-126: Add descriptive alt text to images for accessibility.The updated benchmark images are missing
altattributes, causing MD045 warnings and reducing accessibility. Please add concisealttext.- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="Fiber plaintext benchmark" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="Fiber JSON benchmark" width="49%">🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
125-125: Images should have alternate text (alt text)
null(MD045, no-alt-text)
126-126: Images should have alternate text (alt text)
null(MD045, no-alt-text)
.github/README_az.md (1)
126-127: Add descriptive alt text for benchmark images
The<img>tags on lines 126–127 lackaltattributes, which hurts accessibility. Please include meaningfulalttext, for example:- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="Plaintext benchmark results" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="JSON benchmark results" width="49%">🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
126-126: Images should have alternate text (alt text)
null(MD045, no-alt-text)
127-127: Images should have alternate text (alt text)
null(MD045, no-alt-text)
.github/README_ja.md (1)
126-127: Add descriptive alt text for benchmark images
The<img>tags on lines 126–127 are missingaltattributes, which impacts screen readers. Consider adding Japanese alt text:- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="Plaintext ベンチマーク結果" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="JSON ベンチマーク結果" width="49%">.github/README_it.md (1)
125-126: Add alt text to new benchmark images for accessibilityThe updated images currently lack
altattributes, which are important for accessibility and Markdown linting (MD045). Please add descriptivealttext to these<img>tags.- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%" alt="Plaintext benchmark result"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%" alt="JSON benchmark result">.github/README_fr.md (1)
125-126: Add alt text to new benchmark images for accessibilityThe new
<img>tags for the benchmark section are missingaltattributes, causing a Markdown lint warning (MD045) and reducing accessibility. Please include descriptivealttext.- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%" alt="résultats du benchmark plaintext"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%" alt="résultats du benchmark JSON">🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
125-125: Images should have alternate text (alt text)
null(MD045, no-alt-text)
126-126: Images should have alternate text (alt text)
null(MD045, no-alt-text)
.github/README_fa.md (1)
140-141: Add descriptive alt text to benchmark images
To improve accessibility and satisfy markdownlint MD045, includealtattributes for these images. For example:- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="TechEmpower plaintext benchmark results" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="Go Web JSON benchmark results" width="49%">🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
140-140: Images should have alternate text (alt text)
null(MD045, no-alt-text)
141-141: Images should have alternate text (alt text)
null(MD045, no-alt-text)
.github/README_sa.md (1)
131-132: Add descriptive alt text to benchmark images
To address lint warning MD045 and improve accessibility, includealttext on the images. For example:- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="TechEmpower plaintext benchmark results" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="Go Web JSON benchmark results" width="49%">🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
131-131: Images should have alternate text (alt text)
null(MD045, no-alt-text)
132-132: Images should have alternate text (alt text)
null(MD045, no-alt-text)
.github/README_uk.md (1)
135-136: Add alt text to benchmark images for accessibility.The images lack
altattributes, which is important for accessibility and flagged by markdownlint (MD045). Please include descriptive alt text. For example:- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img alt="Fiber plaintext benchmark results" src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> + <img alt="Fiber JSON benchmark results" src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%">🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
135-135: Images should have alternate text (alt text)
null(MD045, no-alt-text)
136-136: Images should have alternate text (alt text)
null(MD045, no-alt-text)
.github/README_de.md (1)
125-126: Add alt text to benchmark images for accessibility.The images are missing
alttext, which is flagged by markdownlint (MD045). Please add descriptive alt attributes. For example:- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img alt="Plaintext Benchmark Ergebnisse" src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> + <img alt="JSON Benchmark Ergebnisse" src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%">🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
125-125: Images should have alternate text (alt text)
null(MD045, no-alt-text)
126-126: Images should have alternate text (alt text)
null(MD045, no-alt-text)
.github/README_pt.md (1)
125-126: Include alt text for images to improve accessibility.The new
<img>tags lackaltattributes, which are important for screen readers and SEO. For example:- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="Fiber plaintext benchmark" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="Fiber JSON benchmark" width="49%">🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
125-125: Images should have alternate text (alt text)
null(MD045, no-alt-text)
126-126: Images should have alternate text (alt text)
null(MD045, no-alt-text)
.github/README_zh-TW.md (1)
129-130: Include alt text for images to improve accessibility.新增的
<img>標籤應加入描述性的alt屬性，例如：- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="Fiber 純文字基準測試" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="Fiber JSON 基準測試" width="49%">.github/README_nl.md (1)
125-126: Add alt text to benchmark images for accessibilityThe new images lack
altattributes, which will trigger markdown-lint (MD045) warnings and reduce accessibility. Please add descriptive alt text:- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="Plaintext benchmark results" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="JSON benchmark results" width="49%">🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
125-125: Images should have alternate text (alt text)
null(MD045, no-alt-text)
126-126: Images should have alternate text (alt text)
null(MD045, no-alt-text)
.github/README_tr.md (1)
125-126: Add alt text to benchmark images for accessibilityThe new
<img>tags do not includealtattributes, causing markdown-lint (MD045) warnings and affecting accessibility. Please include descriptive alt text. For example:- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="Performans: düz metin" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="Performans: JSON" width="49%">.github/README.md (1)
127-128: Add alt text to imagesThe
<img>tags currently lackaltattributes, which is required for accessibility (MD045).Consider:
- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> + <img alt="Plaintext benchmark result" src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img alt="JSON benchmark result" src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%">.github/README_id.md (1)
125-127: Add alt text to imagesInclude descriptive
altattributes on images for accessibility compliance (MD045).- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> + <img alt="Hasil benchmark plaintext" src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img alt="Hasil benchmark JSON" src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%">🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
125-125: Images should have alternate text (alt text)
null(MD045, no-alt-text)
126-126: Images should have alternate text (alt text)
null(MD045, no-alt-text)
docs/api/middleware/keyauth.md (1)
50-50: Fix hard tab formatting issue.Static analysis detected a hard tab character that should be replaced with spaces for consistency.
- app.Get("/", func(c *fiber.Ctx) error { + app.Get("/", func(c *fiber.Ctx) error {🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
50-50: Hard tabs
Column: 1(MD010, no-hard-tabs)
docs/api/middleware/logger.md (2)
98-98: Fix grammar issues in the description.The description has grammatical issues that should be corrected for better clarity.
-| Done | `func(*fiber.Ctx, []byte)` | Done is a function that is called after the log string for a request is written to Output, and pass the log string as parameter. | `nil` | +| Done | `func(*fiber.Ctx, []byte)` | Done is a function that is called after the log string for a request is written to Output, and passes the log string as a parameter. | `nil` |🧰 Tools
🪛 LanguageTool
[uncategorized] ~98-~98: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...for a request is written to Output, and pass the log string as parameter. |nil...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~98-~98: You might be missing the article “a” here.
Context: ...n to Output, and pass the log string as parameter. |nil...(AI_EN_LECTOR_MISSING_DETERMINER_A)
101-101: Add missing period after "etc".In American English, "etc." requires a period.
-| TimeZone | `string` | TimeZone can be specified, such as "UTC" and "America/New_York" and "Asia/Chongqing", etc | `"Local"` | +| TimeZone | `string` | TimeZone can be specified, such as "UTC" and "America/New_York" and "Asia/Chongqing", etc. | `"Local"` |🧰 Tools
🪛 LanguageTool
[style] ~101-~101: In American English, abbreviations like “etc.” require a period.
Context: ...America/New_York" and "Asia/Chongqing", etc ...(ETC_PERIOD)
middleware/session/session.go (1)
179-223: Well-implemented thread-safe Save method with important usage warning.The mutex properly protects the encoding and storage operations. The comment warning about not using the session after
Save()is crucial for preventing use-after-release bugs.Consider making the warning even more prominent in the function documentation:
// Save will update the storage and client cookie // -// sess.Save() will save the session data to the storage and update the -// client cookie, and it will release the session after saving. +// WARNING: sess.Save() will save the session data to the storage and update the +// client cookie, then release the session to the pool. // -// It's not safe to use the session after calling Save(). +// IMPORTANT: Do NOT use the session after calling Save() - it will cause undefined behavior.docs/extra/benchmarks.md (1)
32-32: Add alt text to images for better accessibility.The benchmark images are missing alt text, which impacts accessibility for users with screen readers.
Add descriptive alt text to the images:
- + - + - + - + - +Also applies to: 41-41, 50-50, 59-59, 68-68
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
32-32: Images should have alternate text (alt text)
null(MD045, no-alt-text)
middleware/cors/utils.go (1)
20-26: Consider improving IPv6 address handling.The current implementation doesn't fully handle IPv6 addresses with ports. For example,
[::1]:8080would not have its port removed because the conditioninput[0] != '['prevents entering the port removal logic.Consider using
net.SplitHostPortfor more robust handling:-// Find and remove port, if present -if len(input) > 0 && input[0] != '[' { - if portIndex := strings.Index(input, ":"); portIndex != -1 { - input = input[:portIndex] - } -} +// Find and remove port, if present +if host, _, err := net.SplitHostPort(input); err == nil { + input = host +} else if !strings.Contains(input, ":") { + // input is already without port +} else { + // For domains with colons but no port (e.g., IPv6 without brackets) + // leave as is +}middleware/healthcheck/config.go (1)
38-41: Consider adding validation for endpoint paths.The default constants are well-named and follow RESTful conventions. However, consider adding validation to ensure endpoints start with "/" in the
defaultConfigfunction.if cfg.LivenessEndpoint == "" { cfg.LivenessEndpoint = DefaultLivenessEndpoint + } else if cfg.LivenessEndpoint[0] != '/' { + cfg.LivenessEndpoint = "/" + cfg.LivenessEndpoint }docs/api/middleware/healthcheck.md (1)
52-53: Consider making the example more concrete.The example shows a placeholder for checking service readiness. Consider providing a more concrete example to help users understand implementation patterns.
- return serviceA.Ready() && serviceB.Ready() && ... + // Example: Check database and cache connectivity + return db.Ping() == nil && cache.Ping() == nilmiddleware/cors/utils_test.go (1)
140-144: Test names could be more descriptive.The test case names "match with different scheme" are duplicated and could be more specific about what's being tested.
- name: "match with different scheme", + name: "no match - http origin with https pattern", sub: subdomain{prefix: "http://api.", suffix: ".example.com"}, origin: "https://api.service.example.com", expected: false, }, { - name: "match with different scheme", + name: "no match - https pattern with http origin",docs/api/middleware/encryptcookie.md (2)
58-58: Fix grammar for better parallel structure.The phrase "make sure it is random and keep it secret" should use "keeps" for parallel structure in formal documentation.
Apply this diff to fix the grammar:
-`Key` must be a 32 character string. It's used to encrypt the values, so make sure it is random and keep it secret. +`Key` must be a 32 character string. It's used to encrypt the values, so make sure it is random and keeps it secret.🧰 Tools
🪛 LanguageTool
[grammar] ~58-~58: Did you mean the verb “keeps”? For a parallel structure in formal text, use “it keeps”, because ‘random’ is an adjective.
Context: ...e values, so make sure it is random and keep it secret. You can run `openssl rand -b...(PRP_IS_JJ_AND_VB)
77-81: Replace hard tabs with spaces for consistent formatting.The code blocks contain hard tabs which should be replaced with spaces for consistent markdown formatting.
Apply this diff to fix the formatting:
var ConfigDefault = Config{ - Next: nil, - Except: []string{}, - Key: "", - Encryptor: EncryptCookie, - Decryptor: DecryptCookie, + Next: nil, + Except: []string{}, + Key: "", + Encryptor: EncryptCookie, + Decryptor: DecryptCookie, }And for the example configuration:
app.Use(encryptcookie.New(encryptcookie.Config{ - Key: "secret-thirty-2-character-string", - Except: []string{csrf.ConfigDefault.CookieName}, // exclude CSRF cookie + Key: "secret-thirty-2-character-string", + Except: []string{csrf.ConfigDefault.CookieName}, // exclude CSRF cookie })) app.Use(csrf.New(csrf.Config{ - KeyLookup: "header:" + csrf.HeaderName, - CookieSameSite: "Lax", - CookieSecure: true, - CookieHTTPOnly: false, + KeyLookup: "header:" + csrf.HeaderName, + CookieSameSite: "Lax", + CookieSecure: true, + CookieHTTPOnly: false, }))Also applies to: 92-99
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
77-77: Hard tabs
Column: 1(MD010, no-hard-tabs)
78-78: Hard tabs
Column: 1(MD010, no-hard-tabs)
79-79: Hard tabs
Column: 1(MD010, no-hard-tabs)
80-80: Hard tabs
Column: 1(MD010, no-hard-tabs)
81-81: Hard tabs
Column: 1(MD010, no-hard-tabs)
docs/api/middleware/cors.md (1)
120-120: Consider style improvement for conciseness.The phrase "whether or not" can be simplified to just "whether" for more concise writing, unless the meaning specifically requires "regardless of whether."
Apply this diff for better style:
-| AllowCredentials | `bool` | AllowCredentials indicates whether or not the response to the request can be exposed when the credentials flag is true. When used as part of a response to a preflight request, this indicates whether or not the actual request can be made using credentials. Note: If true, AllowOrigins cannot be set to a wildcard (`"*"`) to prevent security vulnerabilities. | `false` | +| AllowCredentials | `bool` | AllowCredentials indicates whether the response to the request can be exposed when the credentials flag is true. When used as part of a response to a preflight request, this indicates whether the actual request can be made using credentials. Note: If true, AllowOrigins cannot be set to a wildcard (`"*"`) to prevent security vulnerabilities. | `false` |🧰 Tools
🪛 LanguageTool
[style] ~120-~120: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ... | AllowCredentials indicates whether or not the response to the request can be expo...(WHETHER)
[style] ~120-~120: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ... to a preflight request, this indicates whether or not the actual request can be made using cr...(WHETHER)
middleware/cors/cors.go (1)
251-254: Consider removing or clarifying this warning.Since the initialization already panics when
AllowCredentialsis true andAllowOriginsis "*", this warning would only trigger ifAllowOriginsFunc` returns true for a wildcard origin. Consider either removing this warning or updating the message to be more specific about the AllowOriginsFunc case.if allowOrigin == "*" { c.Set(fiber.HeaderAccessControlAllowOrigin, allowOrigin) - log.Warn("[CORS] 'AllowCredentials' is true, but 'AllowOrigins' cannot be set to '*'.") + log.Warn("[CORS] 'AllowCredentials' is true, but AllowOriginsFunc returned true for wildcard origin '*'.") }ctx_test.go (2)
3669-3669: Remove unnecessary code addition.This line creates and writes to a buffer but doesn't use it for anything meaningful. Consider removing this unused code to keep the tests clean and focused.
- _, _ = buf.WriteString("overwrite")
3691-3691: Remove unnecessary code addition.Same issue as line 3669 - this creates and writes to a buffer but doesn't contribute to the test logic. Consider removing for cleaner code.
- _, _ = buf.WriteString("overwrite")ctx.go (2)
343-343: Consider consistent immutability handling.The immutability is handled differently in two cases:
- Line 343: Delegates to
getBody()which handles immutability internally- Lines 357-360: Manually checks and copies
For consistency and maintainability, consider always using
getBody()or creating a helper method for post-decoding immutability.if len(encodingOrder) == 0 { return c.getBody() } // ... decoding logic ... -if c.app.config.Immutable { - return utils.CopyBytes(body) -} -return body +return c.conditionalCopyBytes(body)Add a helper method:
func (c *Ctx) conditionalCopyBytes(data []byte) []byte { if c.app.config.Immutable { return utils.CopyBytes(data) } return data }Also applies to: 357-360
951-956: Consider error handling for buffer operations.While buffer write operations typically don't fail for in-memory buffers, explicitly ignoring errors with
_assignments should be documented or reconsidered. If these operations are guaranteed not to fail, consider adding a comment explaining why error handling is unnecessary.middleware/cors/cors_test.go (1)
892-1335: Comprehensive benchmark coverage!Excellent set of benchmarks covering:
- Single vs multiple origins
- Wildcard configurations
- Preflight vs regular requests
- Sequential vs parallel execution
One suggestion: Consider adding a benchmark for
AllowOriginsFuncto measure the performance impact of custom origin validation functions.
🛑 Comments failed to post (10)
docs/guide/routing.md (1)
153-166: 🛠️ Refactor suggestion
Fix escaped characters inside inline code formatting in the constraints table
Inline code blocks render<and>literally, so the\>escapes will appear in the rendered docs. Remove the backslashes within all constraint patterns to display them correctly.Apply this diff to correct the patterns:
-| int | `:id<int\>` | 123456789, -123456789 +| int | `:id<int>` | 123456789, -123456789 -| bool | `:active<bool\>` | true,false +| bool | `:active<bool>` | true,false -| guid | `:id<guid\>` | CD2C1638-1638-72D5-1638-DEADBEEF1638 +| guid | `:id<guid>` | CD2C1638-1638-72D5-1638-DEADBEEF1638 -| float | `:weight<float\>` | 1.234, -1,001.01e8 +| float | `:weight<float>` | 1.234, -1,001.01e8 -| minLen(value) | `:username<minLen(4)\>` | Test (must be at least 4 characters) +| minLen(value) | `:username<minLen(4)>` | Test (must be at least 4 characters) ... (apply the same removal of `\` before `>` for all remaining rows) ...📝 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.| int | `:id<int>` | 123456789, -123456789 | | bool | `:active<bool>` | true,false | | guid | `:id<guid>` | CD2C1638-1638-72D5-1638-DEADBEEF1638 | | float | `:weight<float>` | 1.234, -1,001.01e8 | | minLen(value) | `:username<minLen(4)>` | Test (must be at least 4 characters) | | maxLen(value) | `:filename<maxLen(8)>` | MyFile (must be no more than 8 characters | | len(length) | `:filename<len(12)>` | somefile.txt (exactly 12 characters) | | min(value) | `:age<min(18)>` | 19 (Integer value must be at least 18) | | max(value) | `:age<max(120)>` | 91 (Integer value must be no more than 120) | | range(min,max) | `:age<range(18,120)>` | 91 (Integer value must be at least 18 but no more than 120) | | alpha | `:name<alpha>` | Rick (String must consist of one or more alphabetical characters, a-z and case-insensitive) | | datetime | `:dob<datetime(2006\\\\-01\\\\-02)>` | 2005-11-01 | | regex(expression) | `:date<regex(\\d{4}-\\d{2}-\\d{2})>` | 2022-08-27 (Must match regular expression) |🤖 Prompt for AI Agents
In docs/guide/routing.md around lines 153 to 166, the backslashes before the closing angle brackets in the inline code blocks for constraint patterns are unnecessary and cause incorrect rendering. Remove all backslashes before the closing angle brackets inside the inline code formatting so that patterns like `:id<int\>` become `:id<int>`, ensuring correct display in the rendered documentation.log/default.go (1)
30-31: 💡 Verification agent
🧩 Analysis chain
Verify selective lint comment removal strategy.
The removal of
//nolint:errcheckfrombuf.WriteString()calls while keeping them onl.stdlog.Output()creates inconsistency. Ensure this selective approach aligns with project linting standards.
🏁 Script executed:
#!/bin/bash # Description: Check linting consistency across the codebase # Find all remaining nolint:errcheck comments to understand the pattern rg "//nolint:errcheck" -A 1 -B 1 # Check if there are specific linting rules for buffer operations rg -A 3 -B 3 "WriteString.*nolint" # Look for similar patterns in other log files fd -e go -p "log" -x grep -l "WriteString" {} \;Length of output: 10636
Add
//nolint:errcheckto buf.WriteString calls in log/default.goThe two
buf.WriteStringcalls ignore errors without any//nolint:errchecksuppression, which will trigger errcheck warnings in CI. All other ignored WriteString or Output calls in the codebase carry the suppression. For consistency and to avoid breaking lint, please re-apply the directive:• File: log/default.go
Lines ~30–31- _, _ = buf.WriteString(level) + _, _ = buf.WriteString(level) //nolint:errcheck // safe to ignore - _, _ = buf.WriteString(fmt.Sprint(fmtArgs...)) + _, _ = buf.WriteString(fmt.Sprint(fmtArgs...)) //nolint:errcheck // safe to ignore📝 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._, _ = buf.WriteString(level) //nolint:errcheck // safe to ignore _, _ = buf.WriteString(fmt.Sprint(fmtArgs...)) //nolint:errcheck // safe to ignore🤖 Prompt for AI Agents
In log/default.go around lines 30 to 31, the calls to buf.WriteString are ignoring errors but lack the //nolint:errcheck comment, causing lint warnings. To fix this, add the //nolint:errcheck directive to both buf.WriteString calls to maintain consistency with other similar calls in the codebase and suppress the errcheck warnings..github/scripts/sync_docs.sh (1)
21-29:
⚠️ Potential issueFix undefined variable reference for push events.
The push event handling block is commented out, but the script still references the
latest_commitvariable at line 56. This will cause the script to fail whenEVENT="push"sincelatest_commitwill be undefined.Apply this diff to fix the issue:
-#if [ "$EVENT" == "push" ]; then -# latest_commit=$(git rev-parse --short HEAD) -# log_output=$(git log --oneline ${BRANCH} HEAD~1..HEAD --name-status -- docs/) -# if [[ $log_output != "" ]]; then -# cp -a docs/* fiber-docs/docs/${REPO_DIR} -# fi -# -## Handle release event -#el +# Handle release event if [ "$EVENT" == "release" ]; thenAnd update the commit message logic:
if [[ $EVENT == "push" ]]; then - git commit -m "Add docs from ${COMMIT_URL}/commit/${latest_commit}" + latest_commit=$(git rev-parse --short HEAD) + git commit -m "Add docs from ${COMMIT_URL}/commit/${latest_commit}" elif [[ $EVENT == "release" ]]; then📝 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.# Replace the commented-out push block with an explicit release handler # Handle release event if [ "$EVENT" == "release" ]; then # existing release-event docs sync logic… fi # …later in the script, ensure latest_commit is set before use if [[ $EVENT == "push" ]]; then latest_commit=$(git rev-parse --short HEAD) git commit -m "Add docs from ${COMMIT_URL}/commit/${latest_commit}" elif [[ $EVENT == "release" ]]; then # existing release-event commit logic… fi🤖 Prompt for AI Agents
In .github/scripts/sync_docs.sh around lines 21 to 29, the push event handling code is commented out, causing the variable latest_commit to be undefined when EVENT="push". To fix this, uncomment and restore the push event block so that latest_commit is properly assigned. Also, update the commit message logic accordingly to ensure it uses latest_commit only when defined..github/README_pl.md (1)
128-129: 🛠️ Refactor suggestion
Add alt text to benchmark images
Images lack
altattributes, which impacts accessibility for users relying on screen readers. Please include descriptive alternate text for both images.Proposed diff:
- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="Plaintext benchmark results" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="JSON benchmark results" width="49%">📝 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.<img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="Plaintext benchmark results" width="49%"> <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="JSON benchmark results" width="49%">🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
128-128: Images should have alternate text (alt text)
null(MD045, no-alt-text)
129-129: Images should have alternate text (alt text)
null(MD045, no-alt-text)
🤖 Prompt for AI Agents
In .github/README_pl.md around lines 128 to 129, the two image tags lack alt attributes, which reduces accessibility. Add descriptive alt text to each img tag to improve screen reader support, ensuring the alt text clearly describes the content or purpose of each image..github/README_he.md (1)
142-143: 🛠️ Refactor suggestion
Add alt text to benchmark images
The two
<img>elements in the metrics section are missingaltattributes. This hinders accessibility for screen reader users. Please add descriptive alternate text in Hebrew.Proposed diff:
- <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="תוצאות הבנצ'מרק של טקסט רגיל" width="49%"> - <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" width="49%"> + <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="תוצאות הבנצ'מרק של JSON" width="49%">📝 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.<img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/plaintext.png" alt="תוצאות הבנצ'מרק של טקסט רגיל" width="49%"> <img src="https://raw.githubusercontent.com/gofiber/docs/master/static/img/json.png" alt="תוצאות הבנצ'מרק של JSON" width="49%">🤖 Prompt for AI Agents
In .github/README_he.md around lines 142 to 143, the two <img> tags lack alt attributes, which reduces accessibility. Add descriptive alt text in Hebrew to each <img> tag to improve screen reader support and accessibility compliance.middleware/encryptcookie/config.go (1)
39-39: 💡 Verification agent
🧩 Analysis chain
Verify the impact of removing default CSRF exclusion.
The removal of
"csrf_"from the defaultExceptslice is a potential breaking change. Users who relied on CSRF cookies being excluded by default will now need to explicitly configure this exclusion.Ensure this change aligns with the updated documentation and consider adding a migration note if this affects existing users.
🏁 Script executed:
#!/bin/bash # Description: Verify if documentation mentions this change and check for related CSRF middleware usage patterns # Check if documentation mentions the removal of default CSRF exclusion rg -A 5 -B 5 "csrf.*exclude|exclude.*csrf" docs/ # Check for any examples showing CSRF middleware with encryptcookie rg -A 10 -B 10 "encryptcookie.*csrf|csrf.*encryptcookie" docs/ middleware/Length of output: 1175
Update documentation and add migration note for default CSRF exclusion change
The default value of
Config.Exceptinmiddleware/encryptcookie/config.gowas changed from excluding"csrf_"cookies to an empty slice. This is a breaking change for users who relied on CSRF cookies being skipped by default.Please address the following before merging:
• middleware/encryptcookie/config.go (line 39)
- Default
Exceptis now[]string{}instead of[]string{"csrf_"}.- Add a code comment noting the behavioral change, e.g. “Deprecated default exclusion of CSRF cookie—now must be opted-in.”
• docs/api/middleware/encryptcookie.md
- Currently shows only the manual example
Except: []string{csrf.ConfigDefault.CookieName}.- Add a note in the “Configuration” or “Migration” section explaining that the default exclusion of CSRF cookies has been removed, and users upgrading from vX.Y to vZ must explicitly set
Exceptto includecsrf.ConfigDefault.CookieName.• CHANGELOG or migration guide
- Include a bullet under breaking changes: “encryptcookie: default CSRF cookie exclusion removed; update your
Exceptconfig if you still want to skip CSRF cookies.”🤖 Prompt for AI Agents
In middleware/encryptcookie/config.go at line 39, the default Except slice was changed from excluding "csrf_" cookies to an empty slice, which is a breaking change. Add a code comment on line 39 noting this behavioral change, such as "Deprecated default exclusion of CSRF cookie—now must be opted-in." Then update docs/api/middleware/encryptcookie.md to include a migration note explaining that users upgrading must explicitly set Except to include csrf.ConfigDefault.CookieName to maintain previous behavior. Finally, add a bullet point in the CHANGELOG or migration guide under breaking changes about the removal of default CSRF cookie exclusion and the need to update the Except config accordingly.docs/api/middleware/csrf.md (1)
155-155:
⚠️ Potential issueFix the typo in interface{} type declaration.
There's a typo in the type declaration: "inteface{}" should be "interface{}".
Apply this diff to fix the typo:
-| ContextKey | `inteface{}` | Context key to store the generated CSRF token into the context. If left empty, the token will not be stored within the context. | "" | +| ContextKey | `interface{}` | Context key to store the generated CSRF token into the context. If left empty, the token will not be stored within the context. | "" |📝 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.| ContextKey | `interface{}` | Context key to store the generated CSRF token into the context. If left empty, the token will not be stored within the context. | "" |🤖 Prompt for AI Agents
In docs/api/middleware/csrf.md at line 155, correct the typo in the type declaration by changing "inteface{}" to "interface{}" to ensure the documentation accurately reflects the correct Go type syntax.middleware/healthcheck/healthcheck.go (1)
11-13:
⚠️ Potential issueFix the documentation to match the actual type definition.
The comment incorrectly states "ProbeCheckerHandler defines a function that returns a ProbeChecker", but the actual type is
HealthCheckerHandlerthat returns afiber.Handler.-// ProbeCheckerHandler defines a function that returns a ProbeChecker +// HealthCheckerHandler defines a function that returns a fiber.Handler from a HealthChecker type HealthCheckerHandler func(HealthChecker) fiber.Handler📝 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.// HealthCheckerHandler defines a function that returns a fiber.Handler from a HealthChecker type HealthCheckerHandler func(HealthChecker) fiber.Handler🤖 Prompt for AI Agents
In middleware/healthcheck/healthcheck.go around lines 11 to 13, update the comment to correctly describe the type HealthCheckerHandler as a function that takes a HealthChecker and returns a fiber.Handler, instead of mentioning ProbeCheckerHandler and ProbeChecker. Adjust the comment text to accurately reflect the actual type definition.router_test.go (1)
515-538:
⚠️ Potential issueTest function covers important functionality but has a non-idiomatic name.
The test validates the interaction between mounting, static file serving, and middleware, which is valuable. However, the function name uses "n" instead of "and".
-func Test_Router_Mount_n_Static(t *testing.T) { +func Test_Router_Mount_And_Static(t *testing.T) {🤖 Prompt for AI Agents
In router_test.go around lines 515 to 538, rename the test function from Test_Router_Mount_n_Static to Test_Router_Mount_and_Static to follow idiomatic Go naming conventions and improve code readability.helpers.go (1)
1194-1215: 🛠️ Refactor suggestion
Consider validating the data map parameter.
The function handles multiple value types and bracket notation well. However, it doesn't validate if the
dataparameter is nil, which could cause a panic.func formatParserData(out interface{}, data map[string][]string, aliasTag, key string, value interface{}, enableSplitting, supportBracketNotation bool) error { //nolint:revive // it's okay + if data == nil { + return errors.New("data map cannot be nil") + } var err error if supportBracketNotation && strings.Contains(key, "[") {📝 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 formatParserData(out interface{}, data map[string][]string, aliasTag, key string, value interface{}, enableSplitting, supportBracketNotation bool) error { //nolint:revive // it's okay if data == nil { return errors.New("data map cannot be nil") } var err error if supportBracketNotation && strings.Contains(key, "[") { key, err = parseParamSquareBrackets(key) if err != nil { return err } } switch v := value.(type) { case string: assignBindData(out, data, aliasTag, key, v, enableSplitting) case []string: for _, val := range v { assignBindData(out, data, aliasTag, key, val, enableSplitting) } default: return fmt.Errorf("unsupported value type: %T", value) } return err }🤖 Prompt for AI Agents
In helpers.go around lines 1194 to 1215, the function formatParserData does not check if the data map parameter is nil before using it, which can cause a panic. Add a validation at the start of the function to check if data is nil and return an appropriate error if so, preventing runtime panics when the map is accessed later.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the embedded struct parsing issue, ensuring that fields from embedded structs are correctly parsed from query parameters.
- Added a dedicated test for embedded struct parsing in ctx_test.go.
- Refactored field type equality checks in ctx.go to handle recursive anonymous struct fields.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ctx_test.go | Added a new test function for validating embedded struct parsing. |
| ctx.go | Refactored equalFieldType to delegate to a new recursive helper function. |
Comments suppressed due to low confidence (1)
ctx_test.go:4888
- [nitpick] Consider adding additional test cases with deeper levels of nested embedded structs to further validate the recursive parsing behavior.
type Person struct {
|
@ReneWerner87 Seems the CI workflows didnt run? |
|
yeah, i will push another commit and then it start hopefully |
|
@ReneWerner87 Do we have this setup? https://docs.github.com/en/enterprise-cloud@latest/copilot/using-github-copilot/code-review/configuring-automatic-code-review-by-copilot#configuring-automatic-code-review-for-repositories-in-an-organization I have to manually trigger copilot all the time 😆 |
Fixes #2859