Skip to content
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

Failed to run in SES #20

Closed
Jack-Works opened this issue Mar 2, 2022 · 4 comments
Closed

Failed to run in SES #20

Jack-Works opened this issue Mar 2, 2022 · 4 comments

Comments

@Jack-Works
Copy link

image

https://github.com/MikeMcl/decimal.js-light/blob/master/decimal.js#L1862

Change this line to

Object.defineProperty(this, 'constructor', {
    value: Decimal
})

can fix the problem, but this looks like a compiled file, I don't know how to make a PR

@MikeMcl
Copy link
Owner

MikeMcl commented Mar 2, 2022

What is SES?

In JavaScript, constructor is not a read-only property of an object, and this is an ECMAScript 3 compatible library, so Object.defineProperty is unacceptable anyway.

@Jack-Works
Copy link
Author

Hi! SES is a secured EcmaScript environment (and it's polyfill), in that environment, every built-in object (including Function.prototype) is frozen to protect from the prototype pollution attack.

When Function.prototype is not frozen, set on Decimal.constructor will not change the Function.prototype.constructor, but create a defined property:

image

But very unluck, when the prototype is frozen, write the property without Object.defineProperty will throw an error:

image

and this is an ECMAScript 3 compatible library, so Object.defineProperty is unacceptable anyway.

Is it possible to do this?

if (Object.defineProperty) Object.defineProperty(this, 'constructor', { value: Decimal })
else this.constructor = Decimal

@MikeMcl
Copy link
Owner

MikeMcl commented Mar 3, 2022

Is this the only change required to make this library SES-compatible?

Anyway, this is a very hot code path and adding another if statement and then calling Object.defineProperty will be much slower then the existing simple assignment.

SES and similar efforts look interesting but they seem too marginal at the moment for me to start making changes to support them.

@Jack-Works
Copy link
Author

Is this the only change required to make this library SES-compatible?

Maybe not, it is just the first error that appears, maybe some other assignments need to be changed too.

SES and similar efforts look interesting but they seem too marginal at the moment for me to start making changes to support them.

Thanks for your reply, maybe I will choose to patch the package in the node_modules to fix the problem.

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

No branches or pull requests

2 participants