Handle very dynamic policies better

[oreoshake]

Fixes #217

Currently, an idempotency check is performed for every policy addition. Not only does this create a fair number of temporary objects, but it also busts the entire header cache if any changes are detected.

This change defers idempotency checks until just before the header values are generated. If we can determine that the CSP will differ from the cached values, we discard all cached CSP and generate one specific to that UA.

If there is a change to X-Frame-Options, we cache the new value immediately.

As a result, the value for cached_headers will always maintain the headers that will be applied and there is no need to generate them when calling header_hash_for.

Because we are retaining the previously discarded cached headers, we only call dup on the configuration option if it is frozen to save bytes and cycles. All policies added via default and override are already frozen automatically, so a non-frozen config object can be modified freely and we can assume it's dynamic in nature.

To recap:

Before	After
idempotent_additions? called per policy modification	idempotent_additions? called once per request
Entire header cache was busted per policy/xfo modification	the CSP header cache is busted once per request, only policies for the current UA are regenerated
config objects were dup'd per modification	config values dup'd at most one time per request
XFO changes busted the entire header cache	XFO changes modify the header cache directly
Opting out of a header would bust the cache	Opting out of a header modifies the cache

[oreoshake]

I suppose with this change you could argue that we only need to dup/operate on the header cache instead of maintaining a parallel config. Only the case of CSP do we operate on the config values without immediately modifying the header cache.

The only reason to keep the config fields around is to lookup the value (e.g. the CSP config hash) and I'm not sure this would ever happen IRL, or at the least it would be an antipattern - business logic around what headers are currently configured should probably be handled in application code.

[oreoshake]

This is ready for some 👀 if anyone has a moment.

Note: this is a breaking API change - retrieving config values will only work for CSP and secure_cookies. I really can't see anyone working directly with a config object for the other headers and trying to retrieve their values. e.g.

💥

SecureHeaders.override(:my_override) do |config|
  config.x_frame_options = config.x_frame_options == "sameorigin" ? "sameorigin" : "deny"
end

No 💥

SecureHeaders.override(:my_override) do |config|
  config.csp = config.csp.merge(:script_src => %w(data:)) # don't ever add data: to scriptsrc btw
end

[ptoomey3]

Given this PR is optimizing policy overrides are, I was surprised there was no test to validate that you are in fact only busting the cache once even if there are > 1 overrides. Does that seem a reasonable thing to add?

[ptoomey3]

Would it also make sense to add a test to verify that overrides never end up touching csp and only touch dynamic_csp?

[oreoshake]

Added a test here for the cache busting. I'll have to think about your second item.

[ptoomey3]

Other than small test nits, this seems logical. 👍

[coveralls]

Coverage decreased (-0.2%) to 95.061% when pulling 8a314e1 on single-idempotency-check into 76fb828 on master.

[coveralls]

Coverage decreased (-0.1%) to 95.129% when pulling cbe7014 on single-idempotency-check into 76fb828 on master.