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

feat(web): Integrate trusted types into Vue #10491

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/core/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ export type Config = {

// legacy
_lifecycleHooks: Array<string>;

// trusted types (https://github.com/WICG/trusted-types)
trustedTypesPolicyName: string;
};

export default ({
Expand Down Expand Up @@ -126,5 +129,11 @@ export default ({
/**
* Exposed for legacy reasons
*/
_lifecycleHooks: LIFECYCLE_HOOKS
_lifecycleHooks: LIFECYCLE_HOOKS,

/**
* Trusted Types policy name which will be used by Vue. More
* info about Trusted Types on https://github.com/WICG/trusted-types.
*/
trustedTypesPolicyName: 'vue'
}: Config)
4 changes: 3 additions & 1 deletion src/platforms/web/runtime/modules/dom-props.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import { isDef, isUndef, extend, toNumber } from 'shared/util'
import { isSVG } from 'web/util/index'
import {maybeCreateDangerousSvgHTML} from 'web/security'

let svgContainer

Expand All @@ -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
Copy link
Member

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.

Copy link
Author

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.

elm[key] = ''
}
}
Expand Down Expand Up @@ -51,7 +53,7 @@ function updateDOMProps (oldVnode: VNodeWithData, vnode: VNodeWithData) {
} else if (key === 'innerHTML' && isSVG(elm.tagName) && isUndef(elm.innerHTML)) {
// IE doesn't support innerHTML for SVG elements
svgContainer = svgContainer || document.createElement('div')
svgContainer.innerHTML = `<svg>${cur}</svg>`
svgContainer.innerHTML = maybeCreateDangerousSvgHTML(cur)
const svg = svgContainer.firstChild
while (elm.firstChild) {
elm.removeChild(elm.firstChild)
Expand Down
41 changes: 41 additions & 0 deletions src/platforms/web/security.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/* @flow */
import Vue from 'core/index'
import {getTrustedTypes, isTrustedValue} from 'shared/util'

type TrustedTypePolicy = {
// value returned is actually an object with toString method returning the wrapped value
createHTML: (value: any) => string;
};

let policy: ?TrustedTypePolicy
// create policy lazily to simplify testing
function getOrCreatePolicy() {
const tt = getTrustedTypes()
if (tt && !policy) {
policy = tt.createPolicy(Vue.config.trustedTypesPolicyName, {createHTML: (s) => s});
}

return policy
}

if (process.env.NODE_ENV !== 'production') {
// we need this function to clear the policy in tests
Vue.prototype.$clearTrustedTypesPolicy = function() {
policy = undefined
}
}

export function maybeCreateDangerousSvgHTML(value: any): string {
const tt = getTrustedTypes()

if (!tt || !isTrustedValue(value)) return `<svg>${value}</svg>`;
// flow complains that 'getOrCreatePolicy()' might return null
else return (getOrCreatePolicy(): any).createHTML(`<svg>${value}</svg>`);
}

export function getTrustedShouldDecodeInnerHTML(href: boolean): string {
const html = href ? `<a href="\n"/>` : `<div a="\n"/>`;
const p = getOrCreatePolicy()
if (!p) return html;
else return p.createHTML(html);
}
3 changes: 2 additions & 1 deletion src/platforms/web/util/compat.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
/* @flow */

import { inBrowser } from 'core/util/index'
import {getTrustedShouldDecodeInnerHTML} from 'web/security'

// check whether current browser encodes a char inside attribute values
let div
function getShouldDecode (href: boolean): boolean {
div = div || document.createElement('div')
div.innerHTML = href ? `<a href="\n"/>` : `<div a="\n"/>`
div.innerHTML = getTrustedShouldDecodeInnerHTML(href)
return div.innerHTML.indexOf('&#10;') > 0
}

Expand Down
30 changes: 25 additions & 5 deletions src/shared/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,35 @@ export function isPromise (val: any): boolean {
)
}

let trustedTypes = undefined
export function getTrustedTypes() {
if (trustedTypes === undefined) {
// TrustedTypes have been renamed to trustedTypes https://github.com/WICG/trusted-types/issues/177
trustedTypes = typeof window !== 'undefined' ? (window.trustedTypes || window.TrustedTypes) : null;
}
return trustedTypes;
}

export function isTrustedValue(value: any): boolean {
const tt = getTrustedTypes();
if (!tt) return false;
Copy link
Member

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?

Copy link
Author

@Siegrift Siegrift Sep 16, 2019

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.

Copy link
Author

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.

// TrustedURLs are deprecated and will be removed soon: https://github.com/WICG/trusted-types/pull/204
else return tt.isHTML(value) || tt.isScript(value) || tt.isScriptURL(value) || (tt.isURL && tt.isURL(value))
}

/**
* Convert a value to a string that is actually rendered.
*/
export function toString (val: any): string {
return val == null
? ''
: Array.isArray(val) || (isPlainObject(val) && val.toString === _toString)
? JSON.stringify(val, null, 2)
: String(val)
if (isTrustedValue(val)) {
return val;
} else {
return val == null
? ''
: Array.isArray(val) || (isPlainObject(val) && val.toString === _toString)
? JSON.stringify(val, null, 2)
: String(val)
}
}

/**
Expand Down
Loading