-
Notifications
You must be signed in to change notification settings - Fork 128
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
Translate remaining jQuery #915
Conversation
strip-js loads https://github.com/cheeriojs/cheerio, as a dependency, which is similar to jQuery. All our xhr data comes from same origin at this point, so this seems too high a price in page weight to pay. But as security best practice we should still pass all the client side generated html through a xss guard. https://github.com/cure53/DOMPurify comes in at 14.7 kb minified and is actively maintained, any reasons not to use that instead? |
Ughh. I didn't see the dependency. DOMPurify was my first choice, but it's a little too aggressive in it's HTML validation. It stripped all the The Google Caja based scrubbers ( I tried There is also One last thought for now. We could use a really aggressive sanitizer and just filter inserted values instead of chunks of HTML. We could even strip out all HTML if it targeted at just the values. I played with this a little, but it gets pretty nasty in some places. |
I've got a little more on this. DOMPurify internally does something like this.
This will yield
Which also might yield some performance benefits (see cure53/DOMPurify#190 and cure53/DOMPurify#191). |
Worked out the QR code animation. Used the new |
Great. I'm glad that's helping. I'm prepping a PR to complete the feature by rewinding sqlite too. |
Testing well, was not able to find any bugs that are not already on master. Please also remove jquery from the externals declarations in our webpack config |
This is a full translation of all remaining jQuery to native JS.
Thestrip-js
module is used to replacejQuery.parseHTML
and to scrub inserted HTML for cross-site scripting defense. It's an improvement overparseHTML
all around, but doesn't cover some vulnerabilities from older browsers.The
DOMPurify
module is brought in to replacejquery.parseHTML
, and to scrub inserted HTML as an XSS defense.The following are the major points to check if testing.
Address page
Charts page
Mempool page
It looks like #718 changed the response type for the
getmempooltxs
websocket request fromMempoolInfo
toTrimmedTxInfo
which broke the mempool page somewhat. I did not address that in this PR. I will open an issue or just pick it up separately.Sync status page: Progress bar updates. Hard to test, but I checked it on a fresh sync.
Nexthome page:
Also fixed a bug in
humanize.decimalParts
where integers were not being displayed.Raw transaction send/decode: Message decoding and broadcasting working as expected.
Sacrificed a jQuery fade-in effect, though I could create an animation if needed or use one of many packages available through npm.Keyboard navigation
Dark theme
Don't forget to
npm install
.