Merge pull request #198 from twitter/handle-nil-better

Handle nil values and empty arrays

lib/secure_headers/headers/content_security_policy.rb

@@ -13,6 +13,7 @@ class ContentSecurityPolicy
     REPORT_ONLY = "Content-Security-Policy-Report-Only".freeze
     HEADER_NAMES = [HEADER_NAME, REPORT_ONLY]
     DATA_PROTOCOL = "data:".freeze
+    BLOB_PROTOCOL = "blob:".freeze
     SELF = "'self'".freeze
     NONE = "'none'".freeze
     STAR = "*".freeze
@@ -141,7 +142,9 @@ class ContentSecurityPolicy
     WILDCARD_SOURCES = [
       UNSAFE_EVAL,
       UNSAFE_INLINE,
-      STAR
+      STAR,
+      DATA_PROTOCOL,
+      BLOB_PROTOCOL
     ]
 
     class << self
@@ -243,11 +246,11 @@ def validate_directive!(key, value)
       # Does not validate the invididual values of the source expression (e.g.
       # script_src => h*t*t*p: will not raise an exception)
       def validate_source_expression!(key, value)
-        # source expressions
         unless ContentSecurityPolicy::ALL_DIRECTIVES.include?(key)
           raise ContentSecurityPolicyConfigError.new("Unknown directive #{key}")
         end
-        unless value.is_a?(Array) && value.all? { |v| v.is_a?(String) }
+
+        unless value.is_a?(Array) && value.compact.all? { |v| v.is_a?(String) }
           raise ContentSecurityPolicyConfigError.new("#{key} must be an array of strings")
         end
 
@@ -317,7 +320,7 @@ def build_value
         else
           build_directive(directive_name)
         end
-      end.join("; ")
+      end.compact.join("; ")
     end
 
     # Private: builds a string that represents one directive in a minified form.
@@ -330,6 +333,7 @@ def build_value
     # Returns a string representing a directive.
     def build_directive(directive_name)
       source_list = @config[directive_name].compact
+      return if source_list.empty?
 
       value = if source_list.include?(STAR)
         # Discard trailing entries (excluding unsafe-*) since * accomplishes the same.

spec/lib/secure_headers/headers/content_security_policy_spec.rb

@@ -47,6 +47,12 @@ module SecureHeaders
         end.to raise_error(ContentSecurityPolicyConfigError)
       end
 
+      it "allows nil values" do
+        expect do
+          CSP.validate_config!(default_src: %w('self'), script_src: ["https:", nil])
+        end.to_not raise_error
+      end
+
       it "rejects unknown directives / config" do
         expect do
           CSP.validate_config!(default_src: %w('self'), default_src_totally_mispelled: "steve")
@@ -109,9 +115,9 @@ module SecureHeaders
         expect(csp.value).not_to include("'none'")
       end
 
-      it "discards source expressions besides unsafe-* expressions when * is present" do
-        csp = ContentSecurityPolicy.new(default_src: %w(* 'unsafe-inline' 'unsafe-eval' http: https: example.org))
-        expect(csp.value).to eq("default-src * 'unsafe-inline' 'unsafe-eval'")
+      it "discards source expressions (besides unsafe-* and non-host source values) when * is present" do
+        csp = ContentSecurityPolicy.new(default_src: %w(* 'unsafe-inline' 'unsafe-eval' http: https: example.org data: blob:))
+        expect(csp.value).to eq("default-src * 'unsafe-inline' 'unsafe-eval' data: blob:")
       end
 
       it "minifies source expressions based on overlapping wildcards" do
@@ -137,6 +143,11 @@ module SecureHeaders
         expect(csp.value).to eq("default-src example.org")
       end
 
+      it "does not add a directive if the value is an empty array (or all nil)" do
+        csp = ContentSecurityPolicy.new(default_src: ["https://example.org"], script_src: [nil])
+        expect(csp.value).to eq("default-src example.org")
+      end
+
       it "deduplicates any source expressions" do
         csp = ContentSecurityPolicy.new(default_src: %w(example.org example.org example.org))
         expect(csp.value).to eq("default-src example.org")