search

cujojs / most

Ultra-high performance reactive programming
MIT License
3.49k stars 231 forks source link

Use Symbol.observable “ponyfill” #541

Closed kriskowal closed 3 years ago

kriskowal commented 3 years ago

This change upgrades symbol-observable and changes use to symbol-observable/ponyfill so that MostJS can be used in JS environments where the primordials are frozen to mitigate prototype pollution attacks.

I have not added unit tests to verify this, but with your consent, I can add a development dependency on SES and use that to lock down the primordials during tests.

This is not a user-visible change, but does enable MostJS to be used in more environments. I do not see a change log to mention this in.

kriskowal commented 3 years ago

Good to see you again, @briancavalier 😊

briancavalier commented 3 years ago

Hey @kriskowal 👋 Thanks for this, and for the note about globalThis. I'll take a closer look soon.

kriskowal commented 3 years ago

Posted follow-up changes to use a globalthis polyfill so this can still work with a no-eval CSP.

kriskowal commented 3 years ago

Hey @kriskowal 👋 Thanks for this, and for the note about globalThis. I'll take a closer look soon.

Awesome. I took care of globalThis so this should work with a no-eval CSP now. Took this back out of draft.

briancavalier commented 3 years ago

I have not added unit tests to verify this, but with your consent, I can add a development dependency on SES and use that to lock down the primordials during tests.

Oops, I just noticed this! Can you say a bit more about what's involved in adding the SES dep and modifying the tests?

kriskowal commented 3 years ago

I’ve pushed fixes for the remaining symbol-observables, and removed the commit that added a yarn.lock unintentionally.

kriskowal commented 3 years ago

Oops, I just noticed this! Can you say a bit more about what's involved in adding the SES dep and modifying the tests?

I could do this in a follow-up. The purpose would be to block future changes that would introduce prototype pollution, either directly or through an upgraded third-party.

This would involve adding import "ses" and calling lockdown() or lockdown({errorTaming: "unsafe"}) at the head of your test runner, or creating a second entry point to your tests that locks down and runs the tests again under those conditions.

kriskowal commented 3 years ago

@briancavalier I believe this is once again ready for review. Please let me know if this is a satisfactory change.

briancavalier commented 3 years ago

@kriskowal

This would involve adding import "ses" and calling lockdown() or lockdown({errorTaming: "unsafe"}) at the head of your test runner, or creating a second entry point to your tests that locks down and runs the tests again under those conditions.

I like the idea of a second test entrypoint if that's relatively simple. Would you be up for opening a PR for that so we can see what it looks like?

BTW, have you looked at @most/core? It's the new version of the core ideas of most, with a primarily functions-only API. If we're able to add SES testing in a simple way here, I could imagine adding it there as well if the other maintainers are open to it.

Thanks again!

kriskowal commented 3 years ago

Thank you! I will watch for the next release.

briancavalier commented 3 years ago

@kriskowal We just published v1.9.0 with this change. Sorry for the delay. We had to deal with some quirks of most's antiquated build setup.

kriskowal commented 3 years ago

Thank you @briancavalier. You have my sympathies. I have a number of projects with a crusty CI setup that hasn’t run in years. I’ll let you know if I run into integration trouble rolling this up.