Use constants defined in ActionDispatch::Constants instead of conditional assignments

This commit is contained in:
Guillermo Iguaran 2023-07-27 15:21:18 -07:00
parent 1fd79abb54
commit 5b45803984
6 changed files with 17 additions and 35 deletions

View File

@ -21,19 +21,13 @@ module ActionDispatch # :nodoc:
# policy.payment :self, "https://secure.example.com" # policy.payment :self, "https://secure.example.com"
# end # end
# #
class PermissionsPolicy
class Middleware
# The Feature-Policy header has been renamed to Permissions-Policy. # The Feature-Policy header has been renamed to Permissions-Policy.
# The Permissions-Policy requires a different implementation and isn't # The Permissions-Policy requires a different implementation and isn't
# yet supported by all browsers. To avoid having to rename this # yet supported by all browsers. To avoid having to rename this
# middleware in the future we use the new name for the middleware but # middleware in the future we use the new name for the middleware but
# keep the old header name and implementation for now. # keep the old header name and implementation for now.
if Gem::Version.new(Rack::RELEASE) >= Gem::Version.new("3") class PermissionsPolicy
POLICY = "feature-policy" class Middleware
else
POLICY = "Feature-Policy"
end
def initialize(app) def initialize(app)
@app = app @app = app
end end
@ -47,11 +41,11 @@ module ActionDispatch # :nodoc:
request = ActionDispatch::Request.new(env) request = ActionDispatch::Request.new(env)
if policy = request.permissions_policy if policy = request.permissions_policy
headers[POLICY] = policy.build(request.controller_instance) headers[ActionDispatch::Constants::FEATURE_POLICY] = policy.build(request.controller_instance)
end end
if policy_empty?(policy) if policy_empty?(policy)
headers.delete(POLICY) headers.delete(ActionDispatch::Constants::FEATURE_POLICY)
end end
response response
@ -65,7 +59,7 @@ module ActionDispatch # :nodoc:
end end
def policy_present?(headers) def policy_present?(headers)
headers[POLICY] headers[ActionDispatch::Constants::FEATURE_POLICY]
end end
def policy_empty?(policy) def policy_empty?(policy)

View File

@ -38,14 +38,10 @@ module ActionDispatch # :nodoc:
# For `Rack::Headers` (Rack 3+): # For `Rack::Headers` (Rack 3+):
require "rack/headers" require "rack/headers"
Headers = ::Rack::Headers Headers = ::Rack::Headers
LOCATION = "location"
rescue LoadError rescue LoadError
# For `Rack::Utils::HeaderHash`: # For `Rack::Utils::HeaderHash`:
require "rack/utils" require "rack/utils"
Headers = ::Rack::Utils::HeaderHash Headers = ::Rack::Utils::HeaderHash
LOCATION = "Location"
end end
# To be deprecated: # To be deprecated:

View File

@ -38,7 +38,7 @@ module ActionDispatch
[302, { [302, {
Rack::CONTENT_TYPE => "text/html; charset=#{Response.default_charset}", Rack::CONTENT_TYPE => "text/html; charset=#{Response.default_charset}",
Rack::CONTENT_LENGTH => body.bytesize.to_s, Rack::CONTENT_LENGTH => body.bytesize.to_s,
ActionDispatch::Response::LOCATION => location, ActionDispatch::Constants::LOCATION => location,
}, [body]] }, [body]]
end end
end end

View File

@ -50,14 +50,6 @@ module ActionDispatch
"identity" => nil "identity" => nil
} }
if Gem::Version.new(Rack::RELEASE) < Gem::Version.new("3")
VARY = "Vary"
CONTENT_ENCODING = "Content-Encoding"
else
VARY = "vary"
CONTENT_ENCODING = "content-encoding"
end
def initialize(root, index: "index", headers: {}, precompressed: %i[ br gzip ], compressible_content_types: /\A(?:text\/|application\/javascript)/) def initialize(root, index: "index", headers: {}, precompressed: %i[ br gzip ], compressible_content_types: /\A(?:text\/|application\/javascript)/)
@root = root.chomp("/").b @root = root.chomp("/").b
@index = index @index = index
@ -136,10 +128,10 @@ module ActionDispatch
if content_encoding == "identity" if content_encoding == "identity"
return precompressed_filepath, headers return precompressed_filepath, headers
else else
headers[VARY] = "accept-encoding" headers[ActionDispatch::Constants::VARY] = "accept-encoding"
if accept_encoding.any? { |enc, _| /\b#{content_encoding}\b/i.match?(enc) } if accept_encoding.any? { |enc, _| /\b#{content_encoding}\b/i.match?(enc) }
headers[CONTENT_ENCODING] = content_encoding headers[ActionDispatch::Constants::CONTENT_ENCODING] = content_encoding
return precompressed_filepath, headers return precompressed_filepath, headers
end end
end end

View File

@ -76,7 +76,7 @@ class PermissionsPolicyMiddlewareTest < ActionDispatch::IntegrationTest
get "/index" get "/index"
assert_equal "gyroscope 'self'", response.headers[ActionDispatch::PermissionsPolicy::Middleware::POLICY] assert_equal "gyroscope 'self'", response.headers[ActionDispatch::Constants::FEATURE_POLICY]
end end
test "non-html requests will not set a policy" do test "non-html requests will not set a policy" do
@ -84,15 +84,15 @@ class PermissionsPolicyMiddlewareTest < ActionDispatch::IntegrationTest
get "/index" get "/index"
assert_nil response.headers[ActionDispatch::PermissionsPolicy::Middleware::POLICY] assert_nil response.headers[ActionDispatch::Constants::FEATURE_POLICY]
end end
test "existing policies will not be overwritten" do test "existing policies will not be overwritten" do
@app = build_app(->(env) { [200, { ActionDispatch::PermissionsPolicy::Middleware::POLICY => "gyroscope 'none'" }, []] }) @app = build_app(->(env) { [200, { ActionDispatch::Constants::FEATURE_POLICY => "gyroscope 'none'" }, []] })
get "/index" get "/index"
assert_equal "gyroscope 'none'", response.headers[ActionDispatch::PermissionsPolicy::Middleware::POLICY] assert_equal "gyroscope 'none'", response.headers[ActionDispatch::Constants::FEATURE_POLICY]
end end
private private
@ -289,6 +289,6 @@ class PermissionsPolicyWithHelpersIntegrationTest < ActionDispatch::IntegrationT
def assert_policy(expected) def assert_policy(expected)
assert_response :success assert_response :success
assert_equal expected, response.headers["Feature-Policy"] assert_equal expected, response.headers[ActionDispatch::Constants::FEATURE_POLICY]
end end
end end

View File

@ -5,7 +5,7 @@ require "rack/test"
module ApplicationTests module ApplicationTests
class PermissionsPolicyTest < ActiveSupport::TestCase class PermissionsPolicyTest < ActiveSupport::TestCase
POLICY = ActionDispatch::PermissionsPolicy::Middleware::POLICY POLICY = ActionDispatch::Constants::FEATURE_POLICY
include ActiveSupport::Testing::Isolation include ActiveSupport::Testing::Isolation
include Rack::Test::Methods include Rack::Test::Methods