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

fix: update minimal.js to evade override mistake #1742

Merged
merged 3 commits into from Jul 7, 2022

Conversation

erights
Copy link
Contributor

@erights erights commented Jun 11, 2022

See https://github.com/Agoric/agoric-sdk/blob/master/patches/%40confio%2Bics23%2B%2Bprotobufjs%2B6.11.3.patch

The original code used assignment to override the constructor and toString properties inherited from Error.prototype. However, if Error.prototype is frozen, as it is under Hardened JS (aka SES) or under the Node frozen intrinsics flag, then this assignment fails due to the JavaScript "override mistake".

enumerable: true would accurately preserve the behavior of the original assignment, but I'm guessing that was not intentional. For an actual error subclass, this property would not be enumerable, so my PR currently proposes that. But either would work, so let me know if you'd like me to change it.

configurable: false would accurately preserve the behavior of the original, but I'm guessing that was not intentional. For an actual error subclass, this property would be configurable. But either would work, so let me know if you'd like me to change it.

See https://github.com/Agoric/agoric-sdk/blob/master/patches/%40confio%2Bics23%2B%2Bprotobufjs%2B6.11.3.patch

The original code used assignment to override the `constructor` and `toString` properties inherited from Error.prototype. However, if `Error.prototype` is frozen, as it is under Hardened JS (aka SES) or under the Node frozen intrinsics flag, then this assignment fails due to the JavaScript "override mistake".

`enumerable: true` would accurately preserve the behavior of the original assignment, but I'm guessing that was not intentional. For an actual error subclass, this property would not be enumerable, so my PR currently proposes that. But either would work, so let me know if you'd like me to change it.

`configurable: false` would accurately preserve the behavior of the original, but I'm guessing that was not intentional. For an actual error subclass, this property would be configurable. But either would work, so let me know if you'd like me to change it.
@alexander-fenster alexander-fenster changed the title Update minimal.js to evade override mistake fix: update minimal.js to evade override mistake Jul 7, 2022
@alexander-fenster alexander-fenster merged commit e2f33a0 into protobufjs:master Jul 7, 2022
@erights
Copy link
Contributor Author

erights commented Jul 7, 2022

Thanks!

This was referenced Jul 8, 2022
turadg added a commit to Agoric/agoric-sdk that referenced this pull request Feb 26, 2024
turadg added a commit to Agoric/agoric-sdk that referenced this pull request Feb 28, 2024
turadg added a commit to Agoric/agoric-sdk that referenced this pull request Feb 28, 2024
turadg added a commit to Agoric/agoric-sdk that referenced this pull request Feb 28, 2024
turadg added a commit to Agoric/agoric-sdk that referenced this pull request Feb 28, 2024
turadg added a commit to Agoric/agoric-sdk that referenced this pull request Feb 29, 2024
turadg added a commit to Agoric/agoric-sdk that referenced this pull request Feb 29, 2024
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.

None yet

2 participants