From 5db9e906e85a414ae276e7de1033c1539aff8ea5 Mon Sep 17 00:00:00 2001
From: Haoliang Gao <sakura9515@gmail.com>
Date: Wed, 28 Mar 2018 22:38:39 +0800
Subject: [PATCH] fix: more safe context when locals is from query (#8)

---
 lib/assets_context.js                         | 21 +++++++++-
 package.json                                  | 11 +++---
 test/assets.test.js                           | 38 ++++++++++++++-----
 .../context-security/app/controller/home.js   | 13 +++++++
 .../apps/context-security/app/router.js       |  7 ++++
 .../apps/context-security/app/view/index.js   |  0
 .../context-security/config/config.default.js | 10 +++++
 .../context-security/config/manifest.json     |  4 ++
 .../apps/context-security/package.json        |  3 ++
 9 files changed, 91 insertions(+), 16 deletions(-)
 create mode 100644 test/fixtures/apps/context-security/app/controller/home.js
 create mode 100644 test/fixtures/apps/context-security/app/router.js
 create mode 100644 test/fixtures/apps/context-security/app/view/index.js
 create mode 100644 test/fixtures/apps/context-security/config/config.default.js
 create mode 100644 test/fixtures/apps/context-security/config/manifest.json
 create mode 100644 test/fixtures/apps/context-security/package.json

diff --git a/lib/assets_context.js b/lib/assets_context.js
index d0dec6f..9c1ff38 100644
--- a/lib/assets_context.js
+++ b/lib/assets_context.js
@@ -1,6 +1,9 @@
 'use strict';
 
 const assert = require('assert');
+const utility = require('utility');
+
+const CONTEXT_TEMPLATE_ID = 'context' + utility.sha1(String(Date.now()));
 
 class Assets {
   constructor(ctx) {
@@ -36,8 +39,10 @@ class Assets {
   }
 
   getContext(data) {
-    data = data || this.assetsContext || {};
-    return `<script>window.${this.config.contextKey} = ${JSON.stringify(data)};</script>`;
+    data = safeStringify(data || this.assetsContext);
+    let ret = `<div id="${CONTEXT_TEMPLATE_ID}" style="display:none">${data}</div>\n`;
+    ret += `<script>window.${this.config.contextKey} = JSON.parse(document.getElementById('${CONTEXT_TEMPLATE_ID}').textContent || '{}');</script>`;
+    return ret;
   }
 
   setEntry(entry) {
@@ -70,3 +75,15 @@ function linkTpl({ url }) {
 function scriptTpl({ url }) {
   return `<script src="${url}"></script>`;
 }
+
+const escapeMap = {
+  '<': '&lt;',
+  '>': '&gt;',
+};
+function safeStringify(data) {
+  if (!data) return '';
+  return JSON.stringify(data)
+    .replace(/[<>]/g, function(ch) {
+      return escapeMap[ch];
+    });
+}
diff --git a/package.json b/package.json
index 99d22cb..b0ed7e1 100644
--- a/package.json
+++ b/package.json
@@ -20,18 +20,19 @@
     "detect-port": "^1.2.2",
     "mz": "^2.7.0",
     "mz-modules": "^2.1.0",
-    "sdk-base": "^3.4.0"
+    "sdk-base": "^3.4.0",
+    "utility": "^1.13.1"
   },
   "devDependencies": {
     "autod": "^3.0.1",
     "autod-egg": "^1.1.0",
-    "egg": "^2.4.1",
-    "egg-bin": "^4.3.7",
+    "egg": "^2.5.0",
+    "egg-bin": "^4.5.0",
     "egg-ci": "^1.8.0",
-    "egg-mock": "^3.15.0",
+    "egg-mock": "^3.16.0",
     "egg-view-ejs": "^2.0.0",
     "egg-view-nunjucks": "^2.1.6",
-    "eslint": "^4.18.2",
+    "eslint": "^4.19.1",
     "eslint-config-egg": "^7.0.0",
     "supertest": "^3.0.0",
     "webstorm-disable-index": "^1.2.0"
diff --git a/test/assets.test.js b/test/assets.test.js
index c55196d..5a1ae20 100644
--- a/test/assets.test.js
+++ b/test/assets.test.js
@@ -28,7 +28,7 @@ describe('test/assets.test.js', () => {
           .get('/')
           .expect(/<div id="root"><\/div>/)
           .expect(/<link rel="stylesheet" href="http:\/\/127.0.0.1:8000\/index.css"><\/link>/)
-          .expect(/<script>window.context = {"data":1};<\/script>/)
+          .expect(/style="display:none">{"data":1}<\/div>/)
           .expect(/<script src="http:\/\/127.0.0.1:8000\/index.js"><\/script>/)
           .expect(/<script>window.__webpack_public_path__ = '\/app\/public';<\/script>/)
           .expect(200);
@@ -52,7 +52,7 @@ describe('test/assets.test.js', () => {
           .get('/')
           .expect(/<div id="root"><\/div>/)
           .expect(/<link rel="stylesheet" href="http:\/\/cdn.com\/app\/public\/index.b8e2efea.css"><\/link>/)
-          .expect(/<script>window.context = {"data":1};<\/script>/)
+          .expect(/style="display:none">{"data":1}<\/div>/)
           .expect(/<script src="http:\/\/cdn.com\/app\/public\/index.c4ae6394.js"><\/script>/)
           .expect(/<script>window.__webpack_public_path__ = '\/app\/public';<\/script>/)
           .expect(200);
@@ -78,7 +78,7 @@ describe('test/assets.test.js', () => {
           .get('/')
           .expect(/<div id="root"><\/div>/)
           .expect(/<link rel="stylesheet" href="http:\/\/127.0.0.1:8000\/index.css"><\/link>/)
-          .expect(/<script>window.context = {};<\/script>/)
+          .expect(/style="display:none"><\/div>/)
           .expect(/<script src="http:\/\/127.0.0.1:8000\/index.js"><\/script>/)
           .expect(200);
       });
@@ -86,7 +86,7 @@ describe('test/assets.test.js', () => {
       it('should render context', () => {
         return app.httpRequest()
           .get('/context')
-          .expect(/<script>window.context = {"data":1};<\/script>/)
+          .expect(/style="display:none">{"data":1}<\/div>/)
           .expect(200);
       });
 
@@ -95,7 +95,7 @@ describe('test/assets.test.js', () => {
           .get('/options')
           .expect(/<div id="root"><\/div>/)
           .expect(/<link rel="stylesheet" href="http:\/\/127.0.0.1:8000\/index.css"><\/link>/)
-          .expect(/<script>window.context = {};<\/script>/)
+          .expect(/style="display:none">{}<\/div>/)
           .expect(/<script src="http:\/\/127.0.0.1:8000\/index.js"><\/script>/)
           .expect(200);
       });
@@ -140,7 +140,7 @@ describe('test/assets.test.js', () => {
           .get('/')
           .expect(/<div id="root"><\/div>/)
           .expect(/<link rel="stylesheet" href="http:\/\/cdn.com\/index.b8e2efea.css"><\/link>/)
-          .expect(/<script>window.context = {};<\/script>/)
+          .expect(/style="display:none"><\/div>/)
           .expect(/<script src="http:\/\/cdn.com\/index.c4ae6394.js"><\/script>/)
           .expect(200);
       });
@@ -164,7 +164,7 @@ describe('test/assets.test.js', () => {
         return app.httpRequest()
           .get('/')
           .expect(/<link rel="stylesheet" href="http:\/\/127.0.0.1:8000\/index.css"><\/link>/)
-          .expect(/<script>window.context = {"data":1};<\/script>/)
+          .expect(/style="display:none">{"data":1}<\/div>/)
           .expect(/<script src="http:\/\/127.0.0.1:8000\/index.js"><\/script>/)
           .expect(/<script>window.__webpack_public_path__ = '\/app\/public';<\/script>/)
           .expect(/<script>window.resourceBaseUrl = 'http:\/\/127.0.0.1:8000\/';<\/script/)
@@ -186,7 +186,7 @@ describe('test/assets.test.js', () => {
         return app.httpRequest()
           .get('/')
           .expect(/<link rel="stylesheet" href="http:\/\/cdn.com\/app\/public\/index.b8e2efea.css"><\/link>/)
-          .expect(/<script>window.context = {"data":1};<\/script>/)
+          .expect(/style="display:none">{"data":1}<\/div>/)
           .expect(/<script src="http:\/\/cdn.com\/app\/public\/index.c4ae6394.js"><\/script>/)
           .expect(/<script>window.__webpack_public_path__ = '\/app\/public';<\/script>/)
           .expect(/<script>window.resourceBaseUrl = 'http:\/\/cdn.com\/app\/public\/';<\/script/)
@@ -230,7 +230,27 @@ describe('test/assets.test.js', () => {
     it('should GET /', () => {
       return app.httpRequest()
         .get('/')
-        .expect(/<script>window.__context__ = {};<\/script>/)
+        .expect(/<script>window.__context__ =/)
+        .expect(200);
+    });
+  });
+
+  describe('context security', () => {
+    let app;
+
+    before(() => {
+      app = mock.cluster({
+        baseDir: 'apps/context-security',
+      });
+      return app.ready();
+    });
+    after(() => app.close());
+
+    it('should GET /', () => {
+      return app.httpRequest()
+        .get('/?query=<x%E2%80%A8x>')
+        .expect(/<div id="[^"]+" style="display:none">\{"query":"&lt;x\u2028x&gt;"\}<\/div>/)
+        .expect(/window.context = JSON.parse\(document.getElementById\('[^']+'\).textContent \|\| '\{\}'\);/)
         .expect(200);
     });
   });
diff --git a/test/fixtures/apps/context-security/app/controller/home.js b/test/fixtures/apps/context-security/app/controller/home.js
new file mode 100644
index 0000000..06bb3b1
--- /dev/null
+++ b/test/fixtures/apps/context-security/app/controller/home.js
@@ -0,0 +1,13 @@
+'use strict';
+
+const Controller = require('egg').Controller;
+
+class HomeController extends Controller {
+  async index() {
+    await this.ctx.render('index.js', {
+      query: this.ctx.query.query,
+    });
+  }
+}
+
+module.exports = HomeController;
diff --git a/test/fixtures/apps/context-security/app/router.js b/test/fixtures/apps/context-security/app/router.js
new file mode 100644
index 0000000..4547a88
--- /dev/null
+++ b/test/fixtures/apps/context-security/app/router.js
@@ -0,0 +1,7 @@
+'use strict';
+
+module.exports = app => {
+  const { router, controller } = app;
+
+  router.get('/', controller.home.index);
+};
diff --git a/test/fixtures/apps/context-security/app/view/index.js b/test/fixtures/apps/context-security/app/view/index.js
new file mode 100644
index 0000000..e69de29
diff --git a/test/fixtures/apps/context-security/config/config.default.js b/test/fixtures/apps/context-security/config/config.default.js
new file mode 100644
index 0000000..395dcde
--- /dev/null
+++ b/test/fixtures/apps/context-security/config/config.default.js
@@ -0,0 +1,10 @@
+'use strict';
+
+exports.keys = '123456';
+exports.view = {
+  mapping: {
+    '.js': 'assets',
+  },
+};
+exports.assets = {
+};
diff --git a/test/fixtures/apps/context-security/config/manifest.json b/test/fixtures/apps/context-security/config/manifest.json
new file mode 100644
index 0000000..687d7be
--- /dev/null
+++ b/test/fixtures/apps/context-security/config/manifest.json
@@ -0,0 +1,4 @@
+{
+  "index.css": "index.css",
+  "index.js": "index.js"
+}
diff --git a/test/fixtures/apps/context-security/package.json b/test/fixtures/apps/context-security/package.json
new file mode 100644
index 0000000..13148e0
--- /dev/null
+++ b/test/fixtures/apps/context-security/package.json
@@ -0,0 +1,3 @@
+{
+  "name": "egg-view-assets"
+}