-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
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
feat(web): Integrate trusted types into Vue #10491
Conversation
@yyx990803 Are the broken tests caused by my feature? They seems pretty unrelated... |
@@ -20,6 +21,7 @@ function updateDOMProps (oldVnode: VNodeWithData, vnode: VNodeWithData) { | |||
|
|||
for (key in oldProps) { | |||
if (!(key in props)) { | |||
// TT_TODO: when (how) is this even called |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is called when users use object bindings, e.g. v-bind="obj"
, and the new object has different keys from the old object. For example, on first render obj
could be { id: 'foo' }
and later updated to { innerHTML: 'bar' }
- in this case id
needs to be unset. This can also happen in manually written render functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, however I don't think I understand.
The props
and oldProps
are domProps from render function and v-bind
binds values from data property of the Vue component. I am still unable to trigger that part of the code, but I was trying to do something like this codesandbox. I also had a look at the original issue here, but their example is different from what I want to see.
The important question is: Can the code (where the TODO comment is) be called for dangerous properties like innerHTML
?
If yes, I need to add integration on this place as well, but I don't wanna include it on places which don't need it.
|
||
export function isTrustedValue(value: any): boolean { | ||
const tt = getTrustedTypes(); | ||
if (!tt) return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is window.trustedTypes
only present when the relevant CSP is enabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it will be available natively in browsers (for now just chrome with Experimental Web Platform features
enabled) or when there is a polyfill.
For this function it's logical to return false
when Trusted Types are not available. However, I think you meant the case when application is running in environment that has Trusted Types, but doesn't want to use them. In this case, the application won't have the CSP header present and in that case Trusted Types act as noop. However, we might add a parameter to Trusted Types to tell if the application wants to use them or not. This is already tracked issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: The code before was not compliant with what I said as I would throw and error in a specific case (innerHTML in svg in IE with trustedTypes available). I've fixed the code not to throw in this case.
Are there any plans to integrate this into vue? |
@Siegrift Is it still in progress? |
Hi, thanks for the interesest. This PR got quite stale since I first opened it. The integration implementation would likely need to be revisited now. I am not sure whether Vue is interested in having this change merged. |
I think it's best to close this PR and re-implement it. |
@Siegrift , hi, can you assist, when we can expect to get trusted types support? |
No idea, sorry. |
Trusted Types
Trusted Types (spec, introductory article) is a new experimental DOM API implemented within the WICG , with a working Chrome implementation.
The API creates a few new objects available on the global object in the browser, like most other web APIs (impl in TS and in Closure compiler).
Under certain conditions, controlled by a HTTP header (analogous to Content-Security-Policy behavior), the API can enable the enforcement - then it changes the signature of several DOM API functions and property setters, such that they accept specific object types, and reject strings. Colloquially, DOM API becomes strongly typed.
For example, with Trusted Types Element.innerHTML property setter accepts a TrustedHTML object.
Trusted Type objects stringify to their inner value. This API shape is a deliberate choice that enables existing web applications and libraries to gradually migrate from strings to Trusted Types without breaking functionality. In our example, it makes it possible to write the following:
The above code works regardless if the Trusted Types enforcement is enabled or not.
Reading from the DOM is unaffected, so Element.innerHTML getter returns a string. That's for practical reasons -- web applications read from DOM more often than they write to it, and only writing exposes the application to DOM XSS risks. Typing only the setters allows us to secure web applications with minimal code changes.
Integrating Trusted Types to Vue
The integration is fairly simple as there are not many places which break when using Vue with Trusted Types. There one place though, which I am not sure if it's even called (couldn't trigger it myself). It's marked in a code with
TT_TODO: ...
comment. I hope someone can clarify this for me. The PR doesn't introduce any breaking changes. All of the code is applied only when the trustedTypes API is available in the global object.What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
dev
branch for v2.x (or to a previous version branch), not themaster
branchfix #xxx[,#xxx]
, where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information:
The PR adds support for Trusted Types only on client side, but it might be beneficial to use them also on server side. There is no DOM on server side, however we can check if the values are trusted before creating markup with dangerous attributes. This also makes the applications render the same thing on the server and on client. There is already a similar PR for this for React.
If you would like to try this change there is a really simple application with the instructions how to setup vue locally in which you can play with Trusted Types. Vue cli uses webpack for development which provides features like hot-reloading. This currently breaks, but the PR is already fired and we are waiting for review.