Skip to content

Commit 3a5fa84

Browse files
committed
Normalise response headers to lowercase; fix Set-Cookie for Rack 3
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.
1 parent 50a7734 commit 3a5fa84

File tree

2 files changed

+48
-5
lines changed

2 files changed

+48
-5
lines changed

lib/webmachine/adapters/rack.rb

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ def call(env)
7575
response.headers[SERVER] = VERSION_STRING
7676

7777
rack_status = response.code
78-
rack_headers = response.headers.flattened(NEWLINE)
78+
rack_headers = build_rack_response_headers(response.headers)
7979
rack_body = case response.body
8080
when String # Strings are enumerable in ruby 1.8
8181
[response.body]
@@ -117,6 +117,45 @@ def call(env)
117117

118118
protected
119119

120+
# Build a Rack-compatible response headers hash from Webmachine's response
121+
# headers.
122+
#
123+
# Header names are always lowercased: this is required by Rack 3 and is
124+
# harmless for Rack 2 (all Rack 2 handlers match header names
125+
# case-insensitively).
126+
#
127+
# The +set-cookie+ value is formatted differently per Rack version:
128+
#
129+
# * Rack 3 / Rackup: the value must be an Array. Rackup's WEBrick handler
130+
# deletes the lowercase +set-cookie+ key and calls
131+
# +res.cookies.concat(Array(value))+, emitting one Set-Cookie line per
132+
# cookie. Joining with +\n+ instead would produce a header value
133+
# containing a newline, which WEBrick 1.9+ rejects as
134+
# +WEBrick::HTTPResponse::InvalidHeader+.
135+
#
136+
# * Rack 2 / Rack::Handler::WEBrick: the handler splits on +\n+ before
137+
# adding cookies (+vs.split("\n")+), so the value must be a newline-joined
138+
# String. Passing an Array causes a +NoMethodError+ because +Array+ does
139+
# not define +#split+.
140+
def build_rack_response_headers(response_headers)
141+
response_headers.each_with_object({}) do |(key, value), h|
142+
rack_key = key.downcase
143+
h[rack_key] = if rack_key == 'set-cookie'
144+
if rack_v3?
145+
# Array lets Rackup emit one Set-Cookie header per cookie.
146+
Array(value)
147+
else
148+
# Rack 2's handler splits on \n; give it a newline-joined String.
149+
Array(value).join(NEWLINE)
150+
end
151+
elsif value.is_a?(Array)
152+
value.join(NEWLINE)
153+
else
154+
value
155+
end
156+
end
157+
end
158+
120159
def routing_tokens(rack_req)
121160
nil # no-op for default, un-mapped rack adapter
122161
end
@@ -153,6 +192,10 @@ def initialize(method, uri, headers, body, routing_tokens, base_uri, env)
153192

154193
class RackResponse
155194
ONE_FIVE = '1.5'.freeze
195+
# Header names are normalised to lowercase by build_rack_response_headers,
196+
# so use the lowercase form everywhere inside RackResponse too.
197+
LOWERCASE_CONTENT_TYPE = 'content-type'.freeze
198+
156199

157200
def initialize(body, status, headers)
158201
@body = body
@@ -161,8 +204,8 @@ def initialize(body, status, headers)
161204
end
162205

163206
def finish
164-
@headers[CONTENT_TYPE] ||= TEXT_HTML if rack_release_enforcing_content_type
165-
@headers.delete(CONTENT_TYPE) if response_without_body
207+
@headers[LOWERCASE_CONTENT_TYPE] ||= TEXT_HTML if rack_release_enforcing_content_type
208+
@headers.delete(LOWERCASE_CONTENT_TYPE) if response_without_body
166209
[@status, @headers, @body]
167210
end
168211

spec/webmachine/adapters/rack_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
it 'should add Content-Type header on not acceptable response' do
2323
rack_response = described_class.new(double(:body), 406, {})
2424
_rack_status, rack_headers, _rack_body = rack_response.finish
25-
expect(rack_headers).to have_key('Content-Type')
25+
expect(rack_headers).to have_key('content-type')
2626
end
2727
end
2828

@@ -32,7 +32,7 @@
3232
it 'should not add Content-Type header on not acceptable response' do
3333
rack_response = described_class.new(double(:body), 406, {})
3434
_rack_status, rack_headers, _rack_body = rack_response.finish
35-
expect(rack_headers).not_to have_key('Content-Type')
35+
expect(rack_headers).not_to have_key('content-type')
3636
end
3737
end
3838
end

0 commit comments

Comments
 (0)