Skip to content

replace crockford encoding dependencies#4374

Merged
mitchellhenke merged 10 commits intomasterfrom
mitchellhenke/remove-vulnerable-crockford
Nov 3, 2020
Merged

replace crockford encoding dependencies#4374
mitchellhenke merged 10 commits intomasterfrom
mitchellhenke/remove-vulnerable-crockford

Conversation

@mitchellhenke
Copy link
Contributor

Replaces vulnerable version of Crockford encoding library with a different one

I pulled out the encoding/decoding to be able to test easier, but I wasn't sure if there was a more appropriate place to put it?

@aduth
Copy link
Contributor

aduth commented Nov 3, 2020

I pulled out the encoding/decoding to be able to test easier, but I wasn't sure if there was a more appropriate place to put it?

packs can work, but each pack is treated as a distinct Webpack entrypoint, which means it'll generate an output JavaScript file. Normally, this would be intended to be used as standalone, brought into a page using javascript_pack_tag. The way we're using it is more as a utility module.

Instead, I'd imagine one of two alternatives:

  • Keep it in app/javascript/packs/personal-key-page-controller.js. Your tests should be able to import from that file the same way they are currently from personal-key-input-encoder.js.
  • Create it as a utility package in app/javascript/packages (documentation, prior art), which helps keep it a bit more self-contained as well.

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/remove-vulnerable-crockford branch from 527b9a9 to 6c22d87 Compare November 3, 2020 15:43
Mitchell Henke and others added 2 commits November 3, 2020 10:14
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Mitchell Henke and others added 5 commits November 3, 2020 10:37
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@mitchellhenke mitchellhenke merged commit c2878b0 into master Nov 3, 2020
@mitchellhenke mitchellhenke deleted the mitchellhenke/remove-vulnerable-crockford branch November 3, 2020 17:24
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

Successfully merging this pull request may close these issues.

2 participants