Ignore illegal response header (#2439)

[MitchLewis930]

PR_055

[greptile-apps]

Greptile Overview

Greptile Summary

This PR implements validation to ignore illegal response headers according to the Rack specification. The changes prevent HTTP header injection attacks by filtering out headers with illegal characters and banned keys.

Key Changes:

• Replaced simple newline injection check with comprehensive header validation
• Added regex patterns to detect illegal characters in header keys (non-printable ASCII, DQUOTE, delimiters) and values (control characters)
• Implemented filtering for banned header keys (rack.* and status)
• Applied validation to both regular response headers and early hints
• Enabled previously skipped tests to verify the implementation

Issue Found:

• The BANNED_HEADER_KEY regex in lib/puma/const.rb:248 has an unescaped dot that will match any character instead of a literal period, potentially allowing headers like rackx or rack5 to be incorrectly banned

Confidence Score: 3/5

• This PR improves security by preventing header injection, but has a regex bug that needs fixing before merge
• The implementation correctly addresses header injection vulnerabilities and has comprehensive test coverage. However, the unescaped dot in the BANNED_HEADER_KEY regex is a logic error that could cause unintended headers to be filtered. Once this regex is fixed, the PR would be safe to merge.
• lib/puma/const.rb requires attention to fix the regex pattern

Important Files Changed

Filename	Overview
lib/puma/const.rb	Added header validation regexes, but BANNED_HEADER_KEY pattern has unescaped dot that matches any character instead of literal dot
lib/puma/request.rb	Replaced injection checks with more comprehensive illegal header validation for keys and values

Sequence Diagram

sequenceDiagram
    participant App as Rack Application
    participant Server as Puma Server
    participant Request as Request Handler
    participant Client as HTTP Client
    
    App->>Server: Return [status, headers, body]
    Server->>Request: Process response headers
    
    alt Early Hints Flow
        Request->>Request: str_early_hints(headers)
        loop For each header
            Request->>Request: Check illegal_header_key?(k)
            alt Key is illegal
                Request->>Request: Skip header
            else Key is valid
                Request->>Request: Check illegal_header_value?(v)
                alt Value is illegal
                    Request->>Request: Skip header line
                else Value is valid
                    Request->>Client: Send header in 103 response
                end
            end
        end
    end
    
    alt Regular Response Flow
        Request->>Request: str_headers(env, status, headers)
        loop For each header
            Request->>Request: Check illegal_header_key?(k)
            alt Key is illegal
                Request->>Request: Skip header
            else Key is valid
                Request->>Request: Check k.downcase against special cases
                alt Matches BANNED_HEADER_KEY (rack.*, status)
                    Request->>Request: Skip header
                else content-length
                    Request->>Request: Check illegal_header_value?(vs)
                    alt Value is illegal
                        Request->>Request: Skip header
                    else Value is valid
                        Request->>Request: Store content_length
                    end
                else Other headers
                    Request->>Request: Check illegal_header_value?(v)
                    alt Value is illegal
                        Request->>Request: Skip header line
                    else Value is valid
                        Request->>Client: Write header to response
                    end
                end
            end
        end
        Request->>Client: Send complete response
    end

Loading

[greptile-apps]

_{2 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Unescaped dot in regex pattern - the dot in rack. matches any character instead of literal period. Headers like rackx or rack5 would incorrectly be banned. Escape the dot: /rack\.|status/

Prompt To Fix With AI

This is a comment left during a code review.
Path: lib/puma/const.rb
Line: 248:248

Comment:
Unescaped dot in regex pattern - the dot in `rack.` matches any character instead of literal period. Headers like `rackx` or `rack5` would incorrectly be banned. Escape the dot: `/rack\.|status/`

How can I resolve this? If you propose a fix, please make it concise.