Skip to content

Commit

Permalink
feat: catch more invalid discovery messages
Browse files Browse the repository at this point in the history 
Client#startDiscovery has been changed to emit Client#discovery-invalid
Instead of throwing an error in more cases than before when
processing incoming messages
  • Loading branch information
@plasticrake
plasticrake committed Nov 5, 2023
1 parent c6bb459 commit 85efa12
Show file tree
Hide file tree
Showing 3 changed files with 194 additions and 58 deletions.
134 changes: 76 additions & 58 deletions src/client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -559,77 +559,95 @@ export default class Client extends EventEmitter implements ClientEventEmitter {
`client.startDiscovery(): socket:message From: ${rinfo.address} ${rinfo.port} Message: ${decryptedMsg}`
);

let response: DiscoveryResponse;
let sysInfo: Sysinfo;
try {
response = JSON.parse(decryptedMsg);
sysInfo = response.system.get_sysinfo;
} catch (err) {
this.log.debug(
`client.startDiscovery(): Error parsing JSON: %s\nFrom: ${rinfo.address} ${rinfo.port} Original: [%s] Decrypted: [${decryptedMsg}]`,
err,
msg
);
this.emit('discovery-invalid', {
rinfo,
response: msg,
decryptedResponse: decrypt(msg),
});
return;
}

if (deviceTypes && deviceTypes.length > 0) {
const deviceType = this.getTypeFromSysInfo(sysInfo);
if (!(deviceTypes as string[]).includes(deviceType)) {
// TODO: Type checking of response/sysInfo could be improved
let response: DiscoveryResponse;
let sysInfo: Sysinfo;
try {
response = JSON.parse(decryptedMsg);
sysInfo = response.system.get_sysinfo;
if (sysInfo == null)
throw new Error('system.get_sysinfo is null or undefined');
if (!isObjectLike(sysInfo))
throw new Error('system.get_sysinfo is not an object');
} catch (err) {
this.log.debug(
`client.startDiscovery(): Filtered out: ${sysInfo.alias} [${sysInfo.deviceId}] (${deviceType}), allowed device types: (%j)`,
deviceTypes
`client.startDiscovery(): Error parsing JSON: %s\nFrom: ${rinfo.address} ${rinfo.port} Original: [%s] Decrypted: [${decryptedMsg}]`,
err,
msg
);
this.emit('discovery-invalid', {
rinfo,
response: msg,
decryptedResponse: decryptedMsg,
});
return;
}
}

let mac: string;
if ('mac' in sysInfo) mac = sysInfo.mac;
else if ('mic_mac' in sysInfo) mac = sysInfo.mic_mac;
else if ('ethernet_mac' in sysInfo) mac = sysInfo.ethernet_mac;
else mac = '';
if (deviceTypes && deviceTypes.length > 0) {
const deviceType = this.getTypeFromSysInfo(sysInfo);
if (!(deviceTypes as string[]).includes(deviceType)) {
this.log.debug(
`client.startDiscovery(): Filtered out: ${sysInfo.alias} [${sysInfo.deviceId}] (${deviceType}), allowed device types: (%j)`,
deviceTypes
);
return;
}
}

if (macAddresses && macAddresses.length > 0) {
if (!compareMac(mac, macAddresses)) {
this.log.debug(
`client.startDiscovery(): Filtered out: ${sysInfo.alias} [${sysInfo.deviceId}] (${mac}), allowed macs: (%j)`,
macAddresses
);
return;
let mac: string;
if ('mac' in sysInfo) mac = sysInfo.mac;
else if ('mic_mac' in sysInfo) mac = sysInfo.mic_mac;
else if ('ethernet_mac' in sysInfo) mac = sysInfo.ethernet_mac;
else mac = '';

if (macAddresses && macAddresses.length > 0) {
if (!compareMac(mac, macAddresses)) {
this.log.debug(
`client.startDiscovery(): Filtered out: ${sysInfo.alias} [${sysInfo.deviceId}] (${mac}), allowed macs: (%j)`,
macAddresses
);
return;
}
}
}

if (excludeMacAddresses && excludeMacAddresses.length > 0) {
if (compareMac(mac, excludeMacAddresses)) {
this.log.debug(
`client.startDiscovery(): Filtered out: ${sysInfo.alias} [${sysInfo.deviceId}] (${mac}), excluded mac`
);
return;
if (excludeMacAddresses && excludeMacAddresses.length > 0) {
if (compareMac(mac, excludeMacAddresses)) {
this.log.debug(
`client.startDiscovery(): Filtered out: ${sysInfo.alias} [${sysInfo.deviceId}] (${mac}), excluded mac`
);
return;
}
}
}

if (typeof filterCallback === 'function') {
if (!filterCallback(sysInfo)) {
this.log.debug(
`client.startDiscovery(): Filtered out: ${sysInfo.alias} [${sysInfo.deviceId}], callback`
);
return;
if (typeof filterCallback === 'function') {
if (!filterCallback(sysInfo)) {
this.log.debug(
`client.startDiscovery(): Filtered out: ${sysInfo.alias} [${sysInfo.deviceId}], callback`
);
return;
}
}
}

this.createOrUpdateDeviceFromSysInfo({
sysInfo,
host: rinfo.address,
port: devicesUseDiscoveryPort ? rinfo.port : undefined,
breakoutChildren,
options: deviceOptions,
});
this.createOrUpdateDeviceFromSysInfo({
sysInfo,
host: rinfo.address,
port: devicesUseDiscoveryPort ? rinfo.port : undefined,
breakoutChildren,
options: deviceOptions,
});
} catch (err) {
this.log.debug(
`client.startDiscovery(): Error processing response: %s\nFrom: ${rinfo.address} ${rinfo.port} Original: [%s] Decrypted: [${decryptedMsg}]`,
err,
msg
);
this.emit('discovery-invalid', {
rinfo,
response: msg,
decryptedResponse: decrypt(msg),
});
}
});

socket.on('error', (err) => {
Expand Down
111 changes: 111 additions & 0 deletions test/client.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
/* eslint-disable no-unused-expressions */

const sinon = require('sinon');
const dgram = require('dgram');
const EventEmitter = require('events');
const { encrypt } = require('tplink-smarthome-crypto');

const { config, expect, getTestClient, testDevices } = require('./setup');

const { default: Client } = require('../src/client');
Expand All @@ -10,6 +14,22 @@ const { default: Bulb } = require('../src/bulb');

const { compareMac } = require('../src/utils');

const validPlugDiscoveryResponse = {
system: {
get_sysinfo: {
alias: 'test',
deviceId: 'test',
model: 'test',
sw_ver: 'test',
hw_ver: 'test',
type: 'plug',
mac: 'test',
feature: 'test',
relay_state: 0,
},
},
};

describe('Client', function () {
describe('constructor', function () {
it('should use custom logger', function () {
Expand Down Expand Up @@ -181,6 +201,97 @@ describe('Client', function () {
});
});

it('should ignore invalid devices that respond without encryption', function (done) {
const socket = new EventEmitter();

const createSocket = function () {
socket.bind = sinon.fake();
socket.address = () => ({ address: '1.2.3.4', port: 1234 });
socket.setBroadcast = sinon.fake();
return socket;
};

sinon.replace(dgram, 'createSocket', createSocket);

const message = JSON.stringify(validPlugDiscoveryResponse);

client
.startDiscovery({ discoveryInterval: 250 })
.on('device-new', (device) => {
client.stopDiscovery();
done(new Error(`Device should have been ignored: ${device.host}`));
})
.on('discovery-invalid', ({ rinfo, response }) => {
expect(rinfo.address).to.eql('1.2.3.5');
expect(rinfo.port).to.eql(1235);
expect(response).to.eql(message);
done();
});

socket.emit('message', message, {
address: '1.2.3.5',
port: 1235,
});
});

describe('should ignore invalid devices that respond without valid response', function () {
[
JSON.stringify(''),
JSON.stringify('data'),
JSON.stringify({}),
JSON.stringify({ unexpected: 'data' }),
JSON.stringify({ system: undefined }),
JSON.stringify({ system: {} }),
JSON.stringify({ system: 'data' }),
JSON.stringify({ system: { get_sysinfo: undefined } }),
JSON.stringify({ system: { get_sysinfo: {} } }),
JSON.stringify({ system: { get_sysinfo: 'data' } }),
JSON.stringify({ system: { get_sysinfo: { alias: 'test' } } }),
].forEach((t) => {
['encrypted', 'unencrypted'].forEach((te) => {
it(`${t} - ${te}`, function (done) {
const socket = new EventEmitter();

const createSocket = function () {
socket.bind = sinon.fake();
socket.address = () => ({ address: '1.2.3.4', port: 1234 });
socket.setBroadcast = sinon.fake();
return socket;
};

sinon.replace(dgram, 'createSocket', createSocket);

let message;
if (te === 'encrypted') {
message = encrypt(t);
} else {
message = t;
}

client
.startDiscovery({ discoveryInterval: 250 })
.on('device-new', (device) => {
client.stopDiscovery();
done(
new Error(`Device should have been ignored: ${device.host}`)
);
})
.on('discovery-invalid', ({ rinfo, response }) => {
expect(rinfo.address).to.eql('1.2.3.5');
expect(rinfo.port).to.eql(1235);
expect(response).to.eql(message);
done();
});

socket.emit('message', message, {
address: '1.2.3.5',
port: 1235,
});
});
});
});
});

const events = ['new', 'online', 'offline'];
const eventTests = [];
[
Expand Down
7 changes: 7 additions & 0 deletions test/setup-test.js
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const sinon = require('sinon');

const {
config,
expect,
Expand Down Expand Up @@ -112,3 +114,8 @@ describe('Test Environment Setup', function () {
});
});
});

afterEach(() => {
// Restore the default sandbox here
sinon.restore();
});

0 comments on commit 85efa12

Please sign in to comment.