Skip to content
This repository has been archived by the owner on Oct 26, 2019. It is now read-only.

Commit

Permalink
[Issue 6] Filter out execute msgs (from client) with actual code
Browse files Browse the repository at this point in the history
Also added tests.

(c) Copyright IBM Corp. 2016
  • Loading branch information
jhpedemonte committed Jan 7, 2016
1 parent 20ef64d commit 0fb64a6
Show file tree
Hide file tree
Showing 4 changed files with 202 additions and 5 deletions.
8 changes: 7 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -26,14 +27,19 @@
},
"devDependencies": {
"browserify": "^12.0.1",
"chai": "^3.4.1",
"gulp": "^3.9.0",
"gulp-less": "^3.0.1",
"gulp-livereload": "^3.8.0",
"gulp-nodemon": "^2.0.2",
"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"
}
}
1 change: 1 addition & 0 deletions public/js/kernel.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ define([
resultHandler(msg);
}
};
return future;
// TODO error handling
}

Expand Down
25 changes: 21 additions & 4 deletions routes/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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() {
Expand All @@ -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);
});
Expand Down
173 changes: 173 additions & 0 deletions test/routes/api-test.js
Original file line number Diff line number Diff line change
@@ -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);
});
});

0 comments on commit 0fb64a6

Please sign in to comment.