Skip to content

📝 [Proposal]: extractors - add ExtractSource field for non-breaking source-aware extraction #4472

Description

@sixcolors

Feature Proposal Description

PR #4469 fixes a session-fixation vulnerability by tracking which extractor in a chain actually supplied the incoming session ID. To do this, it stores the winning Extractor struct in c.Locals() from within store.getSessionID() — which means store.go must re-walk the chain itself, duplicating logic that already lives inside Chain().

This works and is correct, but it leaves the chain-walking owned by two places. Any future change to Chain semantics (error handling, skip logic, ordering) won't automatically be inherited by store.go.

Proposed Change

Add a second, optional field to Extractor alongside the existing Extract:

type Extractor struct {
    Extract       func(fiber.Ctx) (string, error)          // existing — unchanged
    ExtractSource func(fiber.Ctx) (string, Source, error)  // new — optional
    Key           string
    AuthScheme    string
    Chain         []Extractor
    Source        Source
}

All built-in constructors (FromCookie, FromHeader, FromQuery, FromForm, FromParam, FromAuthHeader, FromCustom) populate both fields. Since both close over the same inner function, there is zero logic duplication — ExtractSource is a thin wrapper that appends the known Source constant:

func FromCookie(key string) Extractor {
    fn := func(c fiber.Ctx) (string, error) {
        value := c.Cookies(key)
        if value == "" {
            return "", ErrNotFound
        }
        return value, nil
    }
    return Extractor{
        Extract: fn,
        ExtractSource: func(c fiber.Ctx) (string, Source, error) {
            v, err := fn(c)
            return v, SourceCookie, err
        },
        Key:    key,
        Source: SourceCookie,
    }
}

Chain propagates the winning extractor's source naturally through ExtractSource, using a shared extractWithSource helper that gracefully degrades for hand-rolled extractors where ExtractSource is nil:

// extractWithSource calls ExtractSource if populated, otherwise falls back
// to Extract with the extractor's static Source metadata.
func extractWithSource(e Extractor, c fiber.Ctx) (string, Source, error) {
    if e.ExtractSource != nil {
        return e.ExtractSource(c)
    }
    v, err := e.Extract(c)
    return v, e.Source, err
}

Impact on PR #4469

store.getSessionID simplifies to:

func (s *Store) getSessionID(c fiber.Ctx) string {
    id, src, err := extractWithSource(s.Extractor, c)
    if err != nil || id == "" {
        return ""
    }
    c.Locals(sessionExtractorContextKey, src)
    return id
}

The duplicated chain-walk in store.go is eliminated entirely. sess.extractor extractors.Extractor on the Session struct can also shrink to sess.extractorSource extractors.Source (a plain int), since getExtractorInfo() only ever switches on the source type — storing the full struct was over-specified.

Non-Breaking Guarantee

  • All existing callers of Extract compile and behave identically.
  • Third-party code that constructs Extractor{Extract: myFn} literals directly gets nil for ExtractSource, which extractWithSource handles safely by falling back to e.Source.
  • No interface changes, no import changes for downstream packages.

Implementation Notes

  • Cycle detection (chainGuardKey guard) must not be shared between the Extract and ExtractSource closures inside Chain — give each its own guard key to prevent false ErrChainCycle if both are called on the same request.
  • Chain's ExtractSource closure must call extractWithSource(ex, c) rather than ex.ExtractSource(c) directly to avoid a nil-panic on hand-rolled chain members.
  • Add a // Deprecated: use ExtractSource doc comment to the Extract field at the point of introduction so linters can surface it ahead of the v4 removal.

Migration Path

Phase Action
This issue Ship ExtractSource + extractWithSource helper; all existing callers untouched
PR #4469 follow-up store.getSessionID adopts extractWithSource; sess.extractor becomes sess.extractorSource Source
v4 / next major Remove Extract, rename ExtractSourceExtract with the richer three-value signature

Relation to Existing Issues

This is a follow-up enhancement to #4469 and does not block it. #4469 should merge as-is since it correctly fixes the security issue. This issue tracks the cleaner long-term design.

Metadata

Metadata

Assignees

Type

No type

Fields

No fields configured for issues without a type.

Projects

Status
No status

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions