breaking: HTML message warning (#29)

* breaking: HTML message warning

* update docs

Author: kazupon

README.md

@@ -86,6 +86,10 @@ The examples are offered that use the following two API styles:
 - `VueI18n.prototype.getChoiceIndex`
   - -> Legacy API style: `pluralizationRules` option of `createI18n` factory function (like `new VueI18n(...)`)
   - -> Compsable API style: `pluralRules` option of `createI18nComposer` factory function
+- `warnHtmlInMessage` option:
+  - Composable API: `warnHtmlMessage` boolean option, default `true`.
+  - For development mode, warning is default.
+  - For production mode, HTML message detect is not check due to performance.
 - `VueI18n.version` -> `import { VERSION } from 'vue-i18n'`
 - `VueI18n.availabilities` -> `import { availabilities } from 'vue-i18n'`
 - See the details [here](https://github.com/intlify/vue-i18n-next/blob/master/docs/vue-i18n.md)
@@ -137,15 +141,15 @@ yarn add vue-i18n@next
 - Intlify message format compiler
   - [x] vue-i18n message format
   - [ ] sourcemap
-  - [ ] HTML format handling
+  - [x] HTML format handling
   - [x] error handling
   - [ ] more unit (fuzzing) tests
   - [ ] performance tests (benchmark)
 - Intlify core runtime
   - [x] translate function
   - [x] datetime function
   - [x] number function
-  - [ ] warnHtmlInMessage
+  - [x] warnHtmlMessage
   - [x] improve translate `args` typing
   - [ ] improve locale messages typing: `LocaleMessages` / `LocaleMessage` / `LocaleMessageDictiory`
   - [x] postTranslation context option
@@ -163,6 +167,7 @@ yarn add vue-i18n@next
     - [x] fallbackFormat
     - [x] dateTimeFormats
     - [x] numberFormats
+    - [x] warnHtmlMessage
   - methods
     - [x] t
     - [x] getLocaleMessages
@@ -195,7 +200,7 @@ yarn add vue-i18n@next
     - [x] silentFallbackWarn
     - [x] formatFallbackMessages
     - [ ] preserveDirectiveContent
-    - [ ] warnHtmlInMessage
+    - [x] warnHtmlInMessage
     - [x] postTranslation
     - [x] t
     - [x] tc

src/composer.ts

@@ -96,6 +96,7 @@ export type ComposerOptions = {
   fallbackRoot?: boolean
   fallbackFormat?: boolean
   postTranslation?: PostTranslationHandler
+  warnHtmlMessage?: boolean
   __i18n?: CustomBlocks // for custom blocks, and internal
   __root?: Composer // for internal
 }
@@ -119,6 +120,7 @@ export type Composer = {
   fallbackWarn: boolean | RegExp
   fallbackRoot: boolean
   fallbackFormat: boolean
+  warnHtmlMessage: boolean
   __id: number // for internal
 
   /**
@@ -302,6 +304,10 @@ export function createComposer(options: ComposerOptions = {}): Composer {
     ? options.postTranslation
     : null
 
+  let _warnHtmlMessage = isBoolean(options.warnHtmlMessage)
+    ? options.warnHtmlMessage
+    : true
+
   // custom linked modifiers
   // prettier-ignore
   const _modifiers = __root
@@ -330,6 +336,7 @@ export function createComposer(options: ComposerOptions = {}): Composer {
       fallbackFormat: _fallbackFormat,
       unresolving: true,
       postTranslation: _postTranslation === null ? undefined : _postTranslation,
+      warnHtmlMessage: _warnHtmlMessage,
       _datetimeFormatters: isPlainObject(_context)
         ? _context._datetimeFormatters
         : undefined,
@@ -645,6 +652,13 @@ export function createComposer(options: ComposerOptions = {}): Composer {
       _fallbackFormat = val
       _context.fallbackFormat = _fallbackFormat
     },
+    get warnHtmlMessage(): boolean {
+      return _warnHtmlMessage
+    },
+    set warnHtmlMessage(val: boolean) {
+      _warnHtmlMessage = val
+      _context.warnHtmlMessage = val
+    },
     __id: composerID,
 
     /**

src/legacy.ts

@@ -110,11 +110,11 @@ export type VueI18n = {
   silentFallbackWarn: boolean | RegExp
   formatFallbackMessages: boolean
   sync: boolean
+  warnHtmlInMessage: WarnHtmlInMessageLevel
   __id: number
   __composer: Composer
   /*
   preserveDirectiveContent: boolean
-  warnHtmlInMessage: WarnHtmlInMessageLevel
   */
 
   /**
@@ -192,6 +192,9 @@ function convertComposerOptions(options: VueI18nOptions): ComposerOptions {
   const postTranslation = isFunction(options.postTranslation)
     ? options.postTranslation
     : undefined
+  const warnHtmlMessage = isString(options.warnHtmlInMessage)
+    ? options.warnHtmlInMessage !== 'off'
+    : true
 
   if (__DEV__ && options.formatter) {
     warn(`not supportted 'formatter' option`)
@@ -225,6 +228,7 @@ function convertComposerOptions(options: VueI18nOptions): ComposerOptions {
     fallbackFormat,
     pluralRules: pluralizationRules,
     postTranslation,
+    warnHtmlMessage,
     __i18n,
     __root
   }
@@ -345,6 +349,14 @@ export function createVueI18n(options: VueI18nOptions = {}): VueI18n {
       options.sync = val
     },
 
+    // warnInHtmlMessage
+    get warnHtmlInMessage(): WarnHtmlInMessageLevel {
+      return composer.warnHtmlMessage ? 'warn' : 'off'
+    },
+    set warnHtmlInMessage(val: WarnHtmlInMessageLevel) {
+      composer.warnHtmlMessage = val !== 'off'
+    },
+
     // for internal
     __id: composer.__id,
     __composer: composer,

src/message/compiler.ts

@@ -3,6 +3,7 @@ import { createParser, ResourceNode } from './parser'
 import { transform } from './transformer'
 import { generate } from './generator'
 import { CompileError, defaultOnError } from './errors'
+import { warn, isBoolean } from '../utils'
 
 export type CompileResult = {
   code: string
@@ -18,22 +19,38 @@ export type Compiler = Readonly<{
 
 export type MessageFunction = (ctx: unknown) => unknown
 
+// TODO: This code should be removed with using rollup (`/*#__PURE__*/`)
+const RE_HTML_TAG = /<\/?[\w\s="/.':;#-\/]+>/
+function checkHtmlMessage(source: string, options: CompileOptions): void {
+  const warnHtmlMessage = isBoolean(options.warnHtmlMessage)
+    ? options.warnHtmlMessage
+    : true
+  if (warnHtmlMessage && RE_HTML_TAG.test(source)) {
+    warn(
+      `Detected HTML in '${source}' message. Recommend not using HTML messages to avoid XSS.`
+    )
+  }
+}
+
 const defaultOnCacheKey = (source: string): string => source
 const compileCache: Record<string, MessageFunction> = Object.create(null)
 
 export function compile(
   source: string,
   options: CompileOptions = {}
 ): MessageFunction {
-  const onCacheKey = options.onCacheKey || defaultOnCacheKey
+  // check HTML message
+  __DEV__ && checkHtmlMessage(source, options)
 
   // check caches
+  const onCacheKey = options.onCacheKey || defaultOnCacheKey
   const key = onCacheKey(source)
   const cached = compileCache[key]
   if (cached) {
     return cached
   }
 
+  // compile error detecting
   let occured = false
   const onError = options.onError || defaultOnError
   options.onError = (err: CompileError): void => {

src/message/options.ts

@@ -34,6 +34,7 @@ export type CodeGenOptions = {
 }
 
 export type CompileOptions = {
+  warnHtmlMessage?: boolean
   onCacheKey?: CompileCacheKeyHandler
 } & TransformOptions &
   CodeGenOptions &

src/runtime/context.ts

@@ -61,6 +61,7 @@ export type RuntimeOptions = {
   unresolving?: boolean
   postTranslation?: PostTranslationHandler
   processor?: MessageProcessor
+  warnHtmlMessage?: boolean
   _datetimeFormatters?: Map<string, Intl.DateTimeFormat>
   _numberFormatters?: Map<string, Intl.NumberFormat>
 }
@@ -80,6 +81,7 @@ export type RuntimeContext = {
   unresolving: boolean
   postTranslation: PostTranslationHandler | null
   processor: MessageProcessor | null
+  warnHtmlMessage: boolean
   _datetimeFormatters: Map<string, Intl.DateTimeFormat>
   _numberFormatters: Map<string, Intl.NumberFormat>
   _fallbackLocaleStack?: Locale[]
@@ -141,6 +143,9 @@ export function createRuntimeContext(
     ? options.postTranslation
     : null
   const processor = isPlainObject(options.processor) ? options.processor : null
+  const warnHtmlMessage = isBoolean(options.warnHtmlMessage)
+    ? options.warnHtmlMessage
+    : true
   const _datetimeFormatters = isObject(options._datetimeFormatters)
     ? options._datetimeFormatters
     : new Map<string, Intl.DateTimeFormat>()
@@ -163,6 +168,7 @@ export function createRuntimeContext(
     unresolving,
     postTranslation,
     processor,
+    warnHtmlMessage,
     _datetimeFormatters,
     _numberFormatters
   }

src/runtime/translate.ts

@@ -165,7 +165,8 @@ export function translate(
     fallbackFormat,
     postTranslation,
     unresolving,
-    fallbackLocale
+    fallbackLocale,
+    warnHtmlMessage
   } = context
   const [key, options] = parseTranslateArgs(...args)
 
@@ -240,7 +241,13 @@ export function translate(
     ? format
     : compile(
         format,
-        getCompileOptions(targetLocale, cacheBaseKey, format, errorDetector)
+        getCompileOptions(
+          targetLocale,
+          cacheBaseKey,
+          format,
+          warnHtmlMessage,
+          errorDetector
+        )
       )
 
   // if occured compile error, return the message format
@@ -298,9 +305,11 @@ function getCompileOptions(
   locale: Locale,
   key: string,
   source: string,
+  warnHtmlMessage: boolean,
   errorDetector?: Function
 ): CompileOptions {
   return {
+    warnHtmlMessage,
     onError: (err: CompileError): void => {
       errorDetector && errorDetector(err)
       if (__DEV__) {
@@ -338,7 +347,13 @@ function getMessageContextOptions(
       }
       const msg = compile(
         val,
-        getCompileOptions(locale, key, val, errorDetector)
+        getCompileOptions(
+          locale,
+          key,
+          val,
+          context.warnHtmlMessage,
+          errorDetector
+        )
       )
       return !occured ? msg : NOOP_MESSAGE_FUNCTION
     } else if (isMessageFunction(val)) {

test/legacy.test.ts

@@ -415,3 +415,26 @@ test('getChoiceIndex', () => {
     `not supportted 'getChoiceIndex' method.`
   )
 })
+
+test('warnHtmlInMessage', () => {
+  const mockWarn = warn as jest.MockedFunction<typeof warn>
+  mockWarn.mockImplementation(() => {}) // eslint-disable-line @typescript-eslint/no-empty-function
+
+  const i18n = createVueI18n({
+    locale: 'en',
+    messages: {
+      en: {
+        hello: '<p>hello</p>'
+      }
+    }
+  })
+
+  expect(i18n.t('hello')).toEqual('<p>hello</p>')
+
+  i18n.warnHtmlInMessage = 'off'
+  expect(i18n.t('hello')).toEqual('<p>hello</p>')
+
+  i18n.warnHtmlInMessage = 'error'
+  expect(i18n.t('hello')).toEqual('<p>hello</p>')
+  expect(mockWarn).toHaveBeenCalledTimes(2)
+})

test/message/__snapshots__/compiler.test.ts.snap

@@ -49,3 +49,19 @@ Object {
   "message": "Plural must have messages",
 }
 `;
+
+exports[`warnHtmlMessage default: code 1`] = `
+"function __msg__ (ctx) {
+  return ctx.normalize([
+    \\"<p>hello</p>\\"
+  ])
+}"
+`;
+
+exports[`warnHtmlMessage false: code 1`] = `
+"function __msg__ (ctx) {
+  return ctx.normalize([
+    \\"<p>hello</p>\\"
+  ])
+}"
+`;

test/message/compiler.test.ts

@@ -1,3 +1,12 @@
+/* eslint-disable @typescript-eslint/no-empty-function */
+
+// utils
+jest.mock('../../src/utils', () => ({
+  ...jest.requireActual('../../src/utils'),
+  warn: jest.fn()
+}))
+import { warn } from '../../src/utils'
+
 import { compile } from '../../src/message/compiler'
 
 /* eslint-disable no-irregular-whitespace */
@@ -7,6 +16,29 @@ test(`@.caml:{'no apples'} | {0} apple | {n}　apples`, () => {
 })
 /* eslint-enable no-irregular-whitespace */
 
+describe('warnHtmlMessage', () => {
+  test('default', () => {
+    const mockWarn = warn as jest.MockedFunction<typeof warn>
+    mockWarn.mockImplementation(() => {})
+
+    const code = compile('<p>hello</p>')
+    expect(code.toString()).toMatchSnapshot('code')
+    expect(mockWarn).toHaveBeenCalled()
+    expect(mockWarn.mock.calls[0][0]).toEqual(
+      `Detected HTML in '<p>hello</p>' message. Recommend not using HTML messages to avoid XSS.`
+    )
+  })
+
+  test('false', () => {
+    const mockWarn = warn as jest.MockedFunction<typeof warn>
+    mockWarn.mockImplementation(() => {})
+
+    const code = compile('<p>hello</p>', { warnHtmlMessage: false })
+    expect(code.toString()).toMatchSnapshot('code')
+    expect(mockWarn).not.toHaveBeenCalled()
+  })
+})
+
 describe('edge cases', () => {
   test(` | | | `, () => {
     const code = compile(` | | | `, {
@@ -17,3 +49,5 @@ describe('edge cases', () => {
     expect(code.toString()).toMatchSnapshot('code')
   })
 })
+
+/* eslint-enable @typescript-eslint/no-empty-function */