Merged
Conversation
Rack 3 removed the requirement for rack.input to implement #rewind. Rack::Lint::InputWrapper in Rack 3 only exposes gets, read, each, and close, so calling rewind unconditionally raised NoMethodError on every PUT/POST request, causing WEBrick to return a 500 error.
Rack 2's Rack::Handler::WEBrick uses a monkey-patch on WEBrick::HTTPResponse#setup_header that sets @Chunked = true only during header setup (so Transfer-Encoding: chunked is emitted), then immediately restores @Chunked = false. This means WEBrick's send_body_string sends the response body as-is, relying on ChunkedBody to have pre-encoded it. Rackup 2.x's WEBrick handler takes the opposite approach: it buffers the entire body into a String, and WEBrick's []= setter permanently sets @Chunked = true when Transfer-Encoding: chunked is assigned, so WEBrick chunks the plain String itself. Pre-encoding with ChunkedBody under Rack 3 therefore produced double-chunked wire data that Net::HTTP decoded to raw chunk markers instead of the actual response body. Introduce a rack_v3? helper and guard the ChunkedBody wrapping behind it: under Rack 3 return the plain body (callable or enumerable) and let the server apply a single layer of chunked encoding; under Rack 2 keep the existing ChunkedBody behaviour.
Two related problems prevented cookies from being sent correctly under
Rack 3 with WEBrick 1.9:
1. Header key casing: Rackup's WEBrick handler looks up the cookie header
with headers.delete('set-cookie') (lowercase). The previous code
produced Title-Case keys via HeaderHash#flattened, so the lookup never
matched and cookies were left in the general headers loop where they
were passed to res[key] = value instead of res.cookies.
2. Embedded newlines in the value: multiple cookies were joined with "\n"
into a single string. WEBrick 1.9 validates every header and cookie
value through check_header, raising WEBrick::HTTPResponse::InvalidHeader
for any value that contains \r or \n, which caused a 500 response.
Introduce build_rack_response_headers which:
- Lowercases all header names (required by Rack 3; harmless for Rack 2
because all Rack 2 handlers match header names case-insensitively).
- Under Rack 3 emits set-cookie as an Array so Rackup can call
res.cookies.concat(Array(value)), emitting one Set-Cookie line per cookie
with no embedded newlines.
- Under Rack 2 emits set-cookie as a newline-joined String, matching the
expectation of Rack::Handler::WEBrick which calls vs.split("\n") before
adding cookies (passing an Array would raise NoMethodError on Array#split).
Because all header keys are now lowercase, update LOWERCASE_CONTENT_TYPE
in RackResponse and adjust the two RackResponse unit specs accordingly.
rack-test 2.x does not populate rack.input in the Rack environment for requests that have no body (e.g. GET). rack.input is only added when there is a request body to expose. The test was asserting that the adapter correctly passes the full Rack env through to the resource; rack.errors is always present regardless of HTTP method or Rack version, so use that as the representative Rack-specific key instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Rack 3 split server functionality out into a separate
rackupgem and introduced several breaking changes to the adapter contract. The existing Rack adapter was written against Rack 2 and failed in four distinct ways under Rack 3 with Rackup + WEBrick 1.9.Changes
CI: The test matrix now runs against both Rack 2 and Rack 3 to prevent regressions across versions.
Rackup::Serverfor Rack 3: Therunmethod now requires and usesRackup::Serverwhen running under Rack 3, falling back toRack::Serveron Rack 2.ChunkedBody double-encoding: Rack 2's
Rack::Handler::WEBrickmonkey-patchesWEBrick::HTTPResponse#setup_headerto temporarily set@chunked = trueduring header setup only, then restores it tofalse— the body is sent as-is, soChunkedBodypre-encoding is required. Rackup 2.x's WEBrick handler takes the opposite approach: it buffers the entire body into aStringand WEBrick's[]=setter permanently sets@chunked = truewhenTransfer-Encoding: chunkedis assigned, so WEBrick applies chunked encoding to the buffered string itself. UsingChunkedBodyunder Rack 3 therefore produced double-encoded wire data. The adapter now skipsChunkedBodyunder Rack 3 and returns the plain body.Set-Cookieheader: Multiple cookies were joined with\ninto a single header value. WEBrick 1.9 rejects any header or cookie value containing\ror\n, raisingWEBrick::HTTPResponse::InvalidHeader. Additionally, Rackup's handler only matches the lowercaseset-cookiekey when routing cookies tores.cookies, so Title-Case keys were never handled. The adapter now lowercases all response header names (required by Rack 3; harmless for Rack 2, which matches case-insensitively) and emitsset-cookieas an Array under Rack 3. Under Rack 2 it remains a newline-joined String, asRack::Handler::WEBrickcallsvs.split("\n")and would raiseNoMethodErroron an Array.RequestBody#to_sunconditionalrewind: Rack 3 removed the requirement forrack.inputto implementrewind.Rack::Lint::InputWrapperin Rack 3 exposes onlygets,read,each, andclose, so the unconditionalrewindcall raisedNoMethodErroron every PUT/POST request. The call is now guarded withrespond_to?(:rewind).Consequences
The rack handler is now compatible with Rack version 2 and version 3.
Resolves #281.
Resources