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

Document.write and such as names are misleading #260

Closed
annevk opened this issue Feb 19, 2020 · 7 comments
Closed

Document.write and such as names are misleading #260

annevk opened this issue Feb 19, 2020 · 7 comments
Labels

Comments

@annevk
Copy link
Member

annevk commented Feb 19, 2020

The Document class does not have a static write property. I think we should make this Document.prototype.write or at least something that does not suggest a misleading thing (and which well might conflict in the future).

Via whatwg/webidl#841 (comment).

@koto
Copy link
Member

koto commented Feb 19, 2020

Thanks, that's a good point. We're taking the data from the context object in steps 1.1 and 1.2 here, so it should be easy. There is still a potential collision as we're using the DOM local names for Elements for simplification.

@annevk
Copy link
Member Author

annevk commented Feb 19, 2020

I guess elements will always be lowercase? However, you probably want to distinguish SVG script from HTML script?

@koto koto added the spec label Mar 2, 2020
@koto
Copy link
Member

koto commented Mar 3, 2020

There's probably no conflict possibility for static vs regular operations. In WebIDL 2.5.3:

The identifier of a static operation also must not be the same as the identifier of a regular operation defined on the same interface

There's a similar restriction for attributes:

The identifier of an attribute must not be the same as the identifier of another interface member defined on the same interface. The identifier of a static attribute must not be "prototype".

I looked into our implementation, and it doesn't seem like we have the information to distinguish between prototype / static readily avaliable in the exception state. We actually (contrary to the spec) always use the constructor name, so:

data:text/html,<meta http-equiv="content-security-policy" content="require-trusted-types-for 'script'"><script>addEventListener('securitypolicyviolation', (e) => alert(e.sample));s=document.createElement('iframe');s.srcdoc = 'aaa'</script>

alerts HTMLIframeElement.srcdoc

It sounds like this might actually be a good outcome - if we always use the constructor name, that suggests to the authors that the sample contains the information about the sink in IDL (which is what the checks will hook on to), regardless on whether it's static or on the prototype.

@annevk
Copy link
Member Author

annevk commented Mar 4, 2020

I didn't realize IDL forbids conflicts, but I don't think it's a good idea to standardize on this rather misleading naming either.

cc @domenic

@domenic
Copy link

domenic commented Mar 4, 2020

+1, as a web developer I find it supremely confusing when browser developers start talking about static methods and properties (like Document.write or HTMLIFrameElement.srcdoc) which do not exist.

At the very least, please use a different separator than "." A space would be a huge improvement.

@koto
Copy link
Member

koto commented Mar 4, 2020

Agreed that we should not confuse - if changing the separator to a space, or anything else really makes it so devs are not confused that the value is the identifier of the IDL interface and not the ES object, I'm all for it. Space it is.

koto added a commit to koto/trusted-types that referenced this issue Mar 4, 2020
@koto koto closed this as completed in cddc9e0 Mar 4, 2020
@koto
Copy link
Member

koto commented Mar 4, 2020

Changes to the spec:

  • referred to IDL's this value identifier instead of object constructor name to signify we're closer to IDL.
  • removed special casing for elements (e.g. svg => SVGElement)
  • used space instead of a dot, so HTMLIFrameElement srcdoc will be the value passed to the default policy, and later to CSP violation.
  • eval violations continue to use eval.
  • used "HTMLScriptElement src" and HTMLScriptElement text for violations when preparing a script. Added a todo for SVG scripts.
  • in CSP violation samples used | instead of a space to separate the sample prefix (sink name) from the payload (the string value).

It's much better now, thanks all!

@w3c w3c deleted a comment from ai-completed Oct 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants