Skip to content

Commit

Permalink
fix: more safe context when locals is from query (#8)
Browse files Browse the repository at this point in the history
  • Loading branch information
@popomore
popomore committed Mar 28, 2018
1 parent c71671c commit 5db9e90
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 16 deletions.
21 changes: 19 additions & 2 deletions lib/assets_context.js
View file
@@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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];
});
}
11 changes: 6 additions & 5 deletions package.json
View file
Expand Up @@ -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"
Expand Down
38 changes: 29 additions & 9 deletions test/assets.test.js
View file
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -78,15 +78,15 @@ 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);
});

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);
});

Expand All @@ -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);
});
Expand Down Expand Up @@ -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);
});
Expand All @@ -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/)
Expand All @@ -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/)
Expand Down Expand Up @@ -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);
});
});
Expand Down
13 changes: 13 additions & 0 deletions test/fixtures/apps/context-security/app/controller/home.js
View file
@@ -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;
7 changes: 7 additions & 0 deletions test/fixtures/apps/context-security/app/router.js
View file
@@ -0,0 +1,7 @@
'use strict';

module.exports = app => {
const { router, controller } = app;

router.get('/', controller.home.index);
};
Empty file added test/fixtures/apps/context-security/app/view/index.js
View file
Empty file.
10 changes: 10 additions & 0 deletions test/fixtures/apps/context-security/config/config.default.js
View file
@@ -0,0 +1,10 @@
'use strict';

exports.keys = '123456';
exports.view = {
mapping: {
'.js': 'assets',
},
};
exports.assets = {
};
4 changes: 4 additions & 0 deletions test/fixtures/apps/context-security/config/manifest.json
View file
@@ -0,0 +1,4 @@
{
"index.css": "index.css",
"index.js": "index.js"
}
3 changes: 3 additions & 0 deletions test/fixtures/apps/context-security/package.json
View file
@@ -0,0 +1,3 @@
{
"name": "egg-view-assets"
}

0 comments on commit 5db9e90

Please sign in to comment.