Skip to content

fix(zone.js): read Symbol.species safely#45369

Closed
arturovt wants to merge 1 commit intoangular:masterfrom
arturovt:fix/zonejs-symbol-species
Closed

fix(zone.js): read Symbol.species safely#45369
arturovt wants to merge 1 commit intoangular:masterfrom
arturovt:fix/zonejs-symbol-species

Conversation

@arturovt
Copy link
Contributor

@arturovt arturovt commented Mar 16, 2022

We must read Symbol.species safely because this may be anything. For instance, this
may be an object without a prototype (created through Object.create(null)); thus
this.constructor will be undefined. One of the use cases is SystemJS creating
prototype-less objects (modules) via Object.create(null). The SystemJS creates an empty
object and copies promise properties into that object (within the getOrCreateLoad
function). The zone.js then checks if the resolved value has the then method and invokes
it with the value context. Otherwise, this will throw an error: TypeError: Cannot read properties of undefined (reading 'Symbol(Symbol.species)').

PR Checklist

PR Type

  • Bugfix

Does this PR introduce a breaking change?

  • Yes
  • No

@pullapprove pullapprove bot requested a review from JiaLiPassion March 16, 2022 19:47
@arturovt arturovt force-pushed the fix/zonejs-symbol-species branch 2 times, most recently from d34c87c to 3832751 Compare March 16, 2022 23:19
@JiaLiPassion
Copy link
Contributor

@arturovt , thank you for the PR, could you add a test case around here

it('should allow subclassing without Symbol.species', () => {
?

Copy link
Contributor

@JiaLiPassion JiaLiPassion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add test case

@arturovt arturovt force-pushed the fix/zonejs-symbol-species branch 6 times, most recently from 09a6baa to 2ae2267 Compare March 17, 2022 12:36
@arturovt arturovt requested a review from JiaLiPassion March 17, 2022 13:00
We must read `Symbol.species` safely because `this` may be anything. For instance, `this`
may be an object without a prototype (created through `Object.create(null)`); thus
`this.constructor` will be undefined. One of the use cases is SystemJS creating
prototype-less objects (modules) via `Object.create(null)`. The SystemJS creates an empty
object and copies promise properties into that object (within the `getOrCreateLoad`
function). The zone.js then checks if the resolved value has the `then` method and invokes
it with the `value` context. Otherwise, this will throw an error: `TypeError: Cannot read
properties of undefined (reading 'Symbol(Symbol.species)')`.
@arturovt arturovt force-pushed the fix/zonejs-symbol-species branch from 2ae2267 to 0046c5d Compare March 18, 2022 23:29
Copy link
Contributor

@JiaLiPassion JiaLiPassion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@JiaLiPassion JiaLiPassion added area: zones Issues related to zone.js target: patch This PR is targeted for the next patch release labels Mar 20, 2022
@ngbot ngbot bot modified the milestone: Backlog Mar 20, 2022
@dylhunn
Copy link
Contributor

dylhunn commented Mar 24, 2022

@arturovt @JiaLiPassion Is this ready to be merged?

@JiaLiPassion JiaLiPassion added the action: merge The PR is ready for merge by the caretaker label Mar 25, 2022
@JiaLiPassion
Copy link
Contributor

@dylhunn , yes, this one is ready to merge, thank you.

@dylhunn
Copy link
Contributor

dylhunn commented Mar 25, 2022

This PR was merged into the repository by commit e2eaac3.

@dylhunn dylhunn closed this in e2eaac3 Mar 25, 2022
dylhunn pushed a commit that referenced this pull request Mar 25, 2022
We must read `Symbol.species` safely because `this` may be anything. For instance, `this`
may be an object without a prototype (created through `Object.create(null)`); thus
`this.constructor` will be undefined. One of the use cases is SystemJS creating
prototype-less objects (modules) via `Object.create(null)`. The SystemJS creates an empty
object and copies promise properties into that object (within the `getOrCreateLoad`
function). The zone.js then checks if the resolved value has the `then` method and invokes
it with the `value` context. Otherwise, this will throw an error: `TypeError: Cannot read
properties of undefined (reading 'Symbol(Symbol.species)')`.

PR Close #45369
@arturovt arturovt deleted the fix/zonejs-symbol-species branch March 25, 2022 01:58
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: zones Issues related to zone.js target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants