Skip to content
This repository has been archived by the owner on Aug 3, 2020. It is now read-only.

Remove browser sniffing #97

Closed
EvanHahn opened this issue Jan 9, 2020 · 6 comments
Closed

Remove browser sniffing #97

EvanHahn opened this issue Jan 9, 2020 · 6 comments
Assignees

Comments

@EvanHahn
Copy link
Member

EvanHahn commented Jan 9, 2020

I plan to remove browser sniffing from the next major version of helmet-csp.

Different browsers have different support for Content Security Policies. Some only support certain directives, where some have different headers (like X-Webkit-CSP). Currently, this module sniffs the browser's User-Agent to figure out what headers to set. However, I'm planning to remove this from the next major version.

My reasons:

  1. Browser sniffing has been the source of many bugs due to its complexity.
  2. Browser sniffing makes this module difficult for me to maintain confidently.
  3. Modern browsers' CSP implementations are fairly stable, and user share of old browsers is dropping. That makes browser sniffing less useful.
  4. Content Security Policies are typically used as defense-in-depth rather than the first line of defense against attacks. For example, you should probably sanitize user input and not rely on CSP.
  5. Parsing and switching on the User-Agent is slower and uses more memory.

I opened this issue to track the work, but mostly to solicit feedback. If you rely on browser sniffing and would be sad to see it go, or if you have other thoughts, let me know!

@XhmikosR
Copy link
Contributor

I think it's fine to do this in a major version bump. Which browsers still need the old header exactly? On the other hand having it disabled by default, does it slow down things still?

@EvanHahn
Copy link
Member Author

Browsers have varying support for CSP. Some use different headers (like X-WebKit-CSP instead of Content-Security-Policy) where others have different names for directives (like allow instead of default-src). This module currently tries to figure those things out, but I intend to remove them unless people think doing so is a bad idea.

@kara-ryli
Copy link
Contributor

Definitely 👍. We've been running with browser sniffing off since #32 landed -- my biggest concern at that time was the cacheability of browser-sniffed requests.

@XhmikosR
Copy link
Contributor

TBH I too have browser sniffing off for years.

@webuniverseio
Copy link

👍 I had to turn off browser sniffing too and reasons for removal make sense.

@EvanHahn
Copy link
Member Author

EvanHahn commented Aug 2, 2020

This has been addressed in helmet@4 and helmet-csp@3.0.0.

I'm going to be archiving this repository soon and moving everything to https://github.com/helmetjs/helmet/, so feel free to open an issue there if you run into any problems.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

4 participants