lzh/noVNC
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Abstract RFB errors to avoid sending strings

Browse Source 
The API allowed strings to be passed from the RFB module to the
application using the disconnect reason. This caused problems since
the application didn't have control over translations for these
strings.

Most of the information being passed using this string was very
technical and not helpful to the end user. One exception to this was
the security result information regarding for example authentication
failures. The protocol allows the VNC server to pass a string
directly to the user in the security result.

So the disconnect reason is replaced by a boolean saying if the
disconnection was clean or not. And for the security result information
from the server, a new event has been added.
This commit is contained in:
Samuel Mannehed
2017-11-11 01:48:47 +01:00
parent ee5cae9fee
commit d472f3f19e
7 changed files with 232 additions and 133 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/playback-ui.js
+3 -3
View File
@@ -115,13 +115,13 @@ IterationPlayer.prototype = {
this._nextIteration();
},
_disconnected: function (rfb, reason, frame) {
if (reason) {
_disconnected: function (rfb, clean, frame) {
if (!clean) {
this._state = 'failed';
}
var evt = new Event('rfbdisconnected');
evt.reason = reason;
evt.clean = clean;
evt.frame = frame;
this.onrfbdisconnected(evt);
tests/playback.js
+4 -3
View File
@@ -78,7 +78,8 @@ RecordingPlayer.prototype = {
// initialize a new RFB
this._rfb = new RFB(document.getElementById('VNC_canvas'), 'wss://test');
this._rfb.viewOnly = true;
this._rfb.ondisconnected = this._handleDisconnect.bind(this);
this._rfb.addEventListener("disconnect",
this._handleDisconnect.bind(this));
this._enablePlaybackMode();
// reset the frame index and timer
@@ -186,8 +187,8 @@ RecordingPlayer.prototype = {
}
},
_handleDisconnect(rfb, reason) {
_handleDisconnect(rfb, clean) {
this._running = false;
this._disconnected(rfb, reason, this._frame_index);
this._disconnected(rfb, clean, this._frame_index);
}
};
tests/test.rfb.js
+54 -21
View File
@@ -377,26 +377,21 @@ describe('Remote Frame Buffer Protocol Client', function() {
expect(client._rfb_connection_state).to.equal('disconnected');
});
it('should set disconnect_reason', function () {
it('should set clean_disconnect variable', function () {
client._rfb_clean_disconnect = true;
client._rfb_connection_state = 'connected';
client._fail('a reason');
expect(client._rfb_disconnect_reason).to.equal('a reason');
client._fail();
expect(client._rfb_clean_disconnect).to.be.false;
});
it('should not include details in disconnect_reason', function () {
client._rfb_connection_state = 'connected';
client._fail('a reason', 'details');
expect(client._rfb_disconnect_reason).to.equal('a reason');
});
it('should result in disconnect callback with message when reason given', function () {
it('should result in disconnect event with clean set to false', function () {
client._rfb_connection_state = 'connected';
var spy = sinon.spy();
client.addEventListener("disconnect", spy);
client._fail('a reason');
client._fail();
this.clock.tick(2000);
expect(spy).to.have.been.calledOnce;
expect(spy.args[0][0].detail.reason).to.equal('a reason');
expect(spy.args[0][0].detail.clean).to.be.false;
});
});
@@ -471,17 +466,16 @@ describe('Remote Frame Buffer Protocol Client', function() {
var client;
beforeEach(function () { client = make_rfb(); });
it('should call the disconnect callback if the state is "disconnected"', function () {
it('should result in a disconnect event if state becomes "disconnected"', function () {
var spy = sinon.spy();
client.addEventListener("disconnect", spy);
client._rfb_connection_state = 'disconnecting';
client._rfb_disconnect_reason = "error";
client._updateConnectionState('disconnected');
expect(spy).to.have.been.calledOnce;
expect(spy.args[0][0].detail.reason).to.equal("error");
expect(spy.args[0][0].detail.clean).to.be.true;
});
it('should not call the disconnect callback if the state is not "disconnected"', function () {
it('should not result in a disconnect event if the state is not "disconnected"', function () {
var spy = sinon.spy();
client.addEventListener("disconnect", spy);
client._sock._websocket.close = function () {}; // explicitly don't call onclose
@@ -489,7 +483,7 @@ describe('Remote Frame Buffer Protocol Client', function() {
expect(spy).to.not.have.been.called;
});
it('should call the disconnect callback without msg when no reason given', function () {
it('should result in a disconnect event without msg when no reason given', function () {
var spy = sinon.spy();
client.addEventListener("disconnect", spy);
client._rfb_connection_state = 'disconnecting';
@@ -653,7 +647,7 @@ describe('Remote Frame Buffer Protocol Client', function() {
expect(client._fail).to.have.been.calledOnce;
expect(client._fail).to.have.been.calledWith(
'Error while negotiating with server','Security failure: whoops');
'Security negotiation failed on no security types (reason: whoops)');
});
it('should transition to the Authentication state and continue on successful negotiation', function () {
@@ -688,7 +682,7 @@ describe('Remote Frame Buffer Protocol Client', function() {
sinon.spy(client, '_fail');
client._sock._websocket._receive_data(new Uint8Array(data));
expect(client._fail).to.have.been.calledWith(
'Authentication failure', 'Whoopsies');
'Security negotiation failed on authentication scheme (reason: Whoopsies)');
});
it('should transition straight to SecurityResult on "no auth" (1) for versions >= 3.8', function () {
@@ -909,14 +903,53 @@ describe('Remote Frame Buffer Protocol Client', function() {
var failure_data = [0, 0, 0, 1, 0, 0, 0, 6, 119, 104, 111, 111, 112, 115];
client._sock._websocket._receive_data(new Uint8Array(failure_data));
expect(client._fail).to.have.been.calledWith(
'Authentication failure', 'whoops');
'Security negotiation failed on security result (reason: whoops)');
});
it('should fail on an error code of 1 with a standard message for version < 3.8', function () {
sinon.spy(client, '_fail');
client._rfb_version = 3.7;
client._sock._websocket._receive_data(new Uint8Array([0, 0, 0, 1]));
expect(client._fail).to.have.been.calledWith('Authentication failure');
expect(client._fail).to.have.been.calledWith(
'Security handshake failed');
});
it('should result in securityfailure event when receiving a non zero status', function () {
var spy = sinon.spy();
client.addEventListener("securityfailure", spy);
client._sock._websocket._receive_data(new Uint8Array([0, 0, 0, 2]));
expect(spy).to.have.been.calledOnce;
expect(spy.args[0][0].detail.status).to.equal(2);
});
it('should include reason when provided in securityfailure event', function () {
client._rfb_version = 3.8;
var spy = sinon.spy();
client.addEventListener("securityfailure", spy);
var failure_data = [0, 0, 0, 1, 0, 0, 0, 12, 115, 117, 99, 104,
32, 102, 97, 105, 108, 117, 114, 101];
client._sock._websocket._receive_data(new Uint8Array(failure_data));
expect(spy.args[0][0].detail.status).to.equal(1);
expect(spy.args[0][0].detail.reason).to.equal('such failure');
});
it('should not include reason when length is zero in securityfailure event', function () {
client._rfb_version = 3.9;
var spy = sinon.spy();
client.addEventListener("securityfailure", spy);
var failure_data = [0, 0, 0, 1, 0, 0, 0, 0];
client._sock._websocket._receive_data(new Uint8Array(failure_data));
expect(spy.args[0][0].detail.status).to.equal(1);
expect('reason' in spy.args[0][0].detail).to.be.false;
});
it('should not include reason in securityfailure event for version < 3.8', function () {
client._rfb_version = 3.6;
var spy = sinon.spy();
client.addEventListener("securityfailure", spy);
client._sock._websocket._receive_data(new Uint8Array([0, 0, 0, 2]));
expect(spy.args[0][0].detail.status).to.equal(2);
expect('reason' in spy.args[0][0].detail).to.be.false;
});
});