Throw exceptions instead of user warnings/errors

[jens1o]

As discussed in #60 (comment), I would like the library is avoid of triggering php errors, but instead throw SPL Exceptions, which were created for this case.

At the moment, I'm tired, and I can't describe more things why I want to catch exceptions instead of errors.

[aidantwoods]

Just to replicate some of the referenced discussion in this issue so this issue is self-contained:

---

This is definitely something worth thinking about for 3.0, would be a BC break to start doing it by default now (at least on current functionality). Perhaps there is room for refactoring things into exceptions and have SecureHeaders convert them to errors by default, but have a config to let the exceptions go uncaught and be exposed to the user.

Things to think about with errors v.s. exceptions:

• Exceptions will generally cause headers not to reach the adapter, and so won't be sent. i.e.
  • Is no CSP better than suboptimal CSP? (bearing in mind loss of reporting header which might be stricter too).
  • Is not sending a suboptimal CSP better than HSTS, Referrer-Policy, cookie upgrades, ...?
• It we throw exceptions, they should be genuine problems. That means not throwing an exception for advice: we cannot use exceptions to tell the dev they they should use something (learning opportunity). Learning opportunities would have to be disposed of if we converted all errors as used currently to exceptions.

So I think it best to discuss decoupling some things that should be exceptions, from things that are learning opportunities. I don't want to throw an exception every time SecureHeaders can advise to do something (in-fact because these advisements are not exceptions, new advisements can be added in a backwards compatible manner – which is good for pushing current best standards. This is not achievable with things that will cause a fatal error).

[franzliedke]

I think I've mentioned this before: if we are talking about genuine problems, then yes, exceptions should be thrown, so that developers notice and can deal with them. Headers will get lost, but so be it, you will likely have larger problems at that point.

For advice, you are right, probably shouldn't use the exception mechanism for that.

[aidantwoods]

No contest to any of that.

---

Speaking to the original issue raised RE #60, I consider the message given there to be advice. It's strictly opportunistic injection (see #1). We want to encourage the user to adopt nonces/hashes in favour of the whitelist (strict dynamic tells CSP3 browsers to kill the whitelist). If they can't do that then we don't inject strict dynamic because killing the whitelist without any nonces/hashes will likely break their application (not because it's wrong on a policy writing level).

---

Further to the discussion here, are there any errors that come to mind that we currently issue, that probably should be exceptions?

[aidantwoods]

@jens1o Were there any other situations you had in mind where SecureHeaders issues a warning but it maybe should be an exception?

I think I'm probably going to look into being able to mute any warning that SecureHeaders issues. You can already do this for some things, but I think it would be beneficial to have an "I accept this specific risk" option for everything. This would let you turn off a specific warning without bulk disabling all errors (and consequently being blind to all advisories). The code disabling that specific warning would also serve as a reminder in future to a reader as-to whether it is still an appropriate risk. Not sure if this would perhaps better address the situation here?

[jens1o]

Hi there,

tbh, I do not know the codebase anymore. So I do not know exactly where I was complaining, and I do not have time to dig into this atm. :/

But I do like your approach in general :)

[aidantwoods]

In that case I think I'll close this for now in favour of the changes described (I'll track these in #71), do feel free to re-open if you spot something at some point though :)

I think I may try and aim towards a way at receiving these warnings and notices in ways other than just page output too – so if desired you could log them separately for example.