Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance issue by supported_directives method #187

Closed
igrep opened this issue Oct 23, 2015 · 3 comments
Closed

Performance issue by supported_directives method #187

igrep opened this issue Oct 23, 2015 · 3 comments

Comments

@igrep
Copy link

igrep commented Oct 23, 2015

Problem

The change in #179 can cause a performance issue.

I learned that lib/secure_headers/headers/content_security_policy.rb,
UserAgentParser.parse(@ua) in supported_directives is executed for each request.
As user_agent_parser's author is planning to remove, UserAgentParser.parse(@ua) is problematic.
Because it loads common regexps yaml file as large as 4.7k lines on every call.

Possible solution

According to user_agent_parser's README's recommendation, we should instantiate a user_agent_parser by UserAgentParser::Parser.new, then cache it.
So, which class / object should cache a user_agent_parser?

@oreoshake
Copy link
Contributor

In the upcoming 3.x series I've moved it here: https://github.com/twitter/secureheaders/blob/dd1fd6ab751e6026431be9ecb25e8664f70ee714/lib/secure_headers.rb#L26. I'll do the same for the 2.x series. Thanks.

oreoshake added a commit that referenced this issue Oct 23, 2015
Thanks to @igrep for pointing this out.

Fixes #187
@oreoshake
Copy link
Contributor

2.4.3 has been released with this fix. Thanks!

@igrep
Copy link
Author

igrep commented Oct 27, 2015

You're welcome! 😄
I confirmed it works perfectly on our production environment! 🎉
Thank you very much for fixing! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants