From 52a1f1c226c2238e16d1a4d32faa8d1e6a36a26f Mon Sep 17 00:00:00 2001 From: Andrew White Date: Mon, 19 Feb 2018 12:00:29 +0000 Subject: Revert "Merge pull request #32045 from eagletmt/skip-csp-header" This reverts commit 86f7c269073a3a9e6ddec9b957deaa2716f2627d, reversing changes made to 5ece2e4a4459065b5efd976aebd209bbf0cab89b. If a policy is set then we should generate it even if it's empty. However what is happening is that we're accidentally generating an empty policy when the initializer is commented out by default. --- railties/test/application/content_security_policy_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'railties') diff --git a/railties/test/application/content_security_policy_test.rb b/railties/test/application/content_security_policy_test.rb index 1539bf4440..97f2957c33 100644 --- a/railties/test/application/content_security_policy_test.rb +++ b/railties/test/application/content_security_policy_test.rb @@ -34,7 +34,7 @@ module ApplicationTests app("development") get "/" - assert_not last_response.headers.key?("Content-Security-Policy") + assert_equal ";", last_response.headers["Content-Security-Policy"] end test "global content security policy in an initializer" do -- cgit v1.2.3 From 57f9c36387f371cfb791aa660c733e9690443d04 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Mon, 19 Feb 2018 12:17:51 +0000 Subject: Don't accidentally create an empty CSP Setting up the request environment was accidentally creating a CSP as a consequence of accessing the option - only set the instance variable if a block is passed. --- railties/lib/rails/application/configuration.rb | 6 ++++- .../application/content_security_policy_test.rb | 30 ++++++++++++++++++++-- 2 files changed, 33 insertions(+), 3 deletions(-) (limited to 'railties') diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index 46ad3557e3..1f765f302c 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -241,7 +241,11 @@ module Rails end def content_security_policy(&block) - @content_security_policy ||= ActionDispatch::ContentSecurityPolicy.new(&block) + if block_given? + @content_security_policy = ActionDispatch::ContentSecurityPolicy.new(&block) + else + @content_security_policy + end end class Custom #:nodoc: diff --git a/railties/test/application/content_security_policy_test.rb b/railties/test/application/content_security_policy_test.rb index 97f2957c33..43f2b333f3 100644 --- a/railties/test/application/content_security_policy_test.rb +++ b/railties/test/application/content_security_policy_test.rb @@ -16,7 +16,7 @@ module ApplicationTests teardown_app end - test "default content security policy is empty" do + test "default content security policy is nil" do controller :pages, <<-RUBY class PagesController < ApplicationController def index @@ -34,7 +34,33 @@ module ApplicationTests app("development") get "/" - assert_equal ";", last_response.headers["Content-Security-Policy"] + assert_nil last_response.headers["Content-Security-Policy"] + end + + test "empty content security policy is generated" do + controller :pages, <<-RUBY + class PagesController < ApplicationController + def index + render html: "

Welcome to Rails!

" + end + end + RUBY + + app_file "config/initializers/content_security_policy.rb", <<-RUBY + Rails.application.config.content_security_policy do |p| + end + RUBY + + app_file "config/routes.rb", <<-RUBY + Rails.application.routes.draw do + root to: "pages#index" + end + RUBY + + app("development") + + get "/" + assert_policy ";" end test "global content security policy in an initializer" do -- cgit v1.2.3 From d85283cc42b1a965944047a2f602153804126f77 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Mon, 19 Feb 2018 12:20:43 +0000 Subject: Remove trailing semi-colon from CSP Although the spec[1] is defined in such a way that a trailing semi-colon is valid it also doesn't allow a semi-colon by itself to indicate an empty policy. Therefore it's easier (and valid) just to omit it rather than to detect whether the policy is empty or not. [1]: https://www.w3.org/TR/CSP2/#policy-syntax --- railties/test/application/content_security_policy_test.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'railties') diff --git a/railties/test/application/content_security_policy_test.rb b/railties/test/application/content_security_policy_test.rb index 43f2b333f3..0d28df16f8 100644 --- a/railties/test/application/content_security_policy_test.rb +++ b/railties/test/application/content_security_policy_test.rb @@ -60,7 +60,7 @@ module ApplicationTests app("development") get "/" - assert_policy ";" + assert_policy "" end test "global content security policy in an initializer" do @@ -87,7 +87,7 @@ module ApplicationTests app("development") get "/" - assert_policy "default-src 'self' https:;" + assert_policy "default-src 'self' https:" end test "global report only content security policy in an initializer" do @@ -116,7 +116,7 @@ module ApplicationTests app("development") get "/" - assert_policy "default-src 'self' https:;", report_only: true + assert_policy "default-src 'self' https:", report_only: true end test "override content security policy in a controller" do @@ -147,7 +147,7 @@ module ApplicationTests app("development") get "/" - assert_policy "default-src https://example.com;" + assert_policy "default-src https://example.com" end test "override content security policy to report only in a controller" do @@ -176,7 +176,7 @@ module ApplicationTests app("development") get "/" - assert_policy "default-src 'self' https:;", report_only: true + assert_policy "default-src 'self' https:", report_only: true end test "global content security policy added to rack app" do @@ -200,7 +200,7 @@ module ApplicationTests app("development") get "/" - assert_policy "default-src 'self' https:;" + assert_policy "default-src 'self' https:" end private -- cgit v1.2.3