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

⭐ new: HTML locale message warning option #567

Merged
merged 8 commits into from
Apr 26, 2019
Merged
Show file tree
Hide file tree
Changes from 4 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
5 changes: 5 additions & 0 deletions decls/i18n.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ declare type FormattedNumberPart = {
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/NumberFormat/formatToParts#Return_value
declare type NumberFormatToPartsResult = Array<FormattedNumberPart>;

declare type WarnHtmlInMessageLevel = 'off' | 'warn' | 'error';

declare type I18nOptions = {
locale?: Locale,
fallbackLocale?: Locale,
Expand All @@ -74,6 +76,7 @@ declare type I18nOptions = {
silentFallbackWarn?: boolean,
pluralizationRules?: PluralizationRules,
preserveDirectiveContent?: boolean,
warnHtmlInMessage?: WarnHtmlInMessageLevel,
};

declare type IntlAvailability = {
Expand Down Expand Up @@ -110,6 +113,8 @@ declare interface I18n {
set pluralizationRules (rules: PluralizationRules): void,
get preserveDirectiveContent (): boolean,
set preserveDirectiveContent (preserve: boolean): void,
get warnHtmlInMessage (): WarnHtmlInMessageLevel,
set warnHtmlInMessage (level: WarnHtmlInMessageLevel): void,

getLocaleMessage (locale: Locale): LocaleMessageObject,
setLocaleMessage (locale: Locale, message: LocaleMessageObject): void,
Expand Down
65 changes: 65 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import { install, Vue } from './install'
import {
warn,
error,
isNull,
parseArgs,
isPlainObject,
Expand All @@ -17,6 +18,7 @@ import I18nPath from './path'

import type { PathValue } from './path'

const htmlTagMatcher = /<\/?[\w\s="/.':;#-\/]+>/gi
const linkKeyMatcher = /(?:@(?:\.[a-z]+)?:(?:[\w\-_|.]+|\([\w\-_|.]+\)))/g
const linkKeyPrefixMatcher = /^@(?:\.([a-z]+))?:/
const bracketsMatcher = /[()]/g
Expand Down Expand Up @@ -46,6 +48,7 @@ export default class VueI18n {
_path: I18nPath
_dataListeners: Array<any>
_preserveDirectiveContent: boolean
_warnHtmlInMessage: WarnHtmlInMessageLevel
pluralizationRules: {
[lang: string]: (choice: number, choicesLength: number) => number
}
Expand Down Expand Up @@ -87,6 +90,7 @@ export default class VueI18n {
? false
: !!options.preserveDirectiveContent
this.pluralizationRules = options.pluralizationRules || {}
this._warnHtmlInMessage = options.warnHtmlInMessage || 'off'

this._exist = (message: Object, key: Path): boolean => {
if (!message || !key) { return false }
Expand All @@ -96,6 +100,12 @@ export default class VueI18n {
return false
}

if (this._warnHtmlInMessage === 'warn' || this._warnHtmlInMessage === 'error') {
Object.keys(messages).forEach(locale => {
this._checkLocaleMessage(locale, this._warnHtmlInMessage, messages[locale])
})
}

this._initVM({
locale,
fallbackLocale,
Expand All @@ -105,6 +115,41 @@ export default class VueI18n {
})
}

_checkLocaleMessage (locale: Locale, level: WarnHtmlInMessageLevel, message: LocaleMessageObject): void {
const stack: Array<string> = []

const fn = (message: mixed, stack: Array<string>) => {
if (isPlainObject(message)) {
Object.keys(message).forEach(key => {
const val = message[key]
const hasObject = isPlainObject(val)
stack.push(key)
hasObject && stack.push('.')
fn(val, stack)
hasObject && stack.pop()
})
} else if (Array.isArray(message)) {
message.forEach((item, index) => {
stack.push(`[${index}]`)
fn(item, stack)
stack.pop()
})
} else if (typeof message === 'string') {
const ret = htmlTagMatcher.test(message)
if (ret) {
const msg = `Detect unsafe locale message '${message}' of keypath '${stack.join('')}' at '${locale}', suggest use component interpolation with '<i18n>'`
Copy link
Collaborator

@exoego exoego Apr 24, 2019

Choose a reason for hiding this comment

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

I suggest to add followings to give more advidses to users.

  • What is a unsafe part of message: HTML
  • Why it is unsafe: it may cause vulnerability known as XSS (technical term)
  • How to resolve: URL to How to user component interpolation
Suggested change
const msg = `Detect unsafe locale message '${message}' of keypath '${stack.join('')}' at '${locale}', suggest use component interpolation with '<i18n>'`
const msg = `Detected HTML in message '${message}' of keypath '${stack.join('')}' at '${locale}'. Consider component interpolation with '<i18n>' to avoid XSS. See https://kazupon.github.io/vue-i18n/guide/interpolation.html`

URL could be shortend by https://bitly.com/

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks a lot!

if (level === 'warn') {
warn(msg)
} else if (level === 'error') {
error(msg)
}
}
}
}

fn(message, stack)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be helpful if error message suggest users to read Formatting and use component interpolation


_initVM (data: {
locale: Locale,
fallbackLocale: Locale,
Expand Down Expand Up @@ -184,6 +229,18 @@ export default class VueI18n {
get preserveDirectiveContent (): boolean { return this._preserveDirectiveContent }
set preserveDirectiveContent (preserve: boolean): void { this._preserveDirectiveContent = preserve }

get warnHtmlInMessage (): WarnHtmlInMessageLevel { return this._warnHtmlInMessage }
set warnHtmlInMessage (level: WarnHtmlInMessageLevel): void {
const orgLevel = this._warnHtmlInMessage
this._warnHtmlInMessage = level
if (orgLevel !== level && (level === 'warn' || level === 'error')) {
const messages = this._getMessages()
Object.keys(messages).forEach(locale => {
this._checkLocaleMessage(locale, this._warnHtmlInMessage, messages[locale])
})
}
}

_getMessages (): LocaleMessages { return this._vm.messages }
_getDateTimeFormats (): DateTimeFormats { return this._vm.dateTimeFormats }
_getNumberFormats (): NumberFormats { return this._vm.numberFormats }
Expand Down Expand Up @@ -499,10 +556,18 @@ export default class VueI18n {
}

setLocaleMessage (locale: Locale, message: LocaleMessageObject): void {
if (this._warnHtmlInMessage === 'warn' || this._warnHtmlInMessage === 'error') {
this._checkLocaleMessage(locale, this._warnHtmlInMessage, message)
if (this._warnHtmlInMessage === 'error') { return }
}
this._vm.$set(this._vm.messages, locale, message)
}

mergeLocaleMessage (locale: Locale, message: LocaleMessageObject): void {
if (this._warnHtmlInMessage === 'warn' || this._warnHtmlInMessage === 'error') {
this._checkLocaleMessage(locale, this._warnHtmlInMessage, message)
if (this._warnHtmlInMessage === 'error') { return }
}
this._vm.$set(this._vm.messages, locale, merge(this._vm.messages[locale] || {}, message))
}

Expand Down
10 changes: 10 additions & 0 deletions src/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,16 @@ export function warn (msg: string, err: ?Error): void {
}
}

export function error (msg: string, err: ?Error): void {
if (typeof console !== 'undefined') {
console.error('[vue-i18n] ' + msg)
/* istanbul ignore if */
if (err) {
console.error(err.stack)
}
}
}

export function isObject (obj: mixed): boolean %checks {
return obj !== null && typeof obj === 'object'
}
Expand Down
148 changes: 148 additions & 0 deletions test/unit/warn_html_in_message.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
describe('warnHtmlInMessage', () => {
let spyWarn
let spyError
beforeEach(() => {
spyWarn = sinon.spy(console, 'warn')
spyError = sinon.spy(console, 'error')
})
afterEach(() => {
spyWarn.restore()
spyError.restore()
})

describe('constructor option', () => {
it('should be worked', () => {
const messages = {
en: {
message: {
foo: {
buz: '<p>buz</p>'
},
bar: [1, '<p>bar</p>'],
buz: 22
}
},
ja: { message: '<p>こんにちは</p>' }
}

// `off`
new VueI18n({
warnHtmlInMessage: 'off',
messages
})
assert(spyWarn.callCount === 0)
assert(spyError.callCount === 0)

// `warn`
new VueI18n({
warnHtmlInMessage: 'warn',
messages
})
assert(spyWarn.callCount === 2)
assert(spyError.callCount === 0)

// `error`
new VueI18n({
warnHtmlInMessage: 'error',
messages
})
assert(spyWarn.callCount === 2)
assert(spyError.callCount === 2)
})
})

describe('property', () => {
it('should be worked', () => {
const messages = {
en: {
message: {
foo: {
buz: '<p>buz</p>'
},
bar: [1, '<p>bar</p>'],
buz: 22
}
},
ja: { message: '<p>こんにちは</p>' }
}

const i18n = new VueI18n({
warnHtmlInMessage: 'off',
messages
})

// `warn`
i18n.warnHtmlInMessage = 'warn'
assert(spyWarn.callCount === 2)
assert(spyError.callCount === 0)

// `error`
i18n.warnHtmlInMessage = 'error'
assert(spyWarn.callCount === 2)
assert(spyError.callCount === 2)

// `off`
i18n.warnHtmlInMessage = 'off'
assert(spyWarn.callCount === 2)
assert(spyError.callCount === 2)
})
})

describe('setLocaleMessage', () => {
it('should be worked', () => {
const i18n = new VueI18n({
warnHtmlInMessage: 'warn',
messages: {
en: {},
ja: {}
}
})

i18n.setLocaleMessage('en', {
hello: '<p>hello</p>'
})
assert(spyWarn.callCount === 1)
assert(spyError.callCount === 0)

i18n.warnHtmlInMessage = 'error'
i18n.setLocaleMessage('ja', {
hello: '<p>こんにちは</p>'
})
assert(spyWarn.callCount === 1)
assert(spyError.callCount === 1)

i18n.warnHtmlInMessage = 'off'
assert(spyWarn.callCount === 1)
assert(spyError.callCount === 1)
})
})

describe('mergeLocaleMessage', () => {
it('should be worked', () => {
const i18n = new VueI18n({
warnHtmlInMessage: 'warn',
messages: {
en: {},
ja: {}
}
})

i18n.mergeLocaleMessage('en', {
hello: '<p>hello</p>'
})
assert(spyWarn.callCount === 1)
assert(spyError.callCount === 0)

i18n.warnHtmlInMessage = 'error'
i18n.mergeLocaleMessage('ja', {
hello: '<p>こんにちは</p>'
})
assert(spyWarn.callCount === 1)
assert(spyError.callCount === 1)

i18n.warnHtmlInMessage = 'off'
assert(spyWarn.callCount === 1)
assert(spyError.callCount === 1)
})
})
})
6 changes: 6 additions & 0 deletions types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ declare namespace VueI18n {

type FormattedNumberPartType = 'currency' | 'decimal' | 'fraction' | 'group' | 'infinity' | 'integer' | 'literal' | 'minusSign' | 'nan' | 'plusSign' | 'percentSign';

type WarnHtmlInMessageLevel = 'off' | 'warn' | 'error';

interface FormattedNumberPart {
type: FormattedNumberPartType;
value: string;
Expand Down Expand Up @@ -103,6 +105,7 @@ declare namespace VueI18n {
silentFallbackWarn?: boolean;
preserveDirectiveContent?: boolean;
pluralizationRules?: PluralizationRulesMap;
warnHtmlInMessage?: WarnHtmlInMessageLevel;
}
}

Expand All @@ -124,6 +127,7 @@ export type NumberFormat = VueI18n.NumberFormat;
export type NumberFormats = VueI18n.NumberFormats;
export type NumberFormatResult = VueI18n.NumberFormatResult;
export type NumberFormatToPartsResult = VueI18n.NumberFormatToPartsResult;
export type WarnHtmlInMessageLevel = VueI18n.WarnHtmlInMessageLevel;
export type Formatter = VueI18n.Formatter;
export type MissingHandler = VueI18n.MissingHandler;
export type IntlAvailability = VueI18n.IntlAvailability;
Expand All @@ -142,6 +146,7 @@ export declare interface IVueI18n {
silentFallbackWarn: boolean;
preserveDirectiveContent: boolean;
pluralizationRules: VueI18n.PluralizationRulesMap;
warnHtmlInMessage: VueI18n.WarnHtmlInMessageLevel;
}

declare class VueI18n {
Expand All @@ -160,6 +165,7 @@ declare class VueI18n {
silentFallbackWarn: boolean;
preserveDirectiveContent: boolean;
pluralizationRules: VueI18n.PluralizationRulesMap;
warnHtmlInMessage: VueI18n.WarnHtmlInMessageLevel;

t(key: VueI18n.Path, values?: VueI18n.Values): VueI18n.TranslateResult;
t(key: VueI18n.Path, locale: VueI18n.Locale, values?: VueI18n.Values): VueI18n.TranslateResult;
Expand Down
Loading