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

CustomElementsRegistry.define should guard against recursion #1329

Closed
dominiccooney opened this issue May 27, 2016 · 1 comment
Closed

CustomElementsRegistry.define should guard against recursion #1329

dominiccooney opened this issue May 27, 2016 · 1 comment
Assignees
Labels
topic: custom elements Relates to custom elements (as defined in DOM and HTML)

Comments

@dominiccooney
Copy link

I’m implementing define in Blink.

Brief background: Step 3 checks that a given name is not already in use in a registry; steps 8, 10, 12, 14 and 16 (maybe others) may run script; and step 19 establishes a definition in a registry.

Here's some feedback: The order of steps 3, 8/10/12/14/16 and 19 permit redefinition of a custom element name in a given registry. This is implementable but pretty undesirable from my point of view.

For example, create a constructor whose prototype property is backed by a getter which at step 10 in turn calls define with an unrelated definition of the same name. This will pass the check in step 2 (because the first invocation did not reach step 19 yet) and add one definition to the registry which is overwritten after the call returns and the original invocation reaches step 19.

It would be simpler if define did not permit recursion, or at step 3 it erected some kind of tombstone with name that prevented any recursive redefinition of name, or moved step 3 to occur immediately before step 19 so no author script could intercede between checking whether a name is in use and putting the name in use.

@domenic domenic self-assigned this May 27, 2016
@domenic domenic added the topic: custom elements Relates to custom elements (as defined in DOM and HTML) label May 27, 2016
@annevk
Copy link
Member

annevk commented May 27, 2016

Should probably also copy @rniwa on these issues (@rniwa, you might want to click on the label to see more).

domenic added a commit that referenced this issue May 27, 2016
Fixes #1329, by disallowing reentrant element definition from defining
something with a name or constructor that is currently being defined.
domenic added a commit that referenced this issue May 27, 2016
Fixes #1329, by disallowing reentrant element definition from defining
something with a name or constructor that is currently being defined.

Also properly wraps the script-executing steps in a "prepare to run
script"/"clean up after running script" pair, as part of #885.
domenic added a commit that referenced this issue May 27, 2016
Fixes #1329, by disallowing reentrant element definition from defining
something with a name or constructor that is currently being defined.

Also properly wraps the script-executing steps in a "prepare to run
script"/"clean up after running script" pair, as part of #855.
domenic added a commit that referenced this issue Jun 6, 2016
Fixes #1329, by disallowing reentrant element definition from defining
something with a name or constructor that is currently being defined.

Also properly wraps the script-executing steps in a "prepare to run
script"/"clean up after running script" pair, as part of #855.
annevk pushed a commit that referenced this issue Jun 7, 2016
Fixes #1329, by disallowing reentrant element definition from defining
something with a name or constructor that is currently being defined.

Also properly wraps the script-executing steps in a "prepare to run
script"/"clean up after running script" pair, as part of #855.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: custom elements Relates to custom elements (as defined in DOM and HTML)
Development

No branches or pull requests

3 participants