Make SecureSecurityPolicyConfig significantly faster (#506)
We have been seeing this gem a lot in profiles. Must of this slowness seems to come from overuse of instance variables in `DynamicConfig` and attempting to use them basically as a hash (which we can do much faster with a hash 😅) The first commit of these is the most important, but the other two also significantly speed things up. There is definitely more improvement available here, we seem to be overly cautious in duplicating arrays, and we also seem to convert unnecessarily between hashes and the config object, but I think this is the best place to start. <details> <summary>Benchmark:</summary> ``` require "secure_headers" require "benchmark/ips" # Copied from README MyCSPConfig = SecureHeaders::ContentSecurityPolicyConfig.new( preserve_schemes: true, # default: false. Schemes are removed from host sources to save bytes and discourage mixed content. disable_nonce_backwards_compatibility: true, # default: false. If false, `unsafe-inline` will be added automatically when using nonces. If true, it won't. See #403 for why you'd want this. # directive values: these values will directly translate into source directives default_src: %w('none'), base_uri: %w('self'), block_all_mixed_content: true, # see https://www.w3.org/TR/mixed-content/ child_src: %w('self'), # if child-src isn't supported, the value for frame-src will be set. connect_src: %w(wss:), font_src: %w('self' data:), form_action: %w('self' github.com), frame_ancestors: %w('none'), img_src: %w(mycdn.com data:), manifest_src: %w('self'), media_src: %w(utoob.com), object_src: %w('self'), sandbox: true, # true and [] will set a maximally restrictive setting plugin_types: %w(application/x-shockwave-flash), script_src: %w('self'), script_src_elem: %w('self'), script_src_attr: %w('self'), style_src: %w('unsafe-inline'), style_src_elem: %w('unsafe-inline'), style_src_attr: %w('unsafe-inline'), worker_src: %w('self'), upgrade_insecure_requests: true, # see https://www.w3.org/TR/upgrade-insecure-requests/ report_uri: %w(https://report-uri.io/example-csp) ) Benchmark.ips do |x| x.report "csp_config.to_h" do MyCSPConfig.to_h end x.report "csp_config.append" do MyCSPConfig.append({}) end x.report "new(config).value" do SecureHeaders::ContentSecurityPolicy.new(MyCSPConfig).value end end ``` </details> **Before:** ``` $ be ruby bench.rb Warming up -------------------------------------- csp_config.to_h 13.737k i/100ms csp_config.append 2.105k i/100ms new(config).value 1.429k i/100ms Calculating ------------------------------------- csp_config.to_h 139.988k (± 0.3%) i/s - 700.587k in 5.004666s csp_config.append 21.133k (± 2.4%) i/s - 107.355k in 5.082856s new(config).value 14.298k (± 0.4%) i/s - 72.879k in 5.097116s ``` **After:** ``` $ be ruby bench.rb Warming up -------------------------------------- csp_config.to_h 123.784k i/100ms csp_config.append 4.181k i/100ms new(config).value 1.617k i/100ms Calculating ------------------------------------- csp_config.to_h 1.238M (± 3.1%) i/s - 6.189M in 5.003769s csp_config.append 40.921k (± 1.0%) i/s - 204.869k in 5.006924s new(config).value 16.095k (± 0.4%) i/s - 80.850k in 5.023259s ``` `to_h` is 10x faster, `append` is 2x faster, and .value (which was not the target of these optimizations but I didn't want to see it regress) is slightly faster --------- Co-authored-by: Kylie Stradley <4666485+KyFaSt@users.noreply.github.com>
This commit is contained in:
John Hawthorn
GitHub
Родитель
ff9797fe96
Коммит
7a23cb6b35
2
Gemfile
2
Gemfile
|
|
@ -3,6 +3,8 @@ source "https://rubygems.org"
|
|||
|
||||
gemspec
|
||||
|
||||
gem "benchmark-ips"
|
||||
|
||||
group :test do
|
||||
gem "coveralls"
|
||||
gem "json"
|
||||
|
|
|
|||
|
|
@ -83,14 +83,17 @@ module SecureHeaders
|
|||
# can lead to modifying parent objects.
|
||||
def deep_copy(config)
|
||||
return unless config
|
||||
config.each_with_object({}) do |(key, value), hash|
|
||||
hash[key] =
|
||||
if value.is_a?(Array)
|
||||
result = {}
|
||||
config.each_pair do |key, value|
|
||||
result[key] =
|
||||
case value
|
||||
when Array
|
||||
value.dup
|
||||
else
|
||||
value
|
||||
end
|
||||
end
|
||||
result
|
||||
end
|
||||
|
||||
# Private: Returns the internal default configuration. This should only
|
||||
|
|
|
|||
|
|
@ -20,9 +20,9 @@ module SecureHeaders
|
|||
config
|
||||
end
|
||||
|
||||
@preserve_schemes = @config.preserve_schemes
|
||||
@script_nonce = @config.script_nonce
|
||||
@style_nonce = @config.style_nonce
|
||||
@preserve_schemes = @config[:preserve_schemes]
|
||||
@script_nonce = @config[:script_nonce]
|
||||
@style_nonce = @config[:style_nonce]
|
||||
end
|
||||
|
||||
##
|
||||
|
|
|
|||
|
|
@ -1,65 +1,23 @@
|
|||
# frozen_string_literal: true
|
||||
module SecureHeaders
|
||||
module DynamicConfig
|
||||
def self.included(base)
|
||||
base.send(:attr_reader, *base.attrs)
|
||||
base.attrs.each do |attr|
|
||||
base.send(:define_method, "#{attr}=") do |value|
|
||||
if self.class.attrs.include?(attr)
|
||||
write_attribute(attr, value)
|
||||
else
|
||||
raise ContentSecurityPolicyConfigError, "Unknown config directive: #{attr}=#{value}"
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def initialize(hash)
|
||||
@base_uri = nil
|
||||
@child_src = nil
|
||||
@connect_src = nil
|
||||
@default_src = nil
|
||||
@font_src = nil
|
||||
@form_action = nil
|
||||
@frame_ancestors = nil
|
||||
@frame_src = nil
|
||||
@img_src = nil
|
||||
@manifest_src = nil
|
||||
@media_src = nil
|
||||
@navigate_to = nil
|
||||
@object_src = nil
|
||||
@plugin_types = nil
|
||||
@prefetch_src = nil
|
||||
@preserve_schemes = nil
|
||||
@report_only = nil
|
||||
@report_uri = nil
|
||||
@require_sri_for = nil
|
||||
@require_trusted_types_for = nil
|
||||
@sandbox = nil
|
||||
@script_nonce = nil
|
||||
@script_src = nil
|
||||
@script_src_elem = nil
|
||||
@script_src_attr = nil
|
||||
@style_nonce = nil
|
||||
@style_src = nil
|
||||
@style_src_elem = nil
|
||||
@style_src_attr = nil
|
||||
@trusted_types = nil
|
||||
@worker_src = nil
|
||||
@upgrade_insecure_requests = nil
|
||||
@disable_nonce_backwards_compatibility = nil
|
||||
@config = {}
|
||||
|
||||
from_hash(hash)
|
||||
end
|
||||
|
||||
def initialize_copy(hash)
|
||||
@config = hash.to_h
|
||||
end
|
||||
|
||||
def update_directive(directive, value)
|
||||
self.send("#{directive}=", value)
|
||||
@config[directive] = value
|
||||
end
|
||||
|
||||
def directive_value(directive)
|
||||
if self.class.attrs.include?(directive)
|
||||
self.send(directive)
|
||||
end
|
||||
# No need to check attrs, as we only assign valid keys
|
||||
@config[directive]
|
||||
end
|
||||
|
||||
def merge(new_hash)
|
||||
|
|
@ -77,10 +35,7 @@ module SecureHeaders
|
|||
end
|
||||
|
||||
def to_h
|
||||
self.class.attrs.each_with_object({}) do |key, hash|
|
||||
value = self.send(key)
|
||||
hash[key] = value unless value.nil?
|
||||
end
|
||||
@config.dup
|
||||
end
|
||||
|
||||
def dup
|
||||
|
|
@ -113,8 +68,11 @@ module SecureHeaders
|
|||
|
||||
def write_attribute(attr, value)
|
||||
value = value.dup if PolicyManagement::DIRECTIVE_VALUE_TYPES[attr] == :source_list
|
||||
attr_variable = "@#{attr}"
|
||||
self.instance_variable_set(attr_variable, value)
|
||||
if value.nil?
|
||||
@config.delete(attr)
|
||||
else
|
||||
@config[attr] = value
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
|
@ -122,7 +80,7 @@ module SecureHeaders
|
|||
class ContentSecurityPolicyConfig
|
||||
HEADER_NAME = "Content-Security-Policy".freeze
|
||||
|
||||
ATTRS = PolicyManagement::ALL_DIRECTIVES + PolicyManagement::META_CONFIGS + PolicyManagement::NONCES
|
||||
ATTRS = Set.new(PolicyManagement::ALL_DIRECTIVES + PolicyManagement::META_CONFIGS + PolicyManagement::NONCES)
|
||||
def self.attrs
|
||||
ATTRS
|
||||
end
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче