Remove eval#21
Conversation
…rs in https://rrwebcloud.com/record.js - we don't want it in the bookmarklet (config already well formed) or in the NPM package
… e.g. for the bookmarklet we will inject some indicator code here
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval#never_use_direct_eval! | ||
| return eval?.(`"use strict";(${obj})`); | ||
| // this is replaced by an eval in prepublish-rrweb.sh | ||
| console.log(`couldn't parse config as JSON: ${obj}`); |
There was a problem hiding this comment.
CRITICAL: Config parsing is completely broken - this function now always returns an empty object {} instead of parsing the config. When JSON.parse fails (line 484-490), it falls back to looseJsonParse which returns {}, meaning users' custom configurations will be lost.
The original code used eval to parse JavaScript-style config objects (e.g., {blockSelector: '.my-block-selector'}). While removing eval is good for security, this replacement doesn't actually parse anything - it just silently fails.
Suggested fix: Either:
- Actually implement JSON5 parsing (there are libraries like json5 npm package)
- Or keep a safe eval alternative that doesn't use
direct eval(e.g., using Function constructor with limited scope)
| } | ||
| const i = document.getElementById('rrwebcloud-recording-indicator'); | ||
| if (i) { | ||
| i.remove(); |
There was a problem hiding this comment.
WARNING: Element.remove() method is not supported in IE or older browsers. Consider using element.parentNode?.removeChild(element) for broader compatibility, or ensure this library only targets modern browsers.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (1 file)
Review generated by Kilo Code Review |
1 participant
Take out the
evalas it will cause problems when publishing or importing depending on the environmentLeave a placeholder function for looseParseJSON which can be post processed to run something like https://github.com/ssnau/loose-json (or the previous code included here)
Also incidentally add some 'stop' code to remove an rrwebcloud bookmarklet indicator if present