From 0fb64a6fea966845f419a639a8242812bfe5a2df Mon Sep 17 00:00:00 2001 From: Drew Logsdon and Javier Pedemonte Date: Thu, 7 Jan 2016 16:48:29 -0600 Subject: [PATCH] [Issue 6] Filter out execute msgs (from client) with actual code Also added tests. (c) Copyright IBM Corp. 2016 --- package.json | 8 +- public/js/kernel.js | 1 + routes/api.js | 25 +++++- test/routes/api-test.js | 173 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 202 insertions(+), 5 deletions(-) create mode 100644 test/routes/api-test.js diff --git a/package.json b/package.json index b3ff734..cc1acbf 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,8 @@ "version": "0.0.1", "private": true, "scripts": { - "start": "node ./bin/www" + "start": "node ./bin/www", + "test": "node_modules/mocha/bin/mocha test/**/*-test.js" }, "dependencies": { "body-parser": "^1.13.3", @@ -26,6 +27,7 @@ }, "devDependencies": { "browserify": "^12.0.1", + "chai": "^3.4.1", "gulp": "^3.9.0", "gulp-less": "^3.0.1", "gulp-livereload": "^3.8.0", @@ -33,7 +35,11 @@ "gulp-open": "^1.0.0", "gulp-plumber": "^1.0.0", "merge-stream": "^1.0.0", + "mocha": "^2.3.4", "node-lessify": "^0.1.1", + "proxyquire": "^1.7.3", + "sinon": "^1.17.2", + "sinon-chai": "^2.8.0", "vinyl-source-stream": "^1.1.0" } } diff --git a/public/js/kernel.js b/public/js/kernel.js index c7c4da7..f4ad125 100644 --- a/public/js/kernel.js +++ b/public/js/kernel.js @@ -86,6 +86,7 @@ define([ resultHandler(msg); } }; + return future; // TODO error handling } diff --git a/routes/api.js b/routes/api.js index 3fa6fdc..c398e44 100644 --- a/routes/api.js +++ b/routes/api.js @@ -38,7 +38,6 @@ function setupWSProxy(_server) { // substitute in code if necessary // for performance reasons, first do a quick string check before JSON parsing - // TODO filter out messages that contain actual code if (d && d.payload.indexOf('execute_request') !== -1) { try { d.payload = JSON.parse(d.payload); @@ -48,10 +47,22 @@ function setupWSProxy(_server) { transformedData = nbstore.get(nbpath).then( function success(nb) { // get code string for cell at index and update WS message + + // code must be an integer corresponding to a cell index var cellIdx = parseInt(d.payload.content.code, 10); - var code = nb.cells[cellIdx].source.join(''); - d.payload.content.code = code; - d.payload = JSON.stringify(d.payload); + + // code must only consist of a non-negative integer + if (cellIdx.toString(10) === d.payload.content.code && + cellIdx >= 0) { + + // substitute cell's actual code into the message + var code = nb.cells[cellIdx].source.join(''); + d.payload.content.code = code; + d.payload = JSON.stringify(d.payload); + } else { + // throw away execute request that has non-integer code + d = null; + } return d; }, function error() { @@ -67,6 +78,12 @@ function setupWSProxy(_server) { }); } Promise.all(codeCellsSubstituted).then(function(data) { + // data is an array of messages + // filter out null messages (if any) + data = data.filter(function(d) { + return !!d; + }); + // re-encode data = wsutils.encodeWebSocket(data); _emit.call(socket, eventName, data); }); diff --git a/test/routes/api-test.js b/test/routes/api-test.js new file mode 100644 index 0000000..dd03527 --- /dev/null +++ b/test/routes/api-test.js @@ -0,0 +1,173 @@ +/** + * Copyright (c) Jupyter Development Team. + * Distributed under the terms of the Modified BSD License. + */ +var sinon = require('sinon'); +var chai = require('chai').use(require('sinon-chai')); +var expect = chai.expect; +var proxyquire = require('proxyquire'); +var Promise = require('es6-promise').Promise; + +/////////////////// +// MODULE STUBS +/////////////////// + +var httpProxyStub = { + createProxyServer: function() { + return { + on: function() {}, + web: function() {}, + ws: function() {} + }; + } +}; + +var wsutilsStub = { + decodeWebSocket: function(data) { + return data; + }, + + encodeWebSocket: function(data) { + return data; + } +}; + +var notebookData = { + cells: [ + { + source: [ + 'line 1;', + 'line 2;' + ] + }, + { + source: [ + 'line 3;', + 'line 4;' + ] + } + ] +}; + +var nbstoreStub = { + get: function() { + return new Promise(function(resolve, reject) { + resolve(notebookData); + }); + } +}; + +var api = proxyquire('../../routes/api', { + 'http-proxy': httpProxyStub, + '../app/ws-utils': wsutilsStub, + '../app/notebook-store': nbstoreStub +}); + + +////////// +// TESTS +////////// + +describe('routes: api', function() { + var serverUpgradeCallback = null; + var req = { + connection: { + server: { + on: function(topic, callback) { + if (topic === 'upgrade') { + serverUpgradeCallback = callback; + } + } + } + }, + url: '/api/kernel' + }; + var emitSpy = sinon.spy(); + var socket = { + emit: emitSpy + }; + + before(function() { + // initialize state of websocket handling + api(req, {}, null); + expect(serverUpgradeCallback).to.not.be.null; + serverUpgradeCallback(req, socket, null); + }); + + beforeEach(function() { + emitSpy.reset(); + }); + + it('should allow execute request with integer', function(done) { + var payload = { + "header": { + "session": "12345", + "msg_type": "execute_request" + }, + "content": { + "code": "0", + } + }; + var data = [ + { + payload: JSON.stringify(payload) + } + ]; + socket.emit('data', data); + + setTimeout(function() { + expect(emitSpy).calledOnce; + var emittedData = emitSpy.firstCall.args[1]; + expect(emittedData).to.have.length(1); + expect(emittedData[0]).to.include.keys('payload'); + expect(emittedData[0].payload).to.contain('line 1;line 2;'); + done(); + }, 0); + }); + + it('should filter execute request with code', function(done) { + // should be passed along + var payload1 = { + "header": { + "session": "12345", + "msg_type": "execute_request" + }, + "content": { + "code": "0", + } + }; + // tests that code is filtered + var payload2 = { + "header": { + "session": "12345", + "msg_type": "execute_request" + }, + "content": { + "code": "foo = 1; print(foo)", + } + }; + // tests that code starting with integer is also filtered + var payload3 = { + "header": { + "session": "12345", + "msg_type": "execute_request" + }, + "content": { + "code": "456; foo = 1; print(foo)", + } + }; + var data = [ + { payload: JSON.stringify(payload1) }, + { payload: JSON.stringify(payload2) }, + { payload: JSON.stringify(payload3) } + ]; + socket.emit('data', data); + + setTimeout(function() { + expect(emitSpy).calledOnce; + var emittedData = emitSpy.firstCall.args[1]; + expect(emittedData).to.have.length(1); + done(); + }, 0); + }); +});