TEST_CODEOWNERS/rails
3
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Make ActionDispatch::SSL compatible with Rack 3.0

Browse Source 
Rack 3 now allows response header values to be an Array when handling
multiple values. Newline encoded headers are no longer supported.

This commit updates `ActionDispatch::SSL#flag_cookies_as_secure!` to
be Rack-3 compliant by setting the `set-cookie` header to an Array
rather than a newline-separated String if the current Rack version is
3+.

Additionally, this commit adds `Rack::Lint` to the Rack app in the
middleware test suite so that we can ensure all of the tests are
compliant with the Rack SPEC.
This commit is contained in:
Adrianna Chang 2023-07-27 14:35:15 -04:00
parent 43be5c4dc4
commit 9d840a1719
No known key found for this signature in database
GPG Key ID: 6816AFC60CB96DE5
3 changed files with 74 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
actionpack/lib/action_dispatch/constants.rb
View File

@ -14,6 +14,7 @@ module Constants
FEATURE_POLICY = "Feature-Policy"
X_REQUEST_ID = "X-Request-Id"
SERVER_TIMING = "Server-Timing"
STRICT_TRANSPORT_SECURITY = "Strict-Transport-Security"
else
VARY = "vary"
CONTENT_ENCODING = "content-encoding"
@ -23,6 +24,7 @@ module Constants
FEATURE_POLICY = "feature-policy"
X_REQUEST_ID = "x-request-id"
SERVER_TIMING = "server-timing"
STRICT_TRANSPORT_SECURITY = "strict-transport-security"
end
end
end

24
actionpack/lib/action_dispatch/middleware/ssl.rb
View File

@ -88,7 +88,7 @@ def call(env)
private
def set_hsts_header!(headers)
headers["Strict-Transport-Security"] ||= @hsts_header
headers[Constants::STRICT_TRANSPORT_SECURITY] ||= @hsts_header
end
def normalize_hsts_options(options)
@ -114,25 +114,33 @@ def build_hsts_header(hsts)
end
def flag_cookies_as_secure!(headers)
if cookies = headers["Set-Cookie"]
if cookies.is_a?(String)
cookies = cookies.split("\n")
end
cookies = headers[Rack::SET_COOKIE]
return unless cookies
headers["Set-Cookie"] = cookies.map { |cookie|
if Gem::Version.new(Rack::RELEASE) < Gem::Version.new("3")
cookies = cookies.split("\n")
headers[Rack::SET_COOKIE] = cookies.map { |cookie|
if !/;\s*secure\s*(;|$)/i.match?(cookie)
"#{cookie}; secure"
else
cookie
end
}.join("\n")
else
headers[Rack::SET_COOKIE] = Array(cookies).map do |cookie|
if !/;\s*secure\s*(;|$)/i.match?(cookie)
"#{cookie}; secure"
else
cookie
end
end
end
end
def redirect_to_https(request)
[ @redirect.fetch(:status, redirection_status(request)),
{ "Content-Type" => "text/html; charset=utf-8",
"Location" => https_location_for(request) },
{ Rack::CONTENT_TYPE => "text/html; charset=utf-8",
Constants::LOCATION => https_location_for(request) },
(@redirect[:body] || []) ]
end

96
actionpack/test/dispatch/ssl_test.rb
View File

@ -3,13 +3,16 @@
require "abstract_unit"
class SSLTest < ActionDispatch::IntegrationTest
HEADERS = { "Content-Type" => "text/html" }
attr_accessor :app
def build_app(headers: {}, ssl_options: {})
headers = HEADERS.merge(headers)
ActionDispatch::SSL.new lambda { |env| [200, headers, []] }, **ssl_options.reverse_merge(hsts: { subdomains: true })
app = lambda { |env| [200, headers, []] }
Rack::Lint.new(
ActionDispatch::SSL.new(
Rack::Lint.new(app),
**ssl_options.reverse_merge(hsts: { subdomains: true }),
)
)
end
end
@ -128,9 +131,9 @@ def assert_hsts(expected, url: "https://example.org", hsts: { subdomains: true }
self.app = build_app ssl_options: { hsts: hsts }, headers: headers
get url
if expected.nil?
assert_nil response.headers["Strict-Transport-Security"]
assert_nil response.headers["strict-transport-security"]
else
assert_equal expected, response.headers["Strict-Transport-Security"]
assert_equal expected, response.headers["strict-transport-security"]
end
end
@ -143,7 +146,8 @@ def assert_hsts(expected, url: "https://example.org", hsts: { subdomains: true }
end
test "defers to app-provided header" do
assert_hsts "app-provided", headers: { "Strict-Transport-Security" => "app-provided" }
headers = { ActionDispatch::Constants::STRICT_TRANSPORT_SECURITY => "app-provided" }
assert_hsts "app-provided", headers: headers
end
test "hsts: true enables default settings" do
@ -180,82 +184,94 @@ def assert_hsts(expected, url: "https://example.org", hsts: { subdomains: true }
end
class SecureCookiesTest < SSLTest
DEFAULT = %(id=1; path=/\ntoken=abc; path=/; secure; HttpOnly)
def get(**options)
self.app = build_app(**options)
super "https://example.org"
end
def assert_cookies(*expected)
assert_equal expected, response.headers["Set-Cookie"].split("\n")
DEFAULT = if Gem::Version.new(Rack::RELEASE) < Gem::Version.new("3")
%(id=1; path=/\ntoken=abc; path=/; secure; HttpOnly)
else
["id=1; path=/", "token=abc; path=/; secure; HttpOnly"]
end
def test_flag_cookies_as_secure
get headers: { "Set-Cookie" => DEFAULT }
get headers: { Rack::SET_COOKIE => DEFAULT }
assert_cookies "id=1; path=/; secure", "token=abc; path=/; secure; HttpOnly"
end
def test_flag_cookies_as_secure_with_single_cookie_in_array
get headers: { "Set-Cookie" => ["id=1"] }
assert_cookies "id=1; secure"
end
def test_flag_cookies_as_secure_with_multiple_cookies_in_array
get headers: { "Set-Cookie" => ["id=1", "problem=def"] }
assert_cookies "id=1; secure", "problem=def; secure"
end
def test_flag_cookies_as_secure_at_end_of_line
get headers: { "Set-Cookie" => "problem=def; path=/; HttpOnly; secure" }
get headers: { Rack::SET_COOKIE => "problem=def; path=/; HttpOnly; secure" }
assert_cookies "problem=def; path=/; HttpOnly; secure"
end
def test_flag_cookies_as_secure_with_more_spaces_before
get headers: { "Set-Cookie" => "problem=def; path=/; HttpOnly; secure" }
get headers: { Rack::SET_COOKIE => "problem=def; path=/; HttpOnly; secure" }
assert_cookies "problem=def; path=/; HttpOnly; secure"
end
def test_flag_cookies_as_secure_with_more_spaces_after
get headers: { "Set-Cookie" => "problem=def; path=/; secure; HttpOnly" }
get headers: { Rack::SET_COOKIE => "problem=def; path=/; secure; HttpOnly" }
assert_cookies "problem=def; path=/; secure; HttpOnly"
end
def test_flag_cookies_as_secure_with_has_not_spaces_before
get headers: { "Set-Cookie" => "problem=def; path=/;secure; HttpOnly" }
get headers: { Rack::SET_COOKIE => "problem=def; path=/;secure; HttpOnly" }
assert_cookies "problem=def; path=/;secure; HttpOnly"
end
def test_flag_cookies_as_secure_with_has_not_spaces_after
get headers: { "Set-Cookie" => "problem=def; path=/; secure;HttpOnly" }
get headers: { Rack::SET_COOKIE => "problem=def; path=/; secure;HttpOnly" }
assert_cookies "problem=def; path=/; secure;HttpOnly"
end
def test_flag_cookies_as_secure_with_ignore_case
get headers: { "Set-Cookie" => "problem=def; path=/; Secure; HttpOnly" }
get headers: { Rack::SET_COOKIE => "problem=def; path=/; Secure; HttpOnly" }
assert_cookies "problem=def; path=/; Secure; HttpOnly"
end
def test_cookies_as_not_secure_with_secure_cookies_disabled
get headers: { "Set-Cookie" => DEFAULT }, ssl_options: { secure_cookies: false }
assert_cookies(*DEFAULT.split("\n"))
get headers: { Rack::SET_COOKIE => DEFAULT }, ssl_options: { secure_cookies: false }
assert_cookies("id=1; path=/", "token=abc; path=/; secure; HttpOnly")
end
def test_cookies_as_not_secure_with_exclude
excluding = { exclude: -> request { /example/.match?(request.domain) } }
get headers: { "Set-Cookie" => DEFAULT }, ssl_options: { redirect: excluding }
get headers: { Rack::SET_COOKIE => DEFAULT }, ssl_options: { redirect: excluding }
assert_cookies(*DEFAULT.split("\n"))
assert_cookies("id=1; path=/", "token=abc; path=/; secure; HttpOnly")
assert_response :ok
end
def test_no_cookies
get
assert_nil response.headers["Set-Cookie"]
assert_nil response.headers[Rack::SET_COOKIE]
end
def test_keeps_original_headers_behavior
get headers: { "Connection" => "close" }
assert_equal "close", response.headers["Connection"]
get headers: { "connection" => "close" }
assert_equal "close", response.headers["connection"]
end
# Array-based headers are only supported in Rack 3+
if Gem::Version.new(Rack::RELEASE) >= Gem::Version.new("3")
def test_flag_cookies_as_secure_with_single_cookie_in_array
get headers: { Rack::SET_COOKIE => ["id=1"] }
assert_cookies "id=1; secure"
end
def test_flag_cookies_as_secure_with_multiple_cookies_in_array
get headers: { Rack::SET_COOKIE => ["id=1", "problem=def"] }
assert_cookies "id=1; secure", "problem=def; secure"
end
end
private
def get(**options)
self.app = build_app(**options)
super "https://example.org"
end
def assert_cookies(*expected)
cookies = response.headers[Rack::SET_COOKIE]
if Gem::Version.new(Rack::RELEASE) < Gem::Version.new("3")
cookies = cookies.split("\n")
end
assert_equal expected, cookies
end
end