Conversation
…or Fiber middleware
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new extractors package (implementation, tests, README) and documentation (guide, whats_new) providing a metadata-bearing Extractor type, Source enum, ErrNotFound sentinel, concrete constructors (FromAuthHeader, FromCookie, FromParam, FromForm, FromHeader, FromQuery, FromCustom), and a Chain composer. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant App as Fiber App
participant Chain as Extractor Chain
participant E1 as Extractor[1]
participant E2 as Extractor[2]
participant E3 as Extractor[3]
Client->>App: HTTP request
App->>Chain: Extract(ctx)
activate Chain
Chain->>E1: Extract(ctx)
alt E1 returns value
E1-->>Chain: value, nil
Chain-->>App: value, nil
else E1 empty/error
E1-->>Chain: "", err/ErrNotFound
Chain->>E2: Extract(ctx)
alt E2 returns value
E2-->>Chain: value, nil
Chain-->>App: value, nil
else E2 empty/error
E2-->>Chain: "", err/ErrNotFound
Chain->>E3: Extract(ctx)
alt E3 returns value
E3-->>Chain: value, nil
Chain-->>App: value, nil
else all fail
E3-->>Chain: "", lastErr/ErrNotFound
Chain-->>App: "", lastErr/ErrNotFound
end
end
end
deactivate Chain
App-->>Client: continue or return error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. Comment |
There was a problem hiding this comment.
Summary of Changes
Hello @sixcolors, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a new, centralized extractors package within the Fiber framework. Its primary purpose is to consolidate and standardize the logic for extracting values from various HTTP request components, which was previously duplicated across multiple middleware packages. This change significantly improves code maintainability, reduces redundancy, and enhances the overall security posture by providing a single, well-tested, and consistent mechanism for data retrieval, laying the groundwork for a more streamlined and robust middleware ecosystem.
Highlights
- New Shared
extractorsPackage: Introducesgithub.com/gofiber/fiber/v3/extractorsto centralize value extraction logic. This package provides a consistent API for extracting values from various HTTP request sources (headers, cookies, query parameters, form data, URL parameters) and includes built-in chain/fallback logic. - Comprehensive Extraction Utilities: The new package includes core
Extractorinterface and concrete implementations such asFromAuthHeader(with scheme support),FromCookie,FromParam,FromForm,FromHeader,FromQuery, and aChainfunction to combine multiple extractors with fallback behavior. - Robust Testing and Security Considerations: A comprehensive test suite (
extractors_test.go) with 10 test functions ensures reliability across basic functionality, chain behavior, error propagation, and edge cases. The package also incorporates security warnings and source introspection capabilities to inform developers about potential risks associated with certain extraction sources. - Updated Documentation: New documentation files have been added:
README.mdprovides maintainer-focused details on architecture, testing, and maintenance, whileextractors.mdoffers user-focused guidance, usage examples, migration strategies, and security best practices. - Foundation for Middleware Migration: This PR establishes the necessary foundation for future migrations of existing middleware packages (e.g., session, csrf, keyauth) to utilize this new shared
extractorspackage, aiming to eliminate duplicated code and enhance consistency across the Fiber ecosystem.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3725 +/- ##
==========================================
- Coverage 92.05% 92.04% -0.01%
==========================================
Files 115 116 +1
Lines 11585 11697 +112
==========================================
+ Hits 10664 10767 +103
- Misses 668 674 +6
- Partials 253 256 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request introduces a well-structured extractors package to consolidate value extraction logic, which is a great improvement for code deduplication and maintainability. The implementation is solid, with comprehensive documentation and tests. I've identified a couple of areas for improvement: a potential robustness issue in the FromAuthHeader extractor and a resource leak pattern in the test files. Additionally, the Go version in go.mod seems to be set to a future release. Addressing these points will further strengthen this excellent contribution.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/guide/extractors.md (1)
102-103: Remove stray local path/param tag (leaks local path; breaks docs).Delete the accidental line(s).
-<parameter name="filePath">/Users/sixcolors/Documents/GitHub/fiber/docs/guide/extractors.md
🧹 Nitpick comments (7)
extractors/README.md (1)
52-57: Document empty-is-not-found + trimming semantics.Chain and individual extractors treat empty values as not found; make it explicit.
- Returns first successful extraction (non-empty value, no error) +- All extractors trim whitespace; empty results are treated as not founddocs/guide/extractors.md (1)
4-4: Frontmatter grammar: lower-case “use in”.-description: Shared value extraction utilities for Use In Middleware Packages +description: Shared value extraction utilities for use in middleware packagesextractors/extractors_test.go (2)
25-26: Close http.Response from app.Test to avoid leaks.Capture and close response bodies.
-_, err := app.Test(newRequest(fiber.MethodGet, "/test")) -require.NoError(t, err) +resp, err := app.Test(newRequest(fiber.MethodGet, "/test")) +require.NoError(t, err) +defer resp.Body.Close()-_, err := app.Test(newRequest(fiber.MethodGet, "/test/token_from_param")) -require.NoError(t, err) +resp, err := app.Test(newRequest(fiber.MethodGet, "/test/token_from_param")) +require.NoError(t, err) +defer resp.Body.Close()Also applies to: 79-81
57-64: Reduce AcquireCtx/ReleaseCtx repetition with a small helper.Add this helper and replace repeated acquire/defer blocks:
// test helper (place near newRequest) func withCtx(t *testing.T, app *fiber.App, fn func(fiber.Ctx)) { t.Helper() ctx := app.AcquireCtx(&fasthttp.RequestCtx{}) defer app.ReleaseCtx(ctx) fn(ctx) }extractors/extractors.go (3)
3-7: Clarify package visibility in doc commentThe package is exported (github.com/gofiber/fiber/v3/extractors), but the comment calls it “internal”. Suggest rewording to avoid confusion for users and godoc.
-// This internal package helps reduce code duplication across middleware packages -// while allowing selective inclusion of extractors based on middleware needs. +// This shared package reduces code duplication across middleware packages +// while allowing selective inclusion of extractors based on middleware needs.
15-40: Make the zero value of Source safe (introduce SourceUnknown)Right now, the zero value (iota=0) maps to SourceHeader. Any zero-valued Extractor would misleadingly appear as “header”-backed. Prefer a sentinel “unknown” first to make zero-value structs safer and clearer in introspection.
-// Source represents the type of source from which a value is extracted. -type Source int +// Source represents the type of source from which a value is extracted. +// Use a small underlying type to reduce struct size. +type Source uint8 @@ -const ( - // SourceHeader indicates the value is extracted from an HTTP header. - SourceHeader Source = iota +const ( + // SourceUnknown indicates the source is not specified. + SourceUnknown Source = iota + + // SourceHeader indicates the value is extracted from an HTTP header. + SourceHeader @@ - // SourceCustom indicates the value is extracted using a custom extractor function. + // SourceCustom indicates the value is extracted using a custom extractor function. SourceCustom )Follow-up: consider setting Chain(...) result Source to SourceCustom (metadata-only) if you decide not to inherit the first extractor’s metadata. See Chain comment below.
140-167: Document FormValue search order in FromForm and add strict post-only extractor
- Update the FromForm comment to note that c.FormValue searches QueryArgs → PostArgs → MultipartForm → FormFile in that order (pkg.go.dev).
- Consider introducing a companion FromPostForm that reads only body form fields (PostArgs/MultipartForm) to prevent query-based overrides.
-// FromForm creates an Extractor that retrieves a value from a specified form field in the request. +// FromForm creates an Extractor that retrieves a value from a form field using c.FormValue. // Note: FormValue searches in this order: QueryArgs, PostArgs, MultipartForm, FormFile. // A query parameter with the same name can override a body form field; use stricter APIs for secrets.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
extractors/go.modis excluded by!**/*.modextractors/go.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (4)
docs/guide/extractors.md(1 hunks)extractors/README.md(1 hunks)extractors/extractors.go(1 hunks)extractors/extractors_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Review and update the contents of the
docsfolder if necessary when modifying code
Files:
docs/guide/extractors.md
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Format Go code using gofumpt (enforced viamake format)
Ensure code passes golangci-lint checks (enforced viamake lint)
Optimize struct field alignment using betteralign (enforced viamake betteralign)
Modernize Go code using gopls modernize (enforced viamake modernize)
Files:
extractors/extractors.goextractors/extractors_test.go
🧠 Learnings (1)
📚 Learning: 2025-07-27T17:28:53.403Z
Learnt from: sixcolors
PR: gofiber/fiber#3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware `Config` struct, the `Extractor` field uses function closures (like `FromCookie(key)`), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
Applied to files:
extractors/README.mddocs/guide/extractors.mdextractors/extractors.goextractors/extractors_test.go
🧬 Code graph analysis (2)
extractors/extractors.go (1)
constants.go (1)
HeaderAuthorization(163-163)
extractors/extractors_test.go (2)
extractors/extractors.go (15)
FromParam(126-138)ErrNotFound(43-43)FromForm(155-167)FromQuery(209-221)FromHeader(178-190)FromAuthHeader(63-85)FromCookie(96-108)Chain(233-274)Extractor(46-52)Source(17-17)SourceCustom(39-39)SourceHeader(21-21)SourceQuery(30-30)SourceCookie(36-36)SourceAuthHeader(24-24)constants.go (4)
MethodGet(5-5)MIMEApplicationForm(28-28)MethodPost(7-7)HeaderAuthorization(163-163)
🪛 LanguageTool
extractors/README.md
[grammar] ~9-~9: There might be a mistake here.
Context: ...: Core extraction function with metadata - Source: Enumeration of extraction sources (Hea...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...der, Query, Form, Param, Cookie, Custom) - ErrNotFound: Standardized error for missing values ...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ...ing): Extract from Authorization header - FromCookie(key string): Extract from HTTP cookies - FromParam...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ...(key string): Extract from HTTP cookies - FromParam(param string): Extract from URL path parameters - Fr...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...ring): Extract from URL path parameters - FromForm(param string): Extract from form data - FromHeader(h...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...m(param string): Extract from form data - FromHeader(header string): Extract from custom HTTP headers - Fr...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...ring): Extract from custom HTTP headers - FromQuery(param string): Extract from URL query parameters - C...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...ing): Extract from URL query parameters - Chain(extractors ...Extractor)`: Chain multiple extractors with fallbac...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ...ain` function implements fallback logic: - Returns first successful extraction (non...
(QB_NEW_EN)
[grammar] ~54-~54: There might be a mistake here.
Context: ...n-empty value, no error) - If all fail, returns last error or ErrNotFound - Preserves...
(QB_NEW_EN)
[grammar] ~55-~55: There might be a mistake here.
Context: ...r or ErrNotFound - Preserves metadata from first extractor - Stores defensive copy...
(QB_NEW_EN)
[grammar] ~66-~66: There might be a mistake here.
Context: ...o test -v ./extractors ``` Tests cover: - Individual extractor functionality - Err...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...er: - Individual extractor functionality - Error handling and edge cases - Chained ...
(QB_NEW_EN)
[grammar] ~68-~68: There might be a mistake here.
Context: ...ionality - Error handling and edge cases - Chained extractor behavior - Security wa...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ... edge cases - Chained extractor behavior - Security warning propagation - Custom ex...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ... behavior - Security warning propagation - Custom extractor support - Error propaga...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ...g propagation - Custom extractor support - Error propagation in chains - Metadata a...
(QB_NEW_EN)
[grammar] ~72-~72: There might be a mistake here.
Context: ...or support - Error propagation in chains - Metadata and introspection ## Maintenan...
(QB_NEW_EN)
docs/guide/extractors.md
[grammar] ~31-~31: There might be a mistake here.
Context: ...metadata (source type, key, auth scheme) - Source: Enumeration of extraction sources (Hea...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...uery, Form, Param, Cookie, Custom, etc.) - ErrNotFound: Standardized error for missing values ...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...uthorization header with optional scheme - FromCookie(key string): Extract from HTTP cookies - `FromParam...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ...(key string): Extract from HTTP cookies - FromParam(param string): Extract from URL path parameters - Fr...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...ring): Extract from URL path parameters - FromForm(param string): Extract from form data - FromHeader(h...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...m(param string): Extract from form data - FromHeader(header string): Extract from custom HTTP headers - Fr...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...ring): Extract from custom HTTP headers - FromQuery(param string): Extract from URL query parameters - C...
(QB_NEW_EN)
[grammar] ~42-~42: There might be a mistake here.
Context: ...ing): Extract from URL query parameters - Chain(extractors ...Extractor)`: Chain multiple extractors with fallbac...
(QB_NEW_EN)
[grammar] ~48-~48: There might be a mistake here.
Context: ...e actual extraction from a Fiber context - Key: The parameter/header name used for ext...
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...arameter/header name used for extraction - Source: The type of source being extracted fro...
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ... be inspected for security restrictions) - AuthScheme: The auth scheme used (for `FromAuthHea...
(QB_NEW_EN)
[grammar] ~51-~51: There might be a mistake here.
Context: ... auth scheme used (for FromAuthHeader) - Chain: For chained extractors, stores all ext...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...tors that try multiple sources in order: - Returns the first successful extraction ...
(QB_NEW_EN)
🪛 GitHub Check: markdownlint
extractors/README.md
[failure] 86-86: Lists should be surrounded by blank lines
extractors/README.md:86 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "- Additional extraction source..."] https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md032.md
[failure] 67-67: Lists should be surrounded by blank lines
extractors/README.md:67 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "- Individual extractor functio..."] https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md032.md
[failure] 53-53: Lists should be surrounded by blank lines
extractors/README.md:53 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "- Returns first successful ext..."] https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md032.md
docs/guide/extractors.md
[failure] 57-57: Lists should be surrounded by blank lines
docs/guide/extractors.md:57 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "- Returns the first successful..."] https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md032.md
[failure] 48-48: Lists should be surrounded by blank lines
docs/guide/extractors.md:48 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "- Extract: Function that per..."] https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md032.md
🪛 GitHub Actions: markdownlint
docs/guide/extractors.md
[error] 48-48: markdownlint-cli2-action: MD032/blanks-around-lists - Lists should be surrounded by blank lines.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: unit (1.25.x, macos-latest)
- GitHub Check: Compare
- GitHub Check: repeated
- GitHub Check: unit (1.25.x, macos-13)
- GitHub Check: unit (1.25.x, windows-latest)
🔇 Additional comments (5)
extractors/extractors_test.go (1)
66-375: Solid coverage and behavior verification.Tests exercise all sources, chain semantics (zero/first-success/last-error), case-insensitive auth scheme, and introspection. Looks good.
extractors/extractors.go (4)
42-44: LGTM: exported sentinel errorStable, comparable with errors.Is, and appropriate for signaling absence distinctly from hard failures.
45-52: Nice: struct + metadata solves prior introspection gapThis addresses the learned pain point where closures hid parameters in session.Config Extractor fields. Public metadata (Key, Source, AuthScheme, Chain) enables validation and diagnostics without reflection.
54-85: LGTM: Authorization extractor behaviorCase-insensitive scheme check, tolerant token splitting, and ErrNotFound on mismatch enable clean fallback in Chain.
87-108: LGTM: Cookie/Param/Header/Query extractorsConsistent empty-check semantics and ErrNotFound behavior look good.
Also applies to: 110-138, 169-190, 192-221
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/guide/extractors.md (1)
104-105: Remove stray local path/tag leakage.This parameter tag leaks a local path and will break rendering.
-<parameter name="filePath">/Users/sixcolors/Documents/GitHub/fiber/docs/guide/extractors.md
♻️ Duplicate comments (4)
extractors/README.md (2)
52-58: Fix markdownlint MD032: ensure blank lines around lists.Add a blank line after each list block (and keep one before) to unblock docs CI.
Example:
The `Chain` function implements fallback logic: - Returns first successful extraction (non-empty value, no error) - If all fail, returns last error or `ErrNotFound` - Preserves metadata from first extractor - Stores defensive copy for introspection +Repeat after the lists ending at Lines 76, 84, and 92.
Also applies to: 67-76, 79-84, 86-92
10-11: Doc/API mismatch and misleading security wording: add SourceAuthHeader; neutralize comments.
- The Source enum list omits AuthHeader even though the API exposes it.
- The switch sample implies headers/cookies are “secure.” All request inputs are client-controlled; prefer neutral risk notes.
Apply:
- - `Source`: Enumeration of extraction sources (Header, Query, Form, Param, Cookie, Custom) + - `Source`: Enumeration of extraction sources (Header, AuthHeader, Query, Form, Param, Cookie, Custom)switch extractor.Source { case SourceQuery: - // Query parameters - potential security risk + // Query parameters — higher leak risk (logs, referrers, history). Avoid for secrets. +case SourceAuthHeader: + // Authorization header; if a scheme is configured, enforce and strip it. case SourceCookie: - // Cookies - generally secure + // Cookies (consider HttpOnly/Secure/SameSite); still client-controlled input. case SourceHeader: - // Headers - secure + // Headers are client-controlled; validate and normalize as needed. }Also applies to: 35-47
docs/guide/extractors.md (2)
57-64: Fix MD032: add a blank line after the list.A blank line is required after the Chain behavior list.
- Stores a defensive copy of all chained extractors for introspection via the `Chain` field - + ## Usage Examples
31-33: Enumerate SourceAuthHeader and remove “etc.”Match the public API and avoid imprecise “etc.”
- - `Source`: Enumeration of extraction sources (Header, Query, Form, Param, Cookie, Custom, etc.) + - `Source`: Enumeration of extraction sources (Header, AuthHeader, Query, Form, Param, Cookie, Custom)
🧹 Nitpick comments (4)
extractors/README.md (1)
25-34: Clarify FromAuthHeader behavior (scheme enforcement/stripping).State explicitly that a non-empty scheme enforces a case-insensitive “ ” prefix and strips it from the returned value; empty scheme returns header value as-is.
Apply:
- - `FromAuthHeader(authScheme string)`: Extract from Authorization header + - `FromAuthHeader(authScheme string)`: Extract from Authorization header; when `authScheme` is non-empty, enforce and strip the "<Scheme> " prefix (case-insensitive). When empty, return the header value as-is.docs/guide/extractors.md (3)
94-101: Broaden security notes; add headers/cookies guidance and AuthHeader caveat.Add concise guidance for Authorization header and cookies; note they’re client-controlled and may leak via logs.
## Security Considerations @@ - **URL Parameters**: Can leak values through logs and browser history +- **Authorization Header**: Prefer scheme-enforced tokens (e.g., Bearer). Avoid logging raw header values. +- **Cookies**: Use HttpOnly/Secure/SameSite; still client-controlled and may appear in logs if not filtered.
4-4: Tighten description sentence casing.Use sentence case.
-description: Shared value extraction utilities for Use In Middleware Packages +description: Shared value extraction utilities for use in middleware packages
37-44: Clarify FromAuthHeader semantics in user docs.Mirror README: scheme enforcement/stripping behavior.
- - `FromAuthHeader(authScheme string)`: Extract from Authorization header with optional scheme + - `FromAuthHeader(authScheme string)`: Extract from Authorization header; when `authScheme` is non-empty, enforce and strip the "<Scheme> " prefix (case-insensitive). When empty, return the header value as-is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/guide/extractors.md(1 hunks)extractors/README.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Review and update the contents of the
docsfolder if necessary when modifying code
Files:
docs/guide/extractors.md
🧠 Learnings (1)
📚 Learning: 2025-07-27T17:28:53.403Z
Learnt from: sixcolors
PR: gofiber/fiber#3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware `Config` struct, the `Extractor` field uses function closures (like `FromCookie(key)`), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
Applied to files:
docs/guide/extractors.mdextractors/README.md
🪛 LanguageTool
docs/guide/extractors.md
[grammar] ~31-~31: There might be a mistake here.
Context: ...metadata (source type, key, auth scheme) - Source: Enumeration of extraction sources (Hea...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...uery, Form, Param, Cookie, Custom, etc.) - ErrNotFound: Standardized error for missing values ...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...uthorization header with optional scheme - FromCookie(key string): Extract from HTTP cookies - `FromParam...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ...(key string): Extract from HTTP cookies - FromParam(param string): Extract from URL path parameters - Fr...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...ring): Extract from URL path parameters - FromForm(param string): Extract from form data - FromHeader(h...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...m(param string): Extract from form data - FromHeader(header string): Extract from custom HTTP headers - Fr...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...ring): Extract from custom HTTP headers - FromQuery(param string): Extract from URL query parameters - C...
(QB_NEW_EN)
[grammar] ~42-~42: There might be a mistake here.
Context: ...ing): Extract from URL query parameters - Chain(extractors ...Extractor)`: Chain multiple extractors with fallbac...
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...e actual extraction from a Fiber context - Key: The parameter/header name used for ext...
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ...arameter/header name used for extraction - Source: The type of source being extracted fro...
(QB_NEW_EN)
[grammar] ~51-~51: There might be a mistake here.
Context: ... be inspected for security restrictions) - AuthScheme: The auth scheme used (for `FromAuthHea...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ... auth scheme used (for FromAuthHeader) - Chain: For chained extractors, stores all ext...
(QB_NEW_EN)
extractors/README.md
[grammar] ~9-~9: There might be a mistake here.
Context: ...: Core extraction function with metadata - Source: Enumeration of extraction sources (Hea...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...der, Query, Form, Param, Cookie, Custom) - ErrNotFound: Standardized error for missing values ...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ...ing): Extract from Authorization header - FromCookie(key string): Extract from HTTP cookies - FromParam...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ...(key string): Extract from HTTP cookies - FromParam(param string): Extract from URL path parameters - Fr...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...ring): Extract from URL path parameters - FromForm(param string): Extract from form data - FromHeader(h...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...m(param string): Extract from form data - FromHeader(header string): Extract from custom HTTP headers - Fr...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...ring): Extract from custom HTTP headers - FromQuery(param string): Extract from URL query parameters - C...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...ing): Extract from URL query parameters - Chain(extractors ...Extractor)`: Chain multiple extractors with fallbac...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ...r: - Individual extractor functionality - Error handling and edge cases - Chained ...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...ionality - Error handling and edge cases - Chained extractor behavior - Security wa...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ... edge cases - Chained extractor behavior - Security warning propagation - Custom ex...
(QB_NEW_EN)
[grammar] ~72-~72: There might be a mistake here.
Context: ... behavior - Security warning propagation - Custom extractor support - Error propaga...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...g propagation - Custom extractor support - Error propagation in chains - Metadata a...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...or support - Error propagation in chains - Metadata and introspection ## Maintenan...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: repeated
- GitHub Check: Compare
🔇 Additional comments (2)
docs/guide/extractors.md (2)
69-92: Usage examples look good.Imports and constructors align; chain order is sensible.
29-34: Extractor and Source types are structs/ints in code, matching the docs; no changes required.
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 66e74f6 | Previous: e83a762 | Ratio |
|---|---|---|---|
Benchmark_Compress_Levels_Parallel/Brotli_LevelBestCompression - B/op |
1 B/op |
0 B/op |
+∞ |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
extractors/README.md (1)
9-11: Doc/code mismatch: include AuthHeader in Source enum list.Align the docs with the exported Source values.
-- `Source`: Enumeration of extraction sources (Header, Query, Form, Param, Cookie, Custom) +- `Source`: Enumeration of extraction sources (Header, AuthHeader, Query, Form, Param, Cookie, Custom)
🧹 Nitpick comments (6)
docs/guide/extractors.md (3)
4-4: Fix title-case in description.Use sentence case: “for use in middleware packages.”
-description: Shared value extraction utilities for Use In Middleware Packages +description: Shared value extraction utilities for use in middleware packages
37-44: Clarify AuthHeader parsing semantics (case-insensitive scheme, 1+ whitespace).Match the implementation and set expectations.
-- `FromAuthHeader(authScheme string)`: Extract from Authorization header with optional scheme +- `FromAuthHeader(authScheme string)`: Extract from Authorization header. If `authScheme` is non-empty, + the scheme match is case-insensitive and requires at least one whitespace before the token; the + scheme prefix is stripped from the returned value.
94-103: Add cookie security note.Document HttpOnly/Secure guidance alongside other warnings.
## Security Considerations @@ - **URL Parameters**: Can leak values through logs and browser history + +- **Cookies**: Prefer HttpOnly and Secure flags; avoid storing long-lived secrets client-side.extractors/extractors.go (2)
3-6: Doc comment says “internal package” but this is public.Adjust wording to avoid confusion.
-// This internal package helps reduce code duplication across middleware packages +// This package helps reduce code duplication across middleware packages
247-275: Chain error semantics: preserve the earliest hard error (don’t overwrite with ErrNotFound).Returning the last error can mask a prior validation/parsing error if a later extractor returns ErrNotFound.
- Extract: func(c fiber.Ctx) (string, error) { - var lastErr error // last error encountered (including ErrNotFound) + Extract: func(c fiber.Ctx) (string, error) { + var firstHardErr error // first non-ErrNotFound error + var lastErr error // track any error (including ErrNotFound) for fallback for _, extractor := range extractors { if extractor.Extract == nil { continue } v, err := extractor.Extract(c) if err == nil && v != "" { return v, nil } - if err != nil { - lastErr = err - } + if err != nil { + if firstHardErr == nil && !errors.Is(err, ErrNotFound) { + firstHardErr = err + } + lastErr = err + } } - if lastErr != nil { + if firstHardErr != nil { + return "", firstHardErr + } + if lastErr != nil { return "", lastErr } return "", ErrNotFound },Would you like to keep “last error wins” for compatibility with existing middleware? If so, consider documenting that rationale explicitly.
extractors/README.md (1)
60-66: Optionally document scheme parsing details.Add one sentence about case-insensitive scheme and 1+ whitespace to mirror code.
-The `Chain` function implements fallback logic: +The `Chain` function implements fallback logic: +(Note: `FromAuthHeader` matches scheme case-insensitively and accepts one or more spaces/tabs after the scheme.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
docs/guide/extractors.md(1 hunks)extractors/README.md(1 hunks)extractors/extractors.go(1 hunks)extractors/extractors_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- extractors/extractors_test.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Format Go code using gofumpt (enforced viamake format)
Ensure code passes golangci-lint checks (enforced viamake lint)
Optimize struct field alignment using betteralign (enforced viamake betteralign)
Modernize Go code using gopls modernize (enforced viamake modernize)
Files:
extractors/extractors.go
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Review and update the contents of the
docsfolder if necessary when modifying code
Files:
docs/guide/extractors.md
🧠 Learnings (1)
📚 Learning: 2025-07-27T17:28:53.403Z
Learnt from: sixcolors
PR: gofiber/fiber#3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware `Config` struct, the `Extractor` field uses function closures (like `FromCookie(key)`), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
Applied to files:
extractors/extractors.godocs/guide/extractors.mdextractors/README.md
🧬 Code graph analysis (1)
extractors/extractors.go (2)
ctx_interface_gen.go (1)
Ctx(17-414)constants.go (1)
HeaderAuthorization(163-163)
🪛 LanguageTool
docs/guide/extractors.md
[grammar] ~31-~31: There might be a mistake here.
Context: ...metadata (source type, key, auth scheme) - Source: Enumeration of extraction sources (Hea...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...der, Query, Form, Param, Cookie, Custom) - ErrNotFound: Standardized error for missing values ...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...uthorization header with optional scheme - FromCookie(key string): Extract from HTTP cookies - `FromParam...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ...(key string): Extract from HTTP cookies - FromParam(param string): Extract from URL path parameters - Fr...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...ring): Extract from URL path parameters - FromForm(param string): Extract from form data - FromHeader(h...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...m(param string): Extract from form data - FromHeader(header string): Extract from custom HTTP headers - Fr...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...ring): Extract from custom HTTP headers - FromQuery(param string): Extract from URL query parameters - C...
(QB_NEW_EN)
[grammar] ~42-~42: There might be a mistake here.
Context: ...ing): Extract from URL query parameters - Chain(extractors ...Extractor)`: Chain multiple extractors with fallbac...
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...e actual extraction from a Fiber context - Key: The parameter/header name used for ext...
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ...arameter/header name used for extraction - Source: The type of source being extracted fro...
(QB_NEW_EN)
[grammar] ~51-~51: There might be a mistake here.
Context: ... be inspected for security restrictions) - AuthScheme: The auth scheme used (for `FromAuthHea...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ... auth scheme used (for FromAuthHeader) - Chain: For chained extractors, stores all ext...
(QB_NEW_EN)
extractors/README.md
[grammar] ~9-~9: There might be a mistake here.
Context: ...: Core extraction function with metadata - Source: Enumeration of extraction sources (Hea...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...der, Query, Form, Param, Cookie, Custom) - ErrNotFound: Standardized error for missing values ...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ...ing): Extract from Authorization header - FromCookie(key string): Extract from HTTP cookies - FromParam...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ...(key string): Extract from HTTP cookies - FromParam(param string): Extract from URL path parameters - Fr...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...ring): Extract from URL path parameters - FromForm(param string): Extract from form data - FromHeader(h...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...m(param string): Extract from form data - FromHeader(header string): Extract from custom HTTP headers - Fr...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...ring): Extract from custom HTTP headers - FromQuery(param string): Extract from URL query parameters - C...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...ing): Extract from URL query parameters - Chain(extractors ...Extractor)`: Chain multiple extractors with fallbac...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ...r: - Individual extractor functionality - Error handling and edge cases - Chained ...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ...ionality - Error handling and edge cases - Chained extractor behavior - Security wa...
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ... edge cases - Chained extractor behavior - Security warning propagation - Custom ex...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ... behavior - Security warning propagation - Custom extractor support - Error propaga...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ...g propagation - Custom extractor support - Error propagation in chains - Metadata a...
(QB_NEW_EN)
[grammar] ~82-~82: There might be a mistake here.
Context: ...or support - Error propagation in chains - Metadata and introspection ## Maintenan...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: unit (1.25.x, macos-13)
- GitHub Check: unit (1.25.x, macos-latest)
- GitHub Check: Compare
- GitHub Check: unit (1.25.x, windows-latest)
- GitHub Check: repeated
🔇 Additional comments (4)
docs/guide/extractors.md (1)
31-33: Source enum listing looks correct and complete.Includes AuthHeader explicitly; resolves prior ambiguity.
extractors/extractors.go (2)
19-40: Source enum is well-structured and exhaustive.AuthHeader explicitly separated from generic Header.
89-223: Extractor constructors are consistent and trim/ErrNotFound semantics are clear.Good use of Fiber’s Ctx getters and uniform empty-value handling.
extractors/README.md (1)
39-56: Source inspection section looks accurate.Includes SourceAuthHeader and neutral security phrasing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
docs/whats_new.md (4)
34-34: Normalize casing for consistency in the overview listUse sentence case like other items (e.g., “Client package”).
- - [🔎 Extractors Package](#-extractors-package) + - [🔎 Extractors package](#-extractors-package)
679-688: Add cross-link to the new Extractors guide and mention ErrNotFoundDirect readers to the dedicated guide and call out the common sentinel error.
-Fiber v3 introduces a new shared `extractors` package that consolidates value extraction utilities previously duplicated across middleware packages. This package provides a unified API for extracting values from headers, cookies, query parameters, form data, and URL parameters with built-in chain/fallback logic and security considerations. +Fiber v3 introduces a new shared `extractors` package that consolidates value extraction utilities previously duplicated across middleware packages. This package provides a unified API for extracting values from headers, cookies, query parameters, form data, and URL parameters with built-in chain/fallback logic and security considerations. See the [Extractors Guide](./guide/extractors.md) for complete API and migration details, including `ErrNotFound` usage patterns. @@ - **Source Awareness**: Source inspection capabilities for security-sensitive operations +- **Sentinel Error**: Consistent `ErrNotFound` when a value is missing - **Type Safety**: Strongly typed extraction with proper error handling - **Performance**: Optimized extraction functions with minimal overhead
701-719: Prefer Fiber status constants in examplesUse
fiber.StatusUnauthorizedfor consistency with other examples in this page.- if err != nil { - return c.Status(401).SendString("API key required") - } + if err != nil { + return c.Status(fiber.StatusUnauthorized).SendString("API key required") + }
763-767: Fix brand and grammar in the Client package blurbTighten phrasing and correct “take a look to”.
-## 🌎 Client package - -The Gofiber client has been completely rebuilt. It includes numerous new features such as Cookiejar, request/response hooks, and more. -You can take a look to [client docs](./client/rest.md) to see what's new with the client. +## 🌎 Client package + +The Fiber client has been completely rebuilt. It includes numerous new features such as Cookiejar, request/response hooks, and more. +See the [client docs](./client/rest.md) for what's new.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/whats_new.md(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Review and update the contents of the
docsfolder if necessary when modifying code
Files:
docs/whats_new.md
🧠 Learnings (1)
📚 Learning: 2024-11-15T07:56:21.623Z
Learnt from: ReneWerner87
PR: gofiber/fiber#3161
File: app.go:923-932
Timestamp: 2024-11-15T07:56:21.623Z
Learning: In the Fiber framework, breaking changes are acceptable when moving from version 2 to version 3, including modifications to method signatures such as in the `Test` method in `app.go`.
Applied to files:
docs/whats_new.md
🪛 LanguageTool
docs/whats_new.md
[grammar] ~34-~34: There might be a mistake here.
Context: ...ing](#-binding) - 🔎 Extractors Package - 🔄️ Redirect - [🌎 Client p...
(QB_NEW_EN)
[grammar] ~677-~677: There might be a mistake here.
Context: ...g.
(QB_NEW_EN)
[grammar] ~683-~683: There might be a mistake here.
Context: ...arameters, form data, and URL parameters - Chain Logic: Built-in fallback mechani...
(QB_NEW_EN)
[grammar] ~684-~684: There might be a mistake here.
Context: ...try multiple extraction sources in order - Source Awareness: Source inspection ca...
(QB_NEW_EN)
[grammar] ~685-~685: There might be a mistake here.
Context: ...lities for security-sensitive operations - Type Safety: Strongly typed extraction...
(QB_NEW_EN)
[grammar] ~686-~686: There might be a mistake here.
Context: ...ed extraction with proper error handling - Performance: Optimized extraction func...
(QB_NEW_EN)
[grammar] ~691-~691: There might be a mistake here.
Context: ...Authorization header with scheme support - FromCookie(key string): Extract from HTTP cookies - `FromParam...
(QB_NEW_EN)
[grammar] ~692-~692: There might be a mistake here.
Context: ...(key string): Extract from HTTP cookies - FromParam(param string): Extract from URL path parameters - Fr...
(QB_NEW_EN)
[grammar] ~693-~693: There might be a mistake here.
Context: ...ring): Extract from URL path parameters - FromForm(param string): Extract from form data - FromHeader(h...
(QB_NEW_EN)
[grammar] ~694-~694: There might be a mistake here.
Context: ...m(param string): Extract from form data - FromHeader(header string): Extract from custom HTTP headers - Fr...
(QB_NEW_EN)
[grammar] ~695-~695: There might be a mistake here.
Context: ...ring): Extract from custom HTTP headers - FromQuery(param string): Extract from URL query parameters - C...
(QB_NEW_EN)
[grammar] ~696-~696: There might be a mistake here.
Context: ...ing): Extract from URL query parameters - Chain(extractors ...Extractor)`: Chain multiple extractors with fallbac...
(QB_NEW_EN)
[grammar] ~725-~725: There might be a mistake here.
Context: ...500+ lines of duplicated extraction code - Consistency: Standardized extraction b...
(QB_NEW_EN)
[grammar] ~726-~726: There might be a mistake here.
Context: ...xtraction behavior across all middleware - Maintainability: Single source of trut...
(QB_NEW_EN)
[grammar] ~727-~727: There might be a mistake here.
Context: ...gle source of truth for extraction logic - Security: Unified security considerati...
(QB_NEW_EN)
[grammar] ~763-~763: There might be a mistake here.
Context: ...ent browsers. ::: ## 🌎 Client package The Gofiber client has been completely r...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Compare
- GitHub Check: unit (1.25.x, macos-13)
- GitHub Check: unit (1.25.x, windows-latest)
- GitHub Check: unit (1.25.x, macos-latest)
- GitHub Check: repeated
🔇 Additional comments (1)
docs/whats_new.md (1)
691-699: Documentation matches implementation TheFromAuthHeader(authScheme string)signature in docs aligns withfunc FromAuthHeader(authScheme string)inextractors/extractors.go.
|
/gemini-code-assist review |
|
@coderabbitai review |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/guide/extractors.md (1)
302-312: Fix case-sensitivity guidance (headers vs others)Header names are case-insensitive; other keys are generally case-sensitive.
-2. Verify the parameter name matches exactly (case-sensitive) +2. Verify the key name matches exactly (headers are case-insensitive; params/cookies/query keys are case-sensitive)
🧹 Nitpick comments (6)
extractors/extractors.go (3)
73-80: Reorder fields for better alignment; run make betteralignReordering shrinks the struct and satisfies our betteralign gate.
type Extractor struct { - Extract func(fiber.Ctx) (string, error) - Key string // The parameter/header name used for extraction - AuthScheme string // The auth scheme used, e.g., "Bearer" - Chain []Extractor // For chained extractors, stores all extractors in the chain - Source Source // The type of source being extracted from + Extract func(fiber.Ctx) (string, error) + Chain []Extractor // For chained extractors, stores all extractors in the chain + Key string // The parameter/header name used for extraction + AuthScheme string // The auth scheme used, e.g., "Bearer" + Source Source // The type of source being extracted from }
237-241: Surface path-unescape errors instead of conflating with not-foundReturning ErrNotFound hides malformed input; bubble the decode error.
- unescapedValue, err := url.PathUnescape(value) - if err != nil { - return "", ErrNotFound - } + unescapedValue, err := url.PathUnescape(value) + if err != nil { + return "", err + }
488-507: Chain error semantics: preserve first hard error, not the lastPrefer first non-ErrNotFound error to avoid masking earlier validation failures. Skips nil already—good.
- var lastErr error // last error encountered (including ErrNotFound) + var firstHardErr error // first error that is not ErrNotFound for _, extractor := range extractors { if extractor.Extract == nil { continue } v, err := extractor.Extract(c) if err == nil && v != "" { return v, nil } - if err != nil { - lastErr = err - } + if err != nil && !errors.Is(err, ErrNotFound) && firstHardErr == nil { + firstHardErr = err + } } - if lastErr != nil { - return "", lastErr + if firstHardErr != nil { + return "", firstHardErr } return "", ErrNotFoundNote: If you adopt this, update docs to reflect “first hard error” behavior.
extractors/README.md (1)
63-71: Sync Chain docs with proposed “first hard error” semanticsIf you accept the Chain change, adjust wording accordingly.
-The `Chain` function implements fallback logic: +The `Chain` function implements fallback logic: - Returns first successful extraction (non-empty value, no error) -- If all extractors fail, returns the last error encountered or `ErrNotFound` +- If all extractors fail, returns the first non-`ErrNotFound` error encountered; otherwise `ErrNotFound` - **Skips extractors with `nil` Extract functions** (graceful error handling) - Preserves metadata from first extractor for introspection - Stores defensive copy for runtime inspection via the `Chain` fielddocs/guide/extractors.md (2)
285-299: Standards note: mention trimming of trailing OWS after schemeIf you adopt the FromAuthHeader trim, reflect it here for clarity.
- **Flexible whitespace handling**: Supports spaces and tabs after scheme + - **OWS handling**: Trims trailing spaces/tabs around the credentials per RFC 7230
64-71: Chain docs: align with “first hard error” if implementedMirror the proposed behavior to avoid confusion.
- - If all extractors fail, returns the last error encountered or `ErrNotFound` + - If all extractors fail, returns the first non-`ErrNotFound` error encountered; otherwise `ErrNotFound`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
docs/guide/extractors.md(1 hunks)extractors/README.md(1 hunks)extractors/extractors.go(1 hunks)extractors/extractors_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- extractors/extractors_test.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Format Go code using gofumpt (enforced viamake format)
Ensure code passes golangci-lint checks (enforced viamake lint)
Optimize struct field alignment using betteralign (enforced viamake betteralign)
Modernize Go code using gopls modernize (enforced viamake modernize)
Files:
extractors/extractors.go
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Review and update the contents of the
docsfolder if necessary when modifying code
Files:
docs/guide/extractors.md
🧠 Learnings (4)
📚 Learning: 2025-09-01T07:16:43.444Z
Learnt from: CR
PR: gofiber/fiber#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T07:16:43.444Z
Learning: Applies to **/*.go : Optimize struct field alignment using betteralign (enforced via `make betteralign`)
Applied to files:
extractors/extractors.go
📚 Learning: 2024-12-01T10:28:36.011Z
Learnt from: ReneWerner87
PR: gofiber/fiber#0
File: :0-0
Timestamp: 2024-12-01T10:28:36.011Z
Learning: Feature request #3224 has been created to add support for square bracket notation and comma-separated values in multipart form data in Fiber, while maintaining binary data transfer capabilities. This would bring parity with the existing form-urlencoded functionality.
Applied to files:
extractors/extractors.go
📚 Learning: 2025-07-27T17:28:53.403Z
Learnt from: sixcolors
PR: gofiber/fiber#3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware `Config` struct, the `Extractor` field uses function closures (like `FromCookie(key)`), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
Applied to files:
extractors/extractors.godocs/guide/extractors.mdextractors/README.md
📚 Learning: 2025-07-19T14:06:29.884Z
Learnt from: sixcolors
PR: gofiber/fiber#3598
File: docs/middleware/csrf.md:37-42
Timestamp: 2025-07-19T14:06:29.884Z
Learning: In Fiber v3, the CookieSameSite constants use lowercase values: CookieSameSiteLaxMode = "lax", CookieSameSiteStrictMode = "strict", CookieSameSiteNoneMode = "none". Documentation examples should use lowercase string values or the typed constants, not capitalized strings like "Lax".
Applied to files:
docs/guide/extractors.md
🧬 Code graph analysis (1)
extractors/extractors.go (1)
constants.go (1)
HeaderAuthorization(163-163)
🪛 LanguageTool
docs/guide/extractors.md
[grammar] ~15-~15: There might be a mistake here.
Context: ...plementations across middleware packages - Ensures Consistency: Maintains identic...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...security practices across all extractors - Simplifies Maintenance: Changes to ext...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ... logic only need to be made in one place - Enables Direct Usage: Middleware can i...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...e can import and use extractors directly - Improves Performance: Shared, optimize...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...uthorization header with optional scheme - FromCookie(key string): Extract from HTTP cookies - `FromParam...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...(key string): Extract from HTTP cookies - FromParam(param string): Extract from URL path parameters - Fr...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...ring): Extract from URL path parameters - FromForm(param string): Extract from form data - FromHeader(h...
(QB_NEW_EN)
[grammar] ~36-~36: There might be a mistake here.
Context: ...m(param string): Extract from form data - FromHeader(header string): Extract from custom HTTP headers - Fr...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...ring): Extract from custom HTTP headers - FromQuery(param string): Extract from URL query parameters - F...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ...ing): Extract from URL query parameters - FromCustom(key string, fn func(fiber.Ctx) (string, error))`: Define custom extraction logic with me...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...ne custom extraction logic with metadata - Chain(extractors ...Extractor): Chain multiple extractors with fallbac...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...horization, X-API-Key`, custom headers - Cookies: Session cookies, authenticati...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ...: Session cookies, authentication tokens - Query Parameters: URL parameters like ...
(QB_NEW_EN)
[grammar] ~58-~58: There might be a mistake here.
Context: ...Query Parameters**: URL parameters like ?token=abc123 - Form Data: POST body form fields - **U...
(QB_NEW_EN)
[grammar] ~59-~59: There might be a mistake here.
Context: ...` - Form Data: POST body form fields - URL Parameters: Route parameters like ...
(QB_NEW_EN)
[grammar] ~174-~174: There might be a mistake here.
Context: ... authentication tokens, widely supported - Custom Headers: Application-specific, ...
(QB_NEW_EN)
[grammar] ~175-~175: There might be a mistake here.
Context: ...fic, less likely to be logged by default - Considerations: Can be intercepted wit...
(QB_NEW_EN)
[grammar] ~180-~180: There might be a mistake here.
Context: ... Designed for secure client-side storage - Considerations: Require proper `Secure...
(QB_NEW_EN)
[grammar] ~181-~181: There might be a mistake here.
Context: ...ecure, HttpOnly, and SameSite` flags - Best for: Session management, remember...
(QB_NEW_EN)
[grammar] ~186-~186: There might be a mistake here.
Context: ...Convenient for simple APIs and debugging - Considerations: Always visible in URLs...
(QB_NEW_EN)
[grammar] ~187-~187: There might be a mistake here.
Context: ...rvers/proxies, stored in browser history - Best for: Non-sensitive parameters, pu...
(QB_NEW_EN)
[grammar] ~192-~192: There might be a mistake here.
Context: ...le for form submissions and API requests - Considerations: Avoid putting sensitiv...
(QB_NEW_EN)
[grammar] ~364-~364: There might be a mistake here.
Context: ...xtractors.ErrNotFound }) ``` :::warning **Custom extractors break source awarene...
(QB_NEW_EN)
[grammar] ~367-~367: There might be a mistake here.
Context: ...nings** for potentially insecure sources - No source-based logging or monitoring ...
(QB_NEW_EN)
[grammar] ~368-~368: There might be a mistake here.
Context: ...sed logging** or monitoring capabilities - Developer responsibility for ensuring ...
(QB_NEW_EN)
extractors/README.md
[grammar] ~13-~13: There might be a mistake here.
Context: ...: Core extraction function with metadata - Source: Enumeration of extraction sources (Hea...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...der, Query, Form, Param, Cookie, Custom) - ErrNotFound: Standardized error for missing values ...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...uthorization header with optional scheme - FromCookie(key string): Extract from HTTP cookies - `FromParam...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...(key string): Extract from HTTP cookies - FromParam(param string): Extract from URL path parameters - Fr...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...ring): Extract from URL path parameters - FromForm(param string): Extract from form data - FromHeader(h...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...m(param string): Extract from form data - FromHeader(header string): Extract from custom HTTP headers - Fr...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...ring): Extract from custom HTTP headers - FromQuery(param string): Extract from URL query parameters - F...
(QB_NEW_EN)
[grammar] ~36-~36: There might be a mistake here.
Context: ...ing): Extract from URL query parameters - FromCustom(key string, fn func(fiber.Ctx) (string, error))`: Define custom extraction logic with me...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...ne custom extraction logic with metadata - Chain(extractors ...Extractor): Chain multiple extractors with fallbac...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...ul error handling) - Preserves metadata from first extractor for introspection - Sto...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ...or functionality across all source types - Error handling and edge cases (whitespac...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ...espace, empty values, malformed headers) - Chained extractor behavior and error pro...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...extractor behavior and error propagation - Custom extractor support including nil f...
(QB_NEW_EN)
[grammar] ~98-~98: There might be a mistake here.
Context: ... support including nil function handling - RFC 7235 compliance for Authorization he...
(QB_NEW_EN)
[grammar] ~99-~99: There might be a mistake here.
Context: ...pliance for Authorization header parsing - Metadata validation and source introspec...
(QB_NEW_EN)
[grammar] ~100-~100: There might be a mistake here.
Context: ...data validation and source introspection - Chain ordering and fallback logic (compr...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Compare
- GitHub Check: unit (1.25.x, windows-latest)
- GitHub Check: unit (1.25.x, macos-latest)
- GitHub Check: repeated
🔇 Additional comments (2)
extractors/extractors.go (1)
47-71: LGTM: Source enum and ErrNotFound sentinelAPI shape is clear and consistent with usage across middleware.
extractors/README.md (1)
29-39: Doc/code promise: “whitespace-only ⇒ ErrNotFound”README claims whitespace-only returns ErrNotFound for several extractors; ensure code adopts the trim-check changes (cookie/form/header/query) so docs remain accurate.
gaby
left a comment
There was a problem hiding this comment.
The docs/guide/extractors.md needs to be refactor. It's assuming that extractors is a separate package, which it isn't anymore
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
extractors/README.md (1)
85-111: Per maintainer feedback, drop Testing and Maintenance sections from this README.These are covered in the main repo docs; remove to keep this README focused.
-## Testing - -Run the comprehensive test suite: - -```bash -go test -v ./extractors -``` - -Tests cover: - -- Individual extractor functionality across all source types -- Error handling and edge cases (empty values, malformed headers) -- Chained extractor behavior and error propagation -- Custom extractor support including nil function handling -- RFC 7235 compliance for Authorization header parsing -- Metadata validation and source introspection -- Chain ordering and fallback logic (comprehensive test functions) - -## Maintenance Notes - -- **Single Source of Truth**: All extraction logic lives here -- **Direct Usage**: Middleware imports and uses extractors directly -- **Security Consistency**: Security warnings and source awareness must be kept in sync across all extractors -- **Breaking Changes**: Require coordinated updates across dependent packages -- **Performance**: Shared functions reduce overhead across middleware -- **Documentation**: Ensure examples and warnings are clear and up-to-date
🧹 Nitpick comments (4)
docs/guide/extractors.md (3)
56-61: Unify terminology: use “Query parameters” and “Route (path) parameters.”Avoid “URL parameters” which conflicts with “Query parameters.”
-- **Query Parameters**: URL parameters like `?token=abc123` -**Form Data**: POST body form fields -- **URL Parameters**: Route parameters like `/users/:id` +- **Query parameters**: e.g., `?token=abc123` +**Form Data**: POST body form fields +- **Route (path) parameters**: e.g., `/users/:id`
85-90: Optional: include imports in the first snippet for copy-pasteability.Keeps the minimal example runnable without guesswork.
-```go +```go +import ( + "github.com/gofiber/fiber/v3" + "github.com/gofiber/fiber/v3/middleware/keyauth" + "github.com/gofiber/fiber/v3/extractors" +) + // KeyAuth middleware extracts key from header app.Use(keyauth.New(keyauth.Config{ Extractor: extractors.FromHeader("Middleware-Key"), }))
292-298: Clarify empty-scheme behavior.Make explicit that empty scheme returns the raw Authorization value after OWS trimming.
-extractors.FromAuthHeader("") // No scheme, returns header (or ErrNotFound if empty) +extractors.FromAuthHeader("") // No scheme: returns trimmed Authorization value (or ErrNotFound if empty)extractors/README.md (1)
31-38: Use consistent names: “Path parameters” or “Route parameters.”Align with the guide and Fiber’s Param terminology.
-- `FromParam(param string)`: Extract from URL path parameters +- `FromParam(param string)`: Extract from path (route) parameters
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/guide/extractors.md(1 hunks)extractors/README.md(1 hunks)extractors/extractors.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- extractors/extractors.go
🧰 Additional context used
📓 Path-based instructions (1)
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Review and update the contents of the
docsfolder if necessary when modifying code
Files:
docs/guide/extractors.md
🧠 Learnings (2)
📚 Learning: 2025-07-27T17:28:53.403Z
Learnt from: sixcolors
PR: gofiber/fiber#3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware `Config` struct, the `Extractor` field uses function closures (like `FromCookie(key)`), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
Applied to files:
docs/guide/extractors.mdextractors/README.md
📚 Learning: 2025-07-19T14:06:29.884Z
Learnt from: sixcolors
PR: gofiber/fiber#3598
File: docs/middleware/csrf.md:37-42
Timestamp: 2025-07-19T14:06:29.884Z
Learning: In Fiber v3, the CookieSameSite constants use lowercase values: CookieSameSiteLaxMode = "lax", CookieSameSiteStrictMode = "strict", CookieSameSiteNoneMode = "none". Documentation examples should use lowercase string values or the typed constants, not capitalized strings like "Lax".
Applied to files:
docs/guide/extractors.md
🪛 LanguageTool
docs/guide/extractors.md
[grammar] ~33-~33: There might be a mistake here.
Context: ...uthorization header with optional scheme - FromCookie(key string): Extract from HTTP cookies - `FromParam...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...(key string): Extract from HTTP cookies - FromParam(param string): Extract from URL path parameters - Fr...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...ring): Extract from URL path parameters - FromForm(param string): Extract from form data - FromHeader(h...
(QB_NEW_EN)
[grammar] ~36-~36: There might be a mistake here.
Context: ...m(param string): Extract from form data - FromHeader(header string): Extract from custom HTTP headers - Fr...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...ring): Extract from custom HTTP headers - FromQuery(param string): Extract from URL query parameters - F...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ...ing): Extract from URL query parameters - FromCustom(key string, fn func(fiber.Ctx) (string, error))`: Define custom extraction logic with me...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...ne custom extraction logic with metadata - Chain(extractors ...Extractor): Chain multiple extractors with fallbac...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...horization, X-API-Key`, custom headers - Cookies: Session cookies, authenticati...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ...: Session cookies, authentication tokens - Query Parameters: URL parameters like ...
(QB_NEW_EN)
[grammar] ~58-~58: There might be a mistake here.
Context: ...Query Parameters**: URL parameters like ?token=abc123 - Form Data: POST body form fields - **U...
(QB_NEW_EN)
[grammar] ~59-~59: There might be a mistake here.
Context: ...` - Form Data: POST body form fields - URL Parameters: Route parameters like ...
(QB_NEW_EN)
[grammar] ~174-~174: There might be a mistake here.
Context: ... authentication tokens, widely supported - Custom Headers: Application-specific, ...
(QB_NEW_EN)
[grammar] ~175-~175: There might be a mistake here.
Context: ...fic, less likely to be logged by default - Considerations: Can be intercepted wit...
(QB_NEW_EN)
[grammar] ~180-~180: There might be a mistake here.
Context: ... Designed for secure client-side storage - Considerations: Require proper `Secure...
(QB_NEW_EN)
[grammar] ~181-~181: There might be a mistake here.
Context: ...ecure, HttpOnly, and SameSite` flags - Best for: Session management, remember...
(QB_NEW_EN)
[grammar] ~186-~186: There might be a mistake here.
Context: ...Convenient for simple APIs and debugging - Considerations: Always visible in URLs...
(QB_NEW_EN)
[grammar] ~187-~187: There might be a mistake here.
Context: ...rvers/proxies, stored in browser history - Best for: Non-sensitive parameters, pu...
(QB_NEW_EN)
[grammar] ~192-~192: There might be a mistake here.
Context: ...le for form submissions and API requests - Considerations: Avoid putting sensitiv...
(QB_NEW_EN)
[grammar] ~364-~364: There might be a mistake here.
Context: ...xtractors.ErrNotFound }) ``` :::warning **Custom extractors break source awarene...
(QB_NEW_EN)
[grammar] ~367-~367: There might be a mistake here.
Context: ...nings** for potentially insecure sources - No source-based logging or monitoring ...
(QB_NEW_EN)
[grammar] ~368-~368: There might be a mistake here.
Context: ...sed logging** or monitoring capabilities - Developer responsibility for ensuring ...
(QB_NEW_EN)
extractors/README.md
[grammar] ~13-~13: There might be a mistake here.
Context: ...: Core extraction function with metadata - Source: Enumeration of extraction sources (Hea...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...der, Query, Form, Param, Cookie, Custom) - ErrNotFound: Standardized error for missing values ...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...uthorization header with optional scheme - FromCookie(key string): Extract from HTTP cookies - `FromParam...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...(key string): Extract from HTTP cookies - FromParam(param string): Extract from URL path parameters - Fr...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...ring): Extract from URL path parameters - FromForm(param string): Extract from form data - FromHeader(h...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...m(param string): Extract from form data - FromHeader(header string): Extract from custom HTTP headers - Fr...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...ring): Extract from custom HTTP headers - FromQuery(param string): Extract from URL query parameters - F...
(QB_NEW_EN)
[grammar] ~36-~36: There might be a mistake here.
Context: ...ing): Extract from URL query parameters - FromCustom(key string, fn func(fiber.Ctx) (string, error))`: Define custom extraction logic with me...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...ne custom extraction logic with metadata - Chain(extractors ...Extractor): Chain multiple extractors with fallbac...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...ul error handling) - Preserves metadata from first extractor for introspection - Sto...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ...or functionality across all source types - Error handling and edge cases (empty val...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ... cases (empty values, malformed headers) - Chained extractor behavior and error pro...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...extractor behavior and error propagation - Custom extractor support including nil f...
(QB_NEW_EN)
[grammar] ~98-~98: There might be a mistake here.
Context: ... support including nil function handling - RFC 7235 compliance for Authorization he...
(QB_NEW_EN)
[grammar] ~99-~99: There might be a mistake here.
Context: ...pliance for Authorization header parsing - Metadata validation and source introspec...
(QB_NEW_EN)
[grammar] ~100-~100: There might be a mistake here.
Context: ...data validation and source introspection - Chain ordering and fallback logic (compr...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit (1.25.x, windows-latest)
- GitHub Check: repeated
- GitHub Check: Compare
🔇 Additional comments (1)
docs/guide/extractors.md (1)
66-71: Skip-nil-extractor behavior covered by tests – Test_Extractor_Chain_NilFunctions in extractors_test.go confirms Chain skips nil Extract functions and returns the first valid result.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
extractors/README.md (4)
65-73: Fix MD032: add a blank line after the bullet list.Lists must be surrounded by blank lines; insert an empty line between the last bullet (Line 72) and “## Security Considerations”.
- - Stores defensive copy for runtime inspection via the `Chain` field - -## Security Considerations + - Stores defensive copy for runtime inspection via the `Chain` field + +## Security Considerations
79-83: Surround list with blank lines and avoid calling any source “secure.”
- Add a blank line after the bullet list (Line 82) before the following paragraph to satisfy MD032.
- Reword “expected secure source” to “expected source” to avoid implying inherent security.
- - **CSRF Protection**: The double-submit-cookie pattern requires tokens to come from cookies. Source awareness allows CSRF middleware to verify tokens originate from the expected secure source + - **CSRF Protection**: The double-submit-cookie pattern requires tokens to come from cookies. Source awareness allows CSRF middleware to verify tokens originate from the expected source (e.g., a cookie) and not from disallowed locations + However, when using `FromCustom`, middleware cannot determine the source of the extracted value, which can compromise security:
31-31: Clarify RFC behavior of FromAuthHeader.Mention SP/HTAB-only trimming and optional scheme enforcement for accuracy with RFC 9110 auth parsing.
- - `FromAuthHeader(authScheme string)`: Extract from Authorization header with optional scheme + - `FromAuthHeader(authScheme string)`: Extract from Authorization header with optional scheme; trims only SP and HTAB per RFC 9110 and enforces scheme match when provided
42-43: Tone down “security-aware” phrasing; emphasize source-awareness.This is an introspection aid enabling security policy decisions; prefer neutral wording.
-The `Source` field provides **security-aware extraction** by explicitly identifying the origin of extracted values. This enables middleware to enforce security policies based on data source: +The `Source` field provides **source-aware extraction** by explicitly identifying the origin of extracted values. This enables middleware to make policy decisions (including security) based on data source:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
extractors/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-27T17:28:53.403Z
Learnt from: sixcolors
PR: gofiber/fiber#3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware `Config` struct, the `Extractor` field uses function closures (like `FromCookie(key)`), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
Applied to files:
extractors/README.md
🪛 LanguageTool
extractors/README.md
[grammar] ~13-~13: There might be a mistake here.
Context: ...: Core extraction function with metadata - Source: Enumeration of extraction sources (Hea...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...der, Query, Form, Param, Cookie, Custom) - ErrNotFound: Standardized error for missing values ...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...uthorization header with optional scheme - FromCookie(key string): Extract from HTTP cookies - `FromParam...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...(key string): Extract from HTTP cookies - FromParam(param string): Extract from URL path parameters - Fr...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...ring): Extract from URL path parameters - FromForm(param string): Extract from form data - FromHeader(h...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...m(param string): Extract from form data - FromHeader(header string): Extract from custom HTTP headers - Fr...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...ring): Extract from custom HTTP headers - FromQuery(param string): Extract from URL query parameters - F...
(QB_NEW_EN)
[grammar] ~36-~36: There might be a mistake here.
Context: ...ing): Extract from URL query parameters - FromCustom(key string, fn func(fiber.Ctx) (string, error))`: Define custom extraction logic with me...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...ne custom extraction logic with metadata - Chain(extractors ...Extractor): Chain multiple extractors with fallbac...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...ul error handling) - Preserves metadata from first extractor for introspection - Sto...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Compare
- GitHub Check: unit (1.25.x, macos-latest)
- GitHub Check: unit (1.25.x, macos-13)
- GitHub Check: unit (1.25.x, windows-latest)
- GitHub Check: repeated
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
extractors/extractors.go (6)
109-161: Authorization parsing: handle leading/trailing OWS and whitespace-only; enforce at least one SP/HTAB after scheme.
Current code: returns raw whitespace-only when authScheme == ""; doesn’t trim trailing OWS; doesn’t allow leading OWS before scheme. Fix below.func FromAuthHeader(authScheme string) Extractor { return Extractor{ Extract: func(c fiber.Ctx) (string, error) { authHeader := c.Get(fiber.HeaderAuthorization) - if authHeader == "" { - return "", ErrNotFound - } + // Treat empty/whitespace-only as not found. + h := trimSPHTAB(authHeader) + if h == "" { + return "", ErrNotFound + } // Check if the header starts with the specified auth scheme if authScheme == "" { - if authHeader == "" { - return "", ErrNotFound - } - return authHeader, nil + // Return normalized (OWS-trimmed) header value. + return h, nil } - // Early return if header is too short for scheme + space + token - if len(authHeader) < len(authScheme)+2 { - return "", ErrNotFound - } - - // Check if header starts with auth scheme (case-insensitive) - if !utils.EqualFold(authHeader[:len(authScheme)], authScheme) { + // Require scheme match (case-insensitive) at start of normalized header. + if len(h) < len(authScheme) { return "", ErrNotFound } - - // While RFC 9110 technically specifies 1*SP, HTTP implementations are generally lenient with whitespace (SP/HTAB) - if authHeader[len(authScheme)] != ' ' && authHeader[len(authScheme)] != '\t' { + if !utils.EqualFold(h[:len(authScheme)], authScheme) { return "", ErrNotFound } - // Get the part after the scheme and required space - rest := authHeader[len(authScheme)+1:] + // After scheme, require at least one SP/HTAB; allow multiple. + rest := h[len(authScheme):] + if len(rest) == 0 { + return "", ErrNotFound + } - // Skip any additional whitespace (SP/HTAB allowed per RFC 9110) - i := 0 - for i < len(rest) && (rest[i] == ' ' || rest[i] == '\t') { - i++ - } + i := 0 + for i < len(rest) && (rest[i] == ' ' || rest[i] == '\t') { + i++ + } + // Must have at least one whitespace after scheme and some content after it. + if i == 0 || i == len(rest) { + return "", ErrNotFound + } - // Must have some content after whitespace - if i == len(rest) { + // Extract token and trim trailing OWS. + token := trimSPHTAB(rest[i:]) + if token == "" { return "", ErrNotFound } - - // Extract the token - token := rest[i:] return token, nil }, Key: fiber.HeaderAuthorization, Source: SourceAuthHeader, AuthScheme: authScheme, } }
471-511: Chain error semantics: preserve first non-ErrNotFound error instead of last.
Avoids masking earlier hard failures; still short-circuits on first success.return Extractor{ Extract: func(c fiber.Ctx) (string, error) { - var lastErr error // last error encountered (including ErrNotFound) + var firstHardErr error // first error that is not ErrNotFound for _, extractor := range extractors { if extractor.Extract == nil { continue } v, err := extractor.Extract(c) if err == nil && v != "" { return v, nil } - if err != nil { - lastErr = err - } + if err != nil && !errors.Is(err, ErrNotFound) && firstHardErr == nil { + firstHardErr = err + } } - if lastErr != nil { - return "", lastErr + if firstHardErr != nil { + return "", firstHardErr } return "", ErrNotFound },
352-363: Query params: handle whitespace-only as missing.Extract: func(c fiber.Ctx) (string, error) { value := c.Query(param) - if value == "" { + if trimSPHTAB(value) == "" { return "", ErrNotFound } return value, nil },
311-322: Headers: handle whitespace-only as missing.Extract: func(c fiber.Ctx) (string, error) { value := c.Get(header) - if value == "" { + if trimSPHTAB(value) == "" { return "", ErrNotFound } return value, nil },
189-200: Treat whitespace-only cookie values as not found (OWS-aware).
Prevents false positives and aligns with docs.Extract: func(c fiber.Ctx) (string, error) { value := c.Cookies(key) - if value == "" { + if trimSPHTAB(value) == "" { return "", ErrNotFound } return value, nil },
272-283: Form values: handle whitespace-only as missing.Extract: func(c fiber.Ctx) (string, error) { value := c.FormValue(param) - if value == "" { + if trimSPHTAB(value) == "" { return "", ErrNotFound } return value, nil },
🧹 Nitpick comments (5)
extractors/extractors.go (3)
9-15: Trim the “Key features” block from source; keep this in docs only.
Reduces noise in exported code headers.-// Key features: -// - Security-aware extraction with source tracking -// - RFC 9110 compliant Authorization header parsing -// - Robust error handling and nil-safe operations -// - Chain/fallback logic for multiple extraction sources -// - Comprehensive test coverage with 17 test functions -//
89-108: Doc/code mismatch: “without modification” vs RFC OWS handling.
If we trim OWS and treat whitespace-only as not found, update docs accordingly.-// - authScheme: The auth scheme to strip from the header value (e.g., "Bearer", "Basic"). -// If empty, the entire header value is returned without modification. +// - authScheme: The auth scheme to strip from the header value (e.g., "Bearer", "Basic"). +// If empty, the header value is returned after trimming optional SP/HTAB (OWS). +// If the result is empty or only whitespace, ErrNotFound is returned. @@ -// // Input: "CustomAuth token123" -> Output: "CustomAuth token123" +// // Input: " CustomAuth token123\t" -> Output: "CustomAuth token123"
229-244: Consider propagating unescape errors instead of ErrNotFound.
Distinguishes malformed input from absent input, improving observability and Chain error semantics.- unescapedValue, err := url.PathUnescape(value) - if err != nil { - return "", ErrNotFound - } + unescapedValue, err := url.PathUnescape(value) + if err != nil { + return "", err + }docs/guide/extractors.md (2)
58-65: Chain docs: update to “first hard error” if you adopt the code change.
Keep behavior consistent across code and docs.-- If all extractors fail, returns the last error encountered or `ErrNotFound` +- If all extractors fail, returns the first non-`ErrNotFound` error encountered, otherwise `ErrNotFound`
277-292: Standards note: call out OWS trimming and whitespace-only rejection.
Clarifies edge cases users will hit.- **OWS handling**: Supports SP/HTAB around the scheme/value per RFC 9110 +- **Empty/whitespace-only**: Treated as not found; header is normalized by trimming SP/HTAB
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/guide/extractors.md(1 hunks)extractors/README.md(1 hunks)extractors/extractors.go(1 hunks)extractors/extractors_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- extractors/extractors_test.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Format Go code using gofumpt (enforced viamake format)
Ensure code passes golangci-lint checks (enforced viamake lint)
Optimize struct field alignment using betteralign (enforced viamake betteralign)
Modernize Go code using gopls modernize (enforced viamake modernize)
Files:
extractors/extractors.go
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Review and update the contents of the
docsfolder if necessary when modifying code
Files:
docs/guide/extractors.md
🧠 Learnings (4)
📚 Learning: 2025-07-27T17:28:53.403Z
Learnt from: sixcolors
PR: gofiber/fiber#3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware `Config` struct, the `Extractor` field uses function closures (like `FromCookie(key)`), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
Applied to files:
extractors/README.mdextractors/extractors.godocs/guide/extractors.md
📚 Learning: 2025-09-01T07:16:43.444Z
Learnt from: CR
PR: gofiber/fiber#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T07:16:43.444Z
Learning: Applies to **/*.go : Optimize struct field alignment using betteralign (enforced via `make betteralign`)
Applied to files:
extractors/extractors.go
📚 Learning: 2024-12-01T10:28:36.011Z
Learnt from: ReneWerner87
PR: gofiber/fiber#0
File: :0-0
Timestamp: 2024-12-01T10:28:36.011Z
Learning: Feature request #3224 has been created to add support for square bracket notation and comma-separated values in multipart form data in Fiber, while maintaining binary data transfer capabilities. This would bring parity with the existing form-urlencoded functionality.
Applied to files:
extractors/extractors.go
📚 Learning: 2025-07-19T14:06:29.884Z
Learnt from: sixcolors
PR: gofiber/fiber#3598
File: docs/middleware/csrf.md:37-42
Timestamp: 2025-07-19T14:06:29.884Z
Learning: In Fiber v3, the CookieSameSite constants use lowercase values: CookieSameSiteLaxMode = "lax", CookieSameSiteStrictMode = "strict", CookieSameSiteNoneMode = "none". Documentation examples should use lowercase string values or the typed constants, not capitalized strings like "Lax".
Applied to files:
docs/guide/extractors.md
🧬 Code graph analysis (1)
extractors/extractors.go (1)
constants.go (1)
HeaderAuthorization(163-163)
🪛 LanguageTool
extractors/README.md
[grammar] ~13-~13: There might be a mistake here.
Context: ...: Core extraction function with metadata - Source: Enumeration of extraction sources (Hea...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...der, Query, Form, Param, Cookie, Custom) - ErrNotFound: Standardized error for missing values ...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...uthorization header with optional scheme - FromCookie(key string): Extract from HTTP cookies - `FromParam...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...(key string): Extract from HTTP cookies - FromParam(param string): Extract from URL path parameters - Fr...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...ring): Extract from URL path parameters - FromForm(param string): Extract from form data - FromHeader(h...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...m(param string): Extract from form data - FromHeader(header string): Extract from custom HTTP headers - Fr...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...ring): Extract from custom HTTP headers - FromQuery(param string): Extract from URL query parameters - F...
(QB_NEW_EN)
[grammar] ~36-~36: There might be a mistake here.
Context: ...ing): Extract from URL query parameters - FromCustom(key string, fn func(fiber.Ctx) (string, error))`: Define custom extraction logic with me...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...ne custom extraction logic with metadata - Chain(extractors ...Extractor): Chain multiple extractors with fallbac...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...ul error handling) - Preserves metadata from first extractor for introspection - Sto...
(QB_NEW_EN)
docs/guide/extractors.md
[grammar] ~15-~15: There might be a mistake here.
Context: ...plementations across middleware packages - Ensures Consistency: Maintains identic...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...security practices across all extractors - Simplifies Maintenance: Changes to ext...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ... logic only need to be made in one place - Enables Direct Usage: Middleware can i...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...e can import and use extractors directly - Improves Performance: Shared, optimize...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ...uthorization header with optional scheme - FromCookie(key string): Extract from HTTP cookies - `FromParam...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ...(key string): Extract from HTTP cookies - FromParam(param string): Extract from URL path parameters - Fr...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...ring): Extract from URL path parameters - FromForm(param string): Extract from form data - FromHeader(h...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...m(param string): Extract from form data - FromHeader(header string): Extract from custom HTTP headers - Fr...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...ring): Extract from custom HTTP headers - FromQuery(param string): Extract from URL query parameters - F...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...ing): Extract from URL query parameters - FromCustom(key string, fn func(fiber.Ctx) (string, error))`: Define custom extraction logic with me...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...ne custom extraction logic with metadata - Chain(extractors ...Extractor): Chain multiple extractors with fallbac...
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ...horization, X-API-Key`, custom headers - Cookies: Session cookies, authenticati...
(QB_NEW_EN)
[grammar] ~51-~51: There might be a mistake here.
Context: ...: Session cookies, authentication tokens - Query Parameters: URL parameters like ...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ...Query Parameters**: URL parameters like ?token=abc123 - Form Data: POST body form fields - **U...
(QB_NEW_EN)
[grammar] ~53-~53: There might be a mistake here.
Context: ...` - Form Data: POST body form fields - URL Parameters: Route parameters like ...
(QB_NEW_EN)
[grammar] ~168-~168: There might be a mistake here.
Context: ... authentication tokens, widely supported - Custom Headers: Application-specific, ...
(QB_NEW_EN)
[grammar] ~169-~169: There might be a mistake here.
Context: ...fic, less likely to be logged by default - Considerations: Can be intercepted wit...
(QB_NEW_EN)
[grammar] ~174-~174: There might be a mistake here.
Context: ... Designed for secure client-side storage - Considerations: Require proper `Secure...
(QB_NEW_EN)
[grammar] ~175-~175: There might be a mistake here.
Context: ...ecure, HttpOnly, and SameSite` flags - Best for: Session management, remember...
(QB_NEW_EN)
[grammar] ~180-~180: There might be a mistake here.
Context: ...Convenient for simple APIs and debugging - Considerations: Always visible in URLs...
(QB_NEW_EN)
[grammar] ~181-~181: There might be a mistake here.
Context: ...rvers/proxies, stored in browser history - Best for: Non-sensitive parameters, pu...
(QB_NEW_EN)
[grammar] ~186-~186: There might be a mistake here.
Context: ...le for form submissions and API requests - Considerations: Avoid putting sensitiv...
(QB_NEW_EN)
[grammar] ~358-~358: There might be a mistake here.
Context: ...xtractors.ErrNotFound }) ``` :::warning **Custom extractors break source awarene...
(QB_NEW_EN)
[grammar] ~361-~361: There might be a mistake here.
Context: ...nings** for potentially insecure sources - No source-based logging or monitoring ...
(QB_NEW_EN)
[grammar] ~362-~362: There might be a mistake here.
Context: ...sed logging** or monitoring capabilities - Developer responsibility for ensuring ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
extractors/README.md
21-21: Hard tabs
Column: 1
(MD010, no-hard-tabs)
22-22: Hard tabs
Column: 1
(MD010, no-hard-tabs)
23-23: Hard tabs
Column: 1
(MD010, no-hard-tabs)
24-24: Hard tabs
Column: 1
(MD010, no-hard-tabs)
25-25: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🪛 GitHub Check: markdownlint
extractors/README.md
[failure] 25-25: Hard tabs
extractors/README.md:25:1 MD010/no-hard-tabs Hard tabs [Column: 1] https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md010.md
[failure] 24-24: Hard tabs
extractors/README.md:24:1 MD010/no-hard-tabs Hard tabs [Column: 1] https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md010.md
[failure] 23-23: Hard tabs
extractors/README.md:23:1 MD010/no-hard-tabs Hard tabs [Column: 1] https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md010.md
[failure] 22-22: Hard tabs
extractors/README.md:22:1 MD010/no-hard-tabs Hard tabs [Column: 1] https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md010.md
[failure] 21-21: Hard tabs
extractors/README.md:21:1 MD010/no-hard-tabs Hard tabs [Column: 1] https://github.com/DavidAnson/markdownlint/blob/v0.38.0/doc/md010.md
🪛 GitHub Actions: markdownlint
extractors/README.md
[error] 21-21: Markdownlint: MD010/no-hard-tabs - Hard tabs found at line 21, column 1.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Compare
- GitHub Check: unit (1.25.x, macos-13)
- GitHub Check: unit (1.25.x, windows-latest)
- GitHub Check: unit (1.25.x, macos-latest)
- GitHub Check: repeated
| ```go | ||
| type Extractor struct { | ||
| Extract func(fiber.Ctx) (string, error) // Extraction function | ||
| Key string // Parameter/header name | ||
| Source Source // Source type for inspection | ||
| AuthScheme string // Auth scheme (FromAuthHeader) | ||
| Chain []Extractor // Chained extractors | ||
| } | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Struct snippet: align with code (field order/comments).
Prevents drift between docs and implementation.
type Extractor struct {
- Extract func(fiber.Ctx) (string, error) // Extraction function
- Key string // Parameter/header name
- Source Source // Source type for inspection
- AuthScheme string // Auth scheme (FromAuthHeader)
- Chain []Extractor // Chained extractors
+ Extract func(fiber.Ctx) (string, error) // Extraction function
+ Key string // Parameter/header name
+ AuthScheme string // Auth scheme (FromAuthHeader)
+ Chain []Extractor // Chained extractors
+ Source Source // Source type for inspection
}📝 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.
| ```go | |
| type Extractor struct { | |
| Extract func(fiber.Ctx) (string, error) // Extraction function | |
| Key string // Parameter/header name | |
| Source Source // Source type for inspection | |
| AuthScheme string // Auth scheme (FromAuthHeader) | |
| Chain []Extractor // Chained extractors | |
| } | |
| ``` |
🤖 Prompt for AI Agents
In docs/guide/extractors.md around lines 40 to 48, the Extractor struct snippet
in the docs is out of sync with the actual implementation; update the snippet so
the field order and inline comments exactly match the source code (including
exact field names, ordering, types and comment text) to prevent drift between
docs and code.
| Source Source // The type of source being extracted from | ||
| } | ||
|
|
||
| // FromAuthHeader extracts a value from the Authorization header with an optional prefix. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a tiny helper for RFC OWS (SP/HTAB) trimming and reuse it.
Prevents duplication and clarifies intent.
+// trimSPHTAB removes only SP and HTAB (OWS) per HTTP specs.
+func trimSPHTAB(s string) string {
+ s = utils.Trim(s, ' ')
+ s = utils.Trim(s, '\t')
+ return s
+}📝 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.
| // FromAuthHeader extracts a value from the Authorization header with an optional prefix. | |
| // trimSPHTAB removes only SP and HTAB (OWS) per HTTP specs. | |
| func trimSPHTAB(s string) string { | |
| s = utils.Trim(s, ' ') | |
| s = utils.Trim(s, '\t') | |
| return s | |
| } | |
| // FromAuthHeader extracts a value from the Authorization header with an optional prefix. | |
| func FromAuthHeader(authScheme string) Extractor { | |
| ... | |
| } |
🤖 Prompt for AI Agents
In extractors/extractors.go around line 82, the FromAuthHeader function trims
optional RFC OWS (optional whitespace: SP/HTAB) inline which duplicates logic
elsewhere; add a small helper function (e.g., trimOWS or trimRFCOWS) that
performs trimming of leading/trailing spaces and horizontal tabs, replace the
inline trimming calls in FromAuthHeader with this new helper, export or keep it
package-private as appropriate, and reuse the helper wherever RFC OWS trimming
is needed to clarify intent and remove duplication.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
extractors/README.md (5)
31-31: Document RFC-9110 trimming and case-insensitive scheme for FromAuthHeader.Readers should know that only SP/HTAB are trimmed and that scheme matching is case-insensitive.
Apply this diff:
- - `FromAuthHeader(authScheme string)`: Extract from Authorization header with optional scheme + - `FromAuthHeader(authScheme string)`: Extract from Authorization header with optional scheme (scheme comparison is case-insensitive; leading/trailing SP and HTAB are trimmed per RFC 9110)
45-61: Tighten Source switch example: add default and call out custom-source caveat.Helps future-proofing and aligns with security notes.
Apply this diff:
switch extractor.Source { case SourceAuthHeader: // Authorization header - commonly used for authentication tokens case SourceHeader: // Custom HTTP headers - application-specific data case SourceCookie: // HTTP cookies - client-side stored data case SourceQuery: // URL query parameters - visible in URLs and logs (security consideration) case SourceForm: // Form data - POST body data case SourceParam: // URL path parameters - route-based data case SourceCustom: - // Custom extraction logic + // Custom extraction logic (source origin may be opaque to middleware) +default: + // Unknown source (consider logging or rejecting) }
65-72: Clarify Chain semantics for empty/trimmed values and error precedence.Make the fallback rules explicit to avoid ambiguity in edge cases.
Apply this diff:
- Returns first successful extraction (non-empty value, no error) - If all extractors fail, returns the last error encountered or `ErrNotFound` - **Skips extractors with `nil` Extract functions** (graceful error handling) - Preserves metadata from first extractor for introspection - Stores defensive copy for runtime inspection via the `Chain` field + - Empty values (including those that become empty after SP/HTAB trimming) are treated as not found + - When multiple errors occur, a non-`ErrNotFound` error takes precedence over `ErrNotFound`
75-83: Reword “security-aware extraction” to avoid implying inherent security; fix grammar.Keeps tone precise and addresses prior feedback.
Apply this diff:
-The `Source` field provides **security-aware extraction** by explicitly identifying the origin of extracted values. This enables middleware to enforce security policies based on data source: +The `Source` field provides explicit source attribution, enabling middleware to enforce source-aware security policies: @@ -- **CSRF Protection**: The double-submit-cookie pattern requires tokens to be submitted in both a cookie AND a form field/header. Source awareness allows CSRF middleware to verify that tokens come from both expected sources, and not for example only from cookies +- **CSRF Protection**: The double-submit-cookie pattern requires tokens in both a cookie and a form field/header. Source attribution lets CSRF middleware verify both expected sources (and detect cookie-only submissions).
39-39: Add a minimal usage snippet after the API list.A short example improves discoverability without duplicating the Guide.
Apply this diff:
- + +#### Example + +```go +// Try Bearer token first, then fall back to a cookie +ex := extractors.Chain( + extractors.FromAuthHeader("Bearer"), + extractors.FromCookie("auth"), +) +token, err := ex.Extract(c) +```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
extractors/README.md(1 hunks)extractors/extractors.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- extractors/extractors.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-27T17:28:53.403Z
Learnt from: sixcolors
PR: gofiber/fiber#3625
File: middleware/session/config.go:57-58
Timestamp: 2025-07-27T17:28:53.403Z
Learning: In the session middleware `Config` struct, the `Extractor` field uses function closures (like `FromCookie(key)`), making it impossible to introspect extractor parameters at runtime for validation purposes without complex reflection techniques.
Applied to files:
extractors/README.md
🪛 LanguageTool
extractors/README.md
[grammar] ~13-~13: There might be a mistake here.
Context: ...: Core extraction function with metadata - Source: Enumeration of extraction sources (Hea...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...der, Query, Form, Param, Cookie, Custom) - ErrNotFound: Standardized error for missing values ...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...uthorization header with optional scheme - FromCookie(key string): Extract from HTTP cookies - `FromParam...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...(key string): Extract from HTTP cookies - FromParam(param string): Extract from URL path parameters - Fr...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...ring): Extract from URL path parameters - FromForm(param string): Extract from form data - FromHeader(h...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...m(param string): Extract from form data - FromHeader(header string): Extract from custom HTTP headers - Fr...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...ring): Extract from custom HTTP headers - FromQuery(param string): Extract from URL query parameters - F...
(QB_NEW_EN)
[grammar] ~36-~36: There might be a mistake here.
Context: ...ing): Extract from URL query parameters - FromCustom(key string, fn func(fiber.Ctx) (string, error))`: Define custom extraction logic with me...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...ne custom extraction logic with metadata - Chain(extractors ...Extractor): Chain multiple extractors with fallbac...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...ul error handling) - Preserves metadata from first extractor for introspection - Sto...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: repeated
- GitHub Check: Compare
🔇 Additional comments (2)
extractors/README.md (2)
7-7: Ensure external Extractors Guide link is valid post-deployment
Manually verify that https://docs.gofiber.io/guide/extractors resolves correctly after release to avoid dead links in the README.
63-72: Implementation matches documentation
Nil extractors are skipped inChainviaif extractor.Extract == nil(line 484).FromAuthHeaderusesutils.EqualFoldfor case-insensitive scheme matching, manually skips SP/HTAB before the token, and returnsErrNotFoundwhen only whitespace remains.
|
@gaby can you check again |
* feat: add extractors package with shared value extraction utilities for Fiber middleware * docs: add missing line breaks for improved readability in extractors documentation * docs: update extractors documentation to clarify source types and chain behavior * docs: add Extractors Package section to What's New in v3 documentation * docs: update extractors documentation to enhance clarity and detail on usage and security considerations * style: update extractor titles for consistency in documentation * docs: remove unnecessary code block from extractors documentation * docs: update title for Extractors Package section in what's new documentation * docs: add custom extraction method and improve documentation for extractors * docs: enhance extractors documentation with error handling and RFC compliance details * docs: update middleware usage examples to specify KeyAuth for clarity * chore: remove unused go.mod and go.sum files from extractors * docs: add Audience section to README for clarity on target audience * fix: improve whitespace handling in extractor functions and update documentation * fix: update FromAuthHeader to return ErrNotFound for empty and whitespace-only headers * Remove unnecessary code and comments in extractors.md * fix: clarify documentation on query parameters and form data considerations * fix: improve extractor functions by removing unnecessary whitespace handling and updating documentation * test: add URL-encoded value extraction tests for FromQuery, FromForm, and FromParam * fix: remove unnecessary whitespace handling from extractor documentation * fix: update README to remove whitespace handling from error cases in tests * fix: clarify case sensitivity in extractor key name verification * fix: enhance README.md * fix: update extractor documentation for RFC compliance and improve descriptions * fix: clarify comments in Extractor struct for better understanding * fix: enhance security documentation for CSRF protection and custom extractors * fix: remove feature list from extractors.go documentation * fix: format remove hard tobs --------- Co-authored-by: RW <rene@gofiber.io>
Summary
This PR introduces a shared
github.com/gofiber/fiber/v3/extractorspackage that consolidates value extraction utilities previously duplicated across multiple middleware packages. The package provides a clean, consistent API for extracting values from various sources (headers, cookies, query parameters, form data, URL parameters) with built-in chain/fallback logic, custom extraction support, and comprehensive security considerations.Key Changes
New Package: extractors
Extractorinterface and concrete implementations:FromAuthHeader- Extracts from Authorization header with scheme supportFromCookie- Extracts from HTTP cookiesFromParam- Extracts from URL parametersFromForm- Extracts from form dataFromHeader- Extracts from HTTP headersFromQuery- Extracts from query parametersFromCustom- Custom extraction logic with source awarenessChain- Combines multiple extractors with fallback logicDocumentation Updates
Migration Strategy
This PR establishes the foundation for migrating middleware packages to use the shared extractors. Future PRs will update individual middleware packages (session, csrf, keyauth, etc.) to import and use
github.com/gofiber/fiber/v3/extractorsdirectly instead of maintaining their own extraction logic.Benefits
Testing
Security Considerations
The package includes comprehensive security features:
Next Steps
After this PR is merged, individual middleware packages will be updated to:
github.com/gofiber/fiber/v3/extractorsThis migration will be done incrementally across multiple PRs to ensure stability and allow for thorough testing of each middleware package.