From 120df0bfa2b28c8d1c7c6bdeaed258d1b3f402b1 Mon Sep 17 00:00:00 2001 From: Miguel Serrano Date: Tue, 27 Oct 2020 11:43:51 +0100 Subject: [PATCH] Merge pull request #3314 from overleaf/jpa-i18n-safe-html-substitute [misc] i18n: safe html substitute GitOrigin-RevId: be74605d24084b419324509a403933cf71ed1c8a --- .../Features/Helpers/SafeHTMLSubstitution.js | 46 ++++++ .../app/src/infrastructure/ExpressLocals.js | 10 +- .../HelperFiles/SafeHTMLSubstituteTests.js | 156 ++++++++++++++++++ 3 files changed, 210 insertions(+), 2 deletions(-) create mode 100644 services/web/app/src/Features/Helpers/SafeHTMLSubstitution.js create mode 100644 services/web/test/unit/src/HelperFiles/SafeHTMLSubstituteTests.js diff --git a/services/web/app/src/Features/Helpers/SafeHTMLSubstitution.js b/services/web/app/src/Features/Helpers/SafeHTMLSubstitution.js new file mode 100644 index 0000000000..0ff84473f9 --- /dev/null +++ b/services/web/app/src/Features/Helpers/SafeHTMLSubstitution.js @@ -0,0 +1,46 @@ +const pug = require('pug-runtime') + +const SPLIT_REGEX = /<(\d+)>(.*?)<\/\1>/g + +function render(locale, components) { + const output = [] + function addPlainText(text) { + if (!text) return + output.push(pug.escape(text)) + } + + // 'PRE<0>INNERPOST' -> ['PRE', '0', 'INNER', 'POST'] + // '<0>INNER' -> ['', '0', 'INNER', ''] + // '<0>' -> ['', '0', '', ''] + // '<0>INNER<0>INNER2' -> ['', '0', 'INNER', '', '0', 'INNER2', ''] + // '<0><1>INNER' -> ['', '0', '<1>INNER', ''] + // 'PLAIN TEXT' -> ['PLAIN TEXT'] + // NOTE: a test suite is verifying these cases: SafeHTMLSubstituteTests + const chunks = locale.split(SPLIT_REGEX) + + // extract the 'PRE' chunk + addPlainText(chunks.shift()) + + while (chunks.length) { + // each batch consists of three chunks: ['0', 'INNER', 'POST'] + const [idx, innerChunk, intermediateChunk] = chunks.splice(0, 3) + + const component = components[idx] + const componentName = + typeof component === 'string' ? component : component.name + // pug is doing any necessary escaping on attribute values + const attributes = (component.attrs && pug.attrs(component.attrs)) || '' + output.push( + `<${componentName + attributes}>`, + ...render(innerChunk, components), + `` + ) + addPlainText(intermediateChunk) + } + return output.join('') +} + +module.exports = { + SPLIT_REGEX, + render +} diff --git a/services/web/app/src/infrastructure/ExpressLocals.js b/services/web/app/src/infrastructure/ExpressLocals.js index eb3a5911dc..cb33cf9e34 100644 --- a/services/web/app/src/infrastructure/ExpressLocals.js +++ b/services/web/app/src/infrastructure/ExpressLocals.js @@ -12,6 +12,7 @@ const Features = require('./Features') const AuthenticationController = require('../Features/Authentication/AuthenticationController') const PackageVersions = require('./PackageVersions') const Modules = require('./Modules') +const SafeHTMLSubstitute = require('../Features/Helpers/SafeHTMLSubstitution') let webpackManifest if (!IS_DEV_ENV) { @@ -176,10 +177,15 @@ module.exports = function(webRouter, privateApiRouter, publicApiRouter) { }) webRouter.use(function(req, res, next) { - res.locals.translate = function(key, vars) { + res.locals.translate = function(key, vars, components) { vars = vars || {} vars.appName = Settings.appName - return req.i18n.translate(key, vars) + const locale = req.i18n.translate(key, vars) + if (components) { + return SafeHTMLSubstitute.render(locale, components) + } else { + return locale + } } // Don't include the query string parameters, otherwise Google // treats ?nocdn=true as the canonical version diff --git a/services/web/test/unit/src/HelperFiles/SafeHTMLSubstituteTests.js b/services/web/test/unit/src/HelperFiles/SafeHTMLSubstituteTests.js new file mode 100644 index 0000000000..87d8d0aff8 --- /dev/null +++ b/services/web/test/unit/src/HelperFiles/SafeHTMLSubstituteTests.js @@ -0,0 +1,156 @@ +const { expect } = require('chai') +const SandboxedModule = require('sandboxed-module') +const MODULE_PATH = require('path').join( + __dirname, + '../../../../app/src/Features/Helpers/SafeHTMLSubstitution.js' +) + +describe('SafeHTMLSubstitution', function() { + let SafeHTMLSubstitution + before(function() { + SafeHTMLSubstitution = SandboxedModule.require(MODULE_PATH) + }) + + describe('SPLIT_REGEX', function() { + const CASES = { + 'PRE<0>INNERPOST': ['PRE', '0', 'INNER', 'POST'], + '<0>INNER': ['', '0', 'INNER', ''], + '<0>': ['', '0', '', ''], + '<0>INNER<0>INNER2': ['', '0', 'INNER', '', '0', 'INNER2', ''], + '<0><1>INNER': ['', '0', '<1>INNER', ''], + 'PLAIN TEXT': ['PLAIN TEXT'] + } + Object.entries(CASES).forEach(([input, output]) => { + it(`should parse "${input}" as expected`, function() { + expect(input.split(SafeHTMLSubstitution.SPLIT_REGEX)).to.deep.equal( + output + ) + }) + }) + }) + + describe('render', function() { + describe('substitution', function() { + it('should substitute a single component', function() { + expect( + SafeHTMLSubstitution.render('<0>good', [{ name: 'b' }]) + ).to.equal('good') + }) + + it('should substitute a single component as string', function() { + expect(SafeHTMLSubstitution.render('<0>good', ['b'])).to.equal( + 'good' + ) + }) + + it('should substitute a single component twice', function() { + expect( + SafeHTMLSubstitution.render('<0>one<0>two', [{ name: 'b' }]) + ).to.equal('onetwo') + }) + + it('should substitute two components', function() { + expect( + SafeHTMLSubstitution.render('<0>one<1>two', [ + { name: 'b' }, + { name: 'i' } + ]) + ).to.equal('onetwo') + }) + + it('should substitute a single component with a class', function() { + expect( + SafeHTMLSubstitution.render('<0>text', [ + { + name: 'b', + attrs: { + class: 'magic' + } + } + ]) + ).to.equal('text') + }) + + it('should substitute two nested components', function() { + expect( + SafeHTMLSubstitution.render('<0><1>nested', [ + { name: 'b' }, + { name: 'i' } + ]) + ).to.equal('nested') + }) + + it('should handle links', function() { + expect( + SafeHTMLSubstitution.render('<0>Go to Login', [ + { name: 'a', attrs: { href: 'https://www.overleaf.com/login' } } + ]) + ).to.equal('Go to Login') + }) + + it('should not complain about too many components', function() { + expect( + SafeHTMLSubstitution.render('<0>good', [ + { name: 'b' }, + { name: 'i' }, + { name: 'u' } + ]) + ).to.equal('good') + }) + }) + + describe('pug.escape', function() { + it('should handle plain text', function() { + expect(SafeHTMLSubstitution.render('plain text')).to.equal('plain text') + }) + + it('should keep a simple string delimiter', function() { + expect(SafeHTMLSubstitution.render("'")).to.equal(`'`) + }) + + it('should escape double quotes', function() { + expect(SafeHTMLSubstitution.render('"')).to.equal(`"`) + }) + + it('should escape &', function() { + expect(SafeHTMLSubstitution.render('&')).to.equal(`&`) + }) + + it('should escape <', function() { + expect(SafeHTMLSubstitution.render('<')).to.equal(`<`) + }) + + it('should escape >', function() { + expect(SafeHTMLSubstitution.render('>')).to.equal(`>`) + }) + + it('should escape html', function() { + expect(SafeHTMLSubstitution.render('bad')).to.equal( + '<b>bad</b>' + ) + }) + }) + + describe('escape around substitutions', function() { + it('should escape text inside a component', function() { + expect( + SafeHTMLSubstitution.render('<0>inner', [{ name: 'b' }]) + ).to.equal('<i>inner</i>') + }) + + it('should escape text in front of a component', function() { + expect( + SafeHTMLSubstitution.render('PRE<0>inner', [{ name: 'b' }]) + ).to.equal('<i>PRE</i>inner') + }) + + it('should escape text after of a component', function() { + expect( + SafeHTMLSubstitution.render('<0>innerPOST', [ + { name: 'b' } + ]) + ).to.equal('inner<i>POST</i>') + }) + }) + }) +})