feat: add flexible key extraction to rate limiter (#2896) by sreenivasivbieb · Pull Request #2991 · gofr-dev/gofr

sreenivasivbieb · 2026-02-14T09:14:18Z

Description

This PR implements flexible key extraction for the rate limiter middleware, addressing issue #2896. The enhancement allows developers to rate limit based on any request attribute (API keys, user IDs, email addresses, tenant IDs, etc.) rather than being limited to IP-based or global rate limiting.

Key Changes

1. New KeyExtractor Type

Added KeyExtractor function type that extracts rate limiting keys from HTTP requests
Returns (key string, err error) to support flexible key derivation with error handling

2. Built-in Extractor Functions (`rate_limiter_extractors.go`)

ExtractHeader(name) - Rate limit by HTTP header (e.g., API keys, auth tokens)
ExtractParam(name) - Rate limit by query parameter (e.g., user_id)
ExtractPathParam(name) - Rate limit by URL path parameter
ExtractBody(field) - Rate limit by JSON body field (e.g., email in login requests)
ExtractCombined(extractors...) - Combine multiple extractors with fallback chain
ExtractStatic(key) - Use a static key for global limits with custom naming
ExtractIP(trustProxies) - Extract IP address (alternative to PerIP flag)

3. Priority System

Priority 1: KeyExtractor (if configured) - Custom key extraction
Priority 2: PerIP (if true) - IP-based rate limiting (backward compatibility)
Priority 3: Global - Single shared rate limit

4. Robust Error Handling

When key extraction fails, automatically falls back to IP address
Tracks extraction failures via app_http_rate_limit_key_extraction_failed metric
Ensures requests are never rejected due to extraction errors

5. Comprehensive Test Coverage

Added 30+ tests covering all extraction scenarios
Tests for backward compatibility with existing PerIP flag
Concurrent request testing and fallback behavior validation
All extractor functions have dedicated unit tests

Use Cases Enabled

// Rate limit by API key in header
config := RateLimiterConfig{
    RequestsPerSecond: 10,
    Burst:             20,
    KeyExtractor:      ExtractHeader("X-API-Key"),
}

// Rate limit login attempts by email
config := RateLimiterConfig{
    RequestsPerSecond: 3,
    Burst:             5,
    KeyExtractor:      ExtractBody("email"),
}

// Rate limit by tenant ID with user fallback
config := RateLimiterConfig{
    RequestsPerSecond: 100,
    Burst:             200,
    KeyExtractor:      ExtractCombined(
        ExtractHeader("X-Tenant-ID"),
        ExtractParam("user_id"),
        ExtractIP(false),
    ),
}

// Custom key extraction logic
config := RateLimiterConfig{
    RequestsPerSecond: 10,
    Burst:             20,
    KeyExtractor: func(r *http.Request) (string, error) {
        // Your custom logic here
        return customKey, nil
    },
}

Breaking Changes

None. This PR maintains full backward compatibility:

Existing PerIP flag continues to work as before
When neither KeyExtractor nor PerIP is set, global rate limiting is used (unchanged behavior)
The KeyExtractor field is optional and defaults to nil

Implementation Details

Files Created

rate_limiter_extractors.go - Extractor implementations (233 lines)
rate_limiter_extractors_test.go - Extractor tests (213 lines)

Files Modified

rate_limiter.go - Added KeyExtractor field and priority logic
rate_limiter_test.go - Added 9 integration tests

Additional Information

Uses standard library packages only (net/http, encoding/json, io, strings)
No external dependencies added
Follows existing GoFr patterns for middleware configuration
Includes comprehensive documentation with usage examples
All extractors handle edge cases (empty values, malformed data, missing fields)

Checklist

I have formatted my code using goimports and golangci-lint
All new code is covered by unit tests (30+ tests added)
This PR does not decrease the overall code coverage
I have reviewed the code comments and documentation for clarity

Closes #2896

- Add KeyExtractor type for custom rate limit key derivation - Implement built-in extractors: ExtractHeader, ExtractParam, ExtractBody, ExtractCombined, ExtractStatic, ExtractIP - Priority system: KeyExtractor > PerIP > Global - Maintain backward compatibility with PerIP flag - Add comprehensive test coverage (30+ tests) - Support use cases: API key, email, user ID, tenant-based rate limiting Closes gofr-dev#2896

Copilot

Pull request overview

This PR adds flexible key extraction capabilities to the rate limiter middleware, enabling rate limiting based on any request attribute (API keys, user IDs, email addresses, etc.) rather than only IP-based or global rate limiting. The implementation maintains full backward compatibility with the existing PerIP flag.

Changes:

Introduced KeyExtractor function type with built-in extractor functions (header, param, body, combined, static, IP)
Added priority-based key selection: KeyExtractor → PerIP → Global
Implemented automatic fallback to IP-based rate limiting when key extraction fails, with metric tracking

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
`pkg/gofr/http/middleware/rate_limiter.go`	Added KeyExtractor field to config and priority-based key extraction logic with fallback handling
`pkg/gofr/http/middleware/rate_limiter_extractors.go`	Implemented 7 built-in extractor functions with comprehensive error handling
`pkg/gofr/http/middleware/rate_limiter_extractors_test.go`	Added 30+ unit tests covering all extractor scenarios and edge cases
`pkg/gofr/http/middleware/rate_limiter_test.go`	Added 9 integration tests for backward compatibility, priority system, and custom extractors

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pkg/gofr/http/middleware/rate_limiter_extractors.go

pkg/gofr/http/middleware/rate_limiter.go

sreenivasivbieb · 2026-02-17T16:04:45Z

@coolwednesday, can you pls check my pr

- Add 1MB body size limit in ExtractBody to prevent memory exhaustion attacks - Replace strings.NewReader with bytes.NewReader to avoid unnecessary allocation - Remove misleading ExtractPathParam function (was just an alias to ExtractParam) - Add security warnings to KeyExtractor documentation about bucket exhaustion attacks - Add panic protection in ExtractCombined when no extractors provided - Add test case for ExtractCombined edge case - Document security best practices for using extractors with user-controlled data

sreenivasivbieb · 2026-02-18T17:37:56Z

@coolwednesday I have made changes can you check the pr

sreenivasivbieb · 2026-02-19T09:00:24Z

@Umang01-hash Can you suggest me the changes in code that i need to consider it would be useful to me

Umang01-hash · 2026-02-19T12:43:27Z

Sure @sreenivasivbieb will review your PR shortly and tell you about any changes needed in this PR.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-23T13:14:14Z

pkg/gofr/http/middleware/rate_limiter_extractors.go

+func ExtractParam(name string) KeyExtractor {
+	return func(r *http.Request) (string, error) {
+		value := r.URL.Query().Get(name)
+		if value == "" {
+			return "", ErrKeyNotFound
+		}
+		return value, nil
+	}
+}


The PR description mentions ExtractPathParam as one of the built-in extractor functions, but this function is not implemented in the code. Either implement ExtractPathParam to extract values from URL path parameters (using gorilla/mux path variables), or remove the reference from the PR description.

Copilot · 2026-02-23T13:14:15Z

pkg/gofr/http/middleware/rate_limiter_extractors.go

+
+		// Close the original body immediately as we'll replace it
+		originalBody := r.Body
+		if closeErr := originalBody.Close(); closeErr != nil && err == nil {
+			err = closeErr
+		}
+
+		if err != nil {


When body reading fails (line 103), the original body is closed (line 106-109) but never restored before returning the error (line 111-112). This means subsequent handlers will receive a request with a closed body, which could cause issues. Consider restoring the body even in error cases, perhaps with an empty reader, to maintain predictable behavior for downstream handlers.

Suggested change

// Close the original body immediately as we'll replace it

originalBody := r.Body

if closeErr := originalBody.Close(); closeErr != nil && err == nil {

err = closeErr

}

if err != nil {

// Close the original body immediately as we'll replace it

originalBody := r.Body

if closeErr := originalBody.Close(); closeErr != nil && err == nil {

err = closeErr

}

if err != nil {

// Restore body with an empty reader so downstream handlers

// see a valid (but empty) body instead of a closed one.

r.Body = io.NopCloser(bytes.NewReader(nil))

Copilot · 2026-02-23T13:14:15Z

pkg/gofr/http/middleware/rate_limiter_extractors.go

+	return func(r *http.Request) (string, error) {
+		// Only work with JSON content
+		contentType := r.Header.Get("Content-Type")
+		if !strings.Contains(contentType, "application/json") {


Using strings.Contains for Content-Type matching is inconsistent with the codebase convention and could lead to false positives. The codebase uses strings.Split(contentType, ";")[0] followed by exact matching (see pkg/gofr/http/request.go:60). Consider following the same pattern here to properly handle Content-Type headers with charset parameters and avoid unintended matches.

Suggested change

if !strings.Contains(contentType, "application/json") {

mediaType := strings.Split(contentType, ";")[0]

if strings.TrimSpace(mediaType) != "application/json" {

pkg/gofr/http/middleware/rate_limiter_extractors_test.go

Copilot · 2026-02-23T13:14:16Z

pkg/gofr/http/middleware/rate_limiter.go

+			if config.KeyExtractor != nil {
+				key, keyErr = config.KeyExtractor(r)
+				if keyErr != nil {
+					// If key extraction fails, use a fallback
+					// Log the error but don't fail the request
+					if m != nil {
+						m.IncrementCounter(r.Context(), "app_http_rate_limit_key_extraction_failed", 
+							"path", r.URL.Path, "error", keyErr.Error())
+					}
+					// Use IP as fallback
+					key = getIP(r, config.TrustedProxies)
+					if key == "" {
+						key = "unknown"
+					}
+				}


When KeyExtractor succeeds (err is nil) but returns an empty string, the empty string is used as the rate limit key. This could cause all requests to share the same bucket unintentionally. Consider validating that the extracted key is non-empty when err is nil, and falling back to IP if the key is empty.

Copilot · 2026-02-23T13:14:16Z

pkg/gofr/http/middleware/rate_limiter_extractors.go

+
+	// ErrNoExtractors is returned when ExtractCombined is called with no extractors.
+	ErrNoExtractors = errors.New("no extractors provided")


The error variable ErrNoExtractors is defined but never used in the codebase. The ExtractCombined function panics instead of returning this error when no extractors are provided. Consider either removing this unused error or changing ExtractCombined to return this error instead of panicking for more graceful error handling.

Suggested change

// ErrNoExtractors is returned when ExtractCombined is called with no extractors.

ErrNoExtractors = errors.New("no extractors provided")

Umang01-hash · 2026-02-25T06:30:24Z

@sreenivasivbieb Since you requested a review from CoPilot lets address its given comments if needed else lets close them so that i can review the PR. Let me know once you have fixed code quality issues, resolved copilot's comment and updated the branch, i will review the PR.

Copilot AI review requested due to automatic review settings February 14, 2026 09:14

Copilot started reviewing on behalf of sreenivasivbieb February 14, 2026 09:14 View session

Copilot AI reviewed Feb 14, 2026

View reviewed changes

Merge branch 'development' into feat/flexible-rate-limiter-2896

9e07e40

sreenivasivbieb requested a review from Copilot February 23, 2026 13:08

Copilot started reviewing on behalf of sreenivasivbieb February 23, 2026 13:09 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

	if !strings.Contains(contentType, "application/json") {
	mediaType := strings.Split(contentType, ";")[0]
	if strings.TrimSpace(mediaType) != "application/json" {


		// ErrNoExtractors is returned when ExtractCombined is called with no extractors.
		ErrNoExtractors = errors.New("no extractors provided")

Conversation

sreenivasivbieb commented Feb 14, 2026

Description

Key Changes

1. New KeyExtractor Type

2. Built-in Extractor Functions (rate_limiter_extractors.go)

3. Priority System

4. Robust Error Handling

5. Comprehensive Test Coverage

Use Cases Enabled

Breaking Changes

Implementation Details

Files Created

Files Modified

Additional Information

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sreenivasivbieb commented Feb 17, 2026

Uh oh!

sreenivasivbieb commented Feb 18, 2026

Uh oh!

sreenivasivbieb commented Feb 19, 2026

Uh oh!

Umang01-hash commented Feb 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Umang01-hash commented Feb 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

2. Built-in Extractor Functions (`rate_limiter_extractors.go`)