Skip to content

fix(http-user-agent): remove eval in favor of CSP compliant browser detection#480

Merged
tdeekens merged 3 commits into
masterfrom
td-fix-csp-browser-detection
Feb 27, 2018
Merged

fix(http-user-agent): remove eval in favor of CSP compliant browser detection#480
tdeekens merged 3 commits into
masterfrom
td-fix-csp-browser-detection

Conversation

@tdeekens
Copy link
Copy Markdown
Contributor

Summary

This relates to #479 and closes #479.

The browser detection was previously performed using a new Function which breaks harder CSP rules. Side note: jQuery also used new Function to parse JSON for a long time.

Detecting the browser can be done in a mirage of ways. Often using an eval or new Function statement. There are also packages like global-or-window which return the Node.JS global or the window. Something we don't specifically need.

The new check is aligned with what we do at DOMPurify. Where we also had an issue filed around the same issue here.

Copy link
Copy Markdown
Member

@emmenko emmenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks!!! ❤️🙏

Copy link
Copy Markdown
Contributor

@wizzy25 wizzy25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

@wizzy25
Copy link
Copy Markdown
Contributor

wizzy25 commented Feb 27, 2018

Unit tests aren't passing

@tdeekens
Copy link
Copy Markdown
Contributor Author

Unit tests aren't passing

Already on it.

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 27, 2018

Codecov Report

Merging #480 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #480      +/-   ##
==========================================
+ Coverage   98.57%   98.57%   +<.01%     
==========================================
  Files          85       85              
  Lines        1899     1900       +1     
  Branches      463      464       +1     
==========================================
+ Hits         1872     1873       +1     
  Misses         24       24              
  Partials        3        3
Impacted Files Coverage Δ
packages/http-user-agent/src/create-user-agent.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac1460f...26e7992. Read the comment docs.

@tdeekens tdeekens merged commit 7455b35 into master Feb 27, 2018
@tdeekens tdeekens deleted the td-fix-csp-browser-detection branch February 27, 2018 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[http-user-agent] Avoid using "new Function" to check browser environment

3 participants