From 56c9f3ed9a5b0c09b8915ad71ff26739b00828d8 Mon Sep 17 00:00:00 2001 From: roblabla Date: Sun, 20 Sep 2015 00:41:03 +0200 Subject: [PATCH 1/4] Index packetFields by name. Use packet names in test.js --- src/packets.js | 4 ++-- src/transforms/serializer.js | 12 ++++------ test/test.js | 44 ++++++++++++++++-------------------- 3 files changed, 26 insertions(+), 34 deletions(-) diff --git a/src/packets.js b/src/packets.js index 49d6a72..8cc3d70 100644 --- a/src/packets.js +++ b/src/packets.js @@ -24,12 +24,12 @@ function readPackets(packets, states) { assert(fields !== undefined, 'missing fields for packet ' + name); assert(!packetNames[state][direction].hasOwnProperty(id), 'duplicate packet id ' + id + ' for ' + name); assert(!packetIds[state][direction].hasOwnProperty(name), 'duplicate packet name ' + name + ' for ' + id); - assert(!packetFields[state][direction].hasOwnProperty(id), 'duplicate packet id ' + id + ' for ' + name); + assert(!packetFields[state][direction].hasOwnProperty(name), 'duplicate packet id ' + id + ' for ' + name); assert(!packetStates[direction].hasOwnProperty(name), 'duplicate packet name ' + name + ' for ' + id + ', must be unique across all states'); packetNames[state][direction][id] = name; packetIds[state][direction][name] = id; - packetFields[state][direction][id] = fields; + packetFields[state][direction][name] = fields; packetStates[direction][name] = state; } }); diff --git a/src/transforms/serializer.js b/src/transforms/serializer.js index c463a6b..eb328a9 100644 --- a/src/transforms/serializer.js +++ b/src/transforms/serializer.js @@ -71,8 +71,8 @@ function createPacketBuffer(packetId, state, params, isServer) { if(typeof packetId === 'string') packetId = packetIds[state][direction][packetId]; assert.notEqual(packetId, undefined); - var packet = get(packetId, state, !isServer); var packetName = packetNames[state][direction][packetId]; + var packet = get(packetName, state, !isServer); assert.notEqual(packet, null); packet.forEach(function(fieldInfo) { tryCatch(() => { @@ -106,9 +106,9 @@ function createPacketBuffer(packetId, state, params, isServer) { } -function get(packetId, state, toServer) { +function get(packetName, state, toServer) { var direction = toServer ? "toServer" : "toClient"; - var packetInfo = packetFields[state][direction][packetId]; + var packetInfo = packetFields[state][direction][packetName]; if(!packetInfo) { return null; } @@ -123,7 +123,7 @@ function parsePacketData(buffer, state, isServer, packetsToParse = {"packet": tr var results = {id: packetId, state: state}; // Only parse the packet if there is a need for it, AKA if there is a listener attached to it - var name = packetNames[state][isServer ? "toServer" : "toClient"][packetId]; + var packetName = packetNames[state][isServer ? "toServer" : "toClient"][packetId]; var shouldParse = (!packetsToParse.hasOwnProperty(name) || packetsToParse[name] <= 0) && (!packetsToParse.hasOwnProperty("packet") || packetsToParse["packet"] <= 0); if(shouldParse) { @@ -133,15 +133,13 @@ function parsePacketData(buffer, state, isServer, packetsToParse = {"packet": tr }; } - var packetInfo = get(packetId, state, isServer); + var packetInfo = get(packetName, state, isServer); if(packetInfo === null) { throw new Error("Unrecognized packetId: " + packetId + " (0x" + packetId.toString(16) + ")") } else { - var packetName = packetNames[state][isServer ? "toServer" : "toClient"][packetId]; debug("read packetId " + state + "." + packetName + " (0x" + packetId.toString(16) + ")"); } - var packetName = packetNames[state][!isServer ? 'toClient' : 'toServer'][packetId]; var i, fieldInfo, readResults; for(i = 0; i < packetInfo.length; ++i) { fieldInfo = packetInfo[i]; diff --git a/test/test.js b/test/test.js index 3eff6d8..9c7a9ae 100644 --- a/test/test.js +++ b/test/test.js @@ -133,42 +133,38 @@ describe("packets", function() { }); client.end(); }); - var packetId, packetInfo, field; + var packetName, packetInfo, field; for(state in mc.packetFields) { if(!mc.packetFields.hasOwnProperty(state)) continue; - for(packetId in mc.packetFields[state].toServer) { - if(!mc.packetFields[state].toServer.hasOwnProperty(packetId)) continue; - packetId = parseInt(packetId, 10); - packetInfo = mc.get(packetId, state, true); - it(state + ",ServerBound,0x" + zfill(parseInt(packetId, 10).toString(16), 2), - callTestPacket(packetId, packetInfo, state, true)); + for(packetName in mc.packetFields[state].toServer) { + if(!mc.packetFields[state].toServer.hasOwnProperty(packetName)) continue; + packetInfo = mc.get(packetName, state, true); + it(state + ",ServerBound," + packetName, + callTestPacket(packetName, packetInfo, state, true)); } - for(packetId in mc.packetFields[state].toClient) { - if(!mc.packetFields[state].toClient.hasOwnProperty(packetId)) continue; - packetId = parseInt(packetId, 10); - packetInfo = mc.get(packetId, state, false); - it(state + ",ClientBound,0x" + zfill(parseInt(packetId, 10).toString(16), 2), - callTestPacket(packetId, packetInfo, state, false)); + for(packetName in mc.packetFields[state].toClient) { + if(!mc.packetFields[state].toClient.hasOwnProperty(packetName)) continue; + packetInfo = mc.get(packetName, state, false); + it(state + ",ClientBound," + packetName, + callTestPacket(packetName, packetInfo, state, false)); } } - function callTestPacket(packetId, packetInfo, state, toServer) { + function callTestPacket(packetName, packetInfo, state, toServer) { return function(done) { client.state = state; serverClient.state = state; - testPacket(packetId, packetInfo, state, toServer, done); + testPacket(packetName, packetInfo, state, toServer, done); }; } - function testPacket(packetId, packetInfo, state, toServer, done) { + function testPacket(packetName, packetInfo, state, toServer, done) { // empty object uses default values var packet = {}; packetInfo.forEach(function(field) { packet[field.name] = getValue(field.type, packet); }); if(toServer) { - serverClient.once([state, packetId], function(receivedPacket) { - delete receivedPacket.id; - delete receivedPacket.state; + serverClient.once(packetName, function(receivedPacket) { try { assertPacketsMatch(packet, receivedPacket); } catch (e) { @@ -177,15 +173,13 @@ describe("packets", function() { } done(); }); - client.write(packetId, packet); + client.write(packetName, packet); } else { - client.once([state, packetId], function(receivedPacket) { - delete receivedPacket.id; - delete receivedPacket.state; + client.once(packetName, function(receivedPacket) { assertPacketsMatch(packet, receivedPacket); done(); }); - serverClient.write(packetId, packet); + serverClient.write(packetName, packet); } } @@ -327,7 +321,7 @@ describe("client", function() { username: 'Player', }); var gotKicked = false; - client.on([states.LOGIN, 0x00], function(packet) { + client.on('disconnect', function(packet) { assert.ok(packet.reason.indexOf('"Failed to verify username!"')!=-1); gotKicked = true; }); From b1441098f9a3e48e57db4850b4c3061e7b8348e7 Mon Sep 17 00:00:00 2001 From: roblabla Date: Sun, 20 Sep 2015 00:54:19 +0200 Subject: [PATCH 2/4] Only allow writing by packetName. Refactor whole internals to use packetNames --- src/client.js | 24 ++---- src/transforms/serializer.js | 148 ++++++++++++++--------------------- test/benchmark.js | 3 +- 3 files changed, 67 insertions(+), 108 deletions(-) diff --git a/src/client.js b/src/client.js index 12d2c97..899b46b 100644 --- a/src/client.js +++ b/src/client.js @@ -130,12 +130,10 @@ Client.prototype.setSocket = function(socket) { this.serializer.pipe(this.framer).pipe(this.socket); this.deserializer.on('data', (parsed) => { - var packet = parsed.results; - var packetName = packetIndexes.packetNames[packet.state][this.isServer ? 'toServer' : 'toClient'][packet.id]; - this.emit('packet', packet); - this.emit(packetName, packet); - this.emit('raw.' + packetName, parsed.buffer, packet.state); - this.emit('raw', parsed.buffer, packet.state); + this.emit('packet', parsed.data, parsed.metadata); + this.emit(parsed.metadata.name, parsed.data, parsed.metadata); + this.emit('raw.' + parsed.metadata.name, parsed.buffer, parsed.metadata); + this.emit('raw', parsed.buffer, parsed.metadata); }); }; @@ -173,18 +171,10 @@ Client.prototype.setCompressionThreshold = function(threshold) { } } -Client.prototype.write = function(packetId, params) { - if(Array.isArray(packetId)) { - if(packetId[0] !== this.state) - return false; - packetId = packetId[1]; - } - if(typeof packetId === "string") - packetId = packetIndexes.packetIds[this.state][this.isServer ? "toClient" : "toServer"][packetId]; - var packetName = packetIndexes.packetNames[this.state][this.isServer ? "toClient" : "toServer"][packetId]; - debug("writing packetId " + this.state + "." + packetName + " (0x" + packetId.toString(16) + ")"); +Client.prototype.write = function(packetName, params) { + debug("writing packet " + this.state + "." + packetName); debug(params); - this.serializer.write({ packetId, params }); + this.serializer.write({ packetName, params }); }; Client.prototype.writeRaw = function(buffer) { diff --git a/src/transforms/serializer.js b/src/transforms/serializer.js index eb328a9..2e8334e 100644 --- a/src/transforms/serializer.js +++ b/src/transforms/serializer.js @@ -60,47 +60,32 @@ var packetStates = packetIndexes.packetStates; // TODO : This does NOT contain the length prefix anymore. -function createPacketBuffer(packetId, state, params, isServer) { - var length = 0; - var direction=!isServer ? 'toServer' : 'toClient'; - if(typeof packetId === 'string' && typeof state !== 'string' && !params) { - // simplified two-argument usage, createPacketBuffer(name, params) - params = state; - state = packetStates[direction][packetId]; - } - if(typeof packetId === 'string') packetId = packetIds[state][direction][packetId]; +function createPacketBuffer(packetName, state, params, isServer) { + var direction = !isServer ? 'toServer' : 'toClient'; + var packetId = packetIds[state][direction][packetName]; assert.notEqual(packetId, undefined); - - var packetName = packetNames[state][direction][packetId]; var packet = get(packetName, state, !isServer); assert.notEqual(packet, null); - packet.forEach(function(fieldInfo) { - tryCatch(() => { - length += proto.sizeOf(params[fieldInfo.name], fieldInfo.type, params); - }, (e) => { - addErrorField(e, fieldInfo.name); - e.message = "sizeOf error for "+[state,direction,packetName,e.field].join(".")+"\n"+ - " in packet 0x" + packetId.toString(16)+" "+JSON.stringify(params)+"\n" - + e.message; - throw e; - }); + + var length = utils.varint[2](packetId); + tryCatch(() => { + length += structures.container[2].call(proto, params, packet, {}); + //length += proto.sizeOf(params, ["container", packet], {}); + }, (e) => { + e.field = [state, direction, packetName, e.field].join("."); + e.message = `SizeOf error for ${e.field} : ${e.message}`; + throw e; }); - length += utils.varint[2](packetId); - var size = length;// + utils.varint[2](length); - var buffer = new Buffer(size); - var offset = 0;//utils.varint[1](length, buffer, 0); - offset = utils.varint[1](packetId, buffer, offset); - packet.forEach(function(fieldInfo) { - var value = params[fieldInfo.name]; - // TODO : This check belongs to the respective datatype. - if(typeof value === "undefined" && fieldInfo.type != "count") - debug(new Error("Missing Property " + fieldInfo.name).stack); - tryCatch(() => { - offset = proto.write(value, buffer, offset, fieldInfo.type, params); - }, (e) => { - e.message = "Write error for " + packetName + "." + e.field + " : " + e.message; - throw e; - }); + + var buffer = new Buffer(length); + var offset = utils.varint[1](packetId, buffer, 0); + tryCatch(() => { + offset = structures.container[1].call(proto, params, buffer, offset, packet, {}); + //offset = proto.write(params, buffer, offset, ["container", packet], {}); + }, (e) => { + e.field = [state, direction, packetName, e.field].join("."); + e.message = `Write error for ${e.field} : ${e.message}`; + throw e; }); return buffer; } @@ -115,55 +100,49 @@ function get(packetName, state, toServer) { return packetInfo; } - -// By default, parse every packets. function parsePacketData(buffer, state, isServer, packetsToParse = {"packet": true}) { - var cursor = 0; - var { value: packetId, size: cursor } = utils.varint[0](buffer, cursor); + var { value: packetId, size: cursor } = utils.varint[0](buffer, 0); - var results = {id: packetId, state: state}; - // Only parse the packet if there is a need for it, AKA if there is a listener attached to it - var packetName = packetNames[state][isServer ? "toServer" : "toClient"][packetId]; - var shouldParse = (!packetsToParse.hasOwnProperty(name) || packetsToParse[name] <= 0) - && (!packetsToParse.hasOwnProperty("packet") || packetsToParse["packet"] <= 0); - if(shouldParse) { - return { - buffer: buffer, - results: results - }; - } + var direction = isServer ? "toServer" : "toClient"; + var packetName = packetNames[state][direction][packetId]; + var results = { + metadata: { + name: packetName, + id: packetId, + state + }, + data: {}, + buffer + }; + + // Only parse the packet if there is a need for it, AKA if there is a listener + // attached to it. + var shouldParse = + (packetsToParse.hasOwnProperty(packetName) && packetsToParse[packetName] > 0) || + (packetsToParse.hasOwnProperty("packet") && packetsToParse["packet"] > 0); + if (!shouldParse) + return results; var packetInfo = get(packetName, state, isServer); - if(packetInfo === null) { + if(packetInfo === null) throw new Error("Unrecognized packetId: " + packetId + " (0x" + packetId.toString(16) + ")") - } else { + else debug("read packetId " + state + "." + packetName + " (0x" + packetId.toString(16) + ")"); - } - var i, fieldInfo, readResults; - for(i = 0; i < packetInfo.length; ++i) { - fieldInfo = packetInfo[i]; - tryCatch(() => { - readResults = proto.read(buffer, cursor, fieldInfo.type, results); - }, (e) => { - e.message = "Read error for " + packetName + "." + e.field + " : " + e.message; - throw e; - }); - if(readResults === null) - throw new Error("A reader returned null. This is _not_ normal"); - if(readResults.error) - throw new Error("A reader returned an error using the old method."); - if (readResults.value != null) - results[fieldInfo.name] = readResults.value; - cursor += readResults.size; - } + var res; + tryCatch(() => { + res = proto.read(buffer, cursor, ["container", packetInfo], {}); + }, (e) => { + e.field = [state, direction, packetName, e.field].join("."); + e.message = `Read error for ${e.field} : ${e.message}`; + throw e; + }); + results.data = res.value; + cursor += res.size; if(buffer.length > cursor) - throw new Error("Packet data not entirely read"); + throw new Error(`Read error for ${packetName} : Packet data not entirely read`); debug(results); - return { - results: results, - buffer: buffer - }; + return results; } class Serializer extends Transform { @@ -173,10 +152,9 @@ class Serializer extends Transform { this.isServer = isServer; } - // TODO : Might make sense to make createPacketBuffer async. _transform(chunk, enc, cb) { try { - var buf = createPacketBuffer(chunk.packetId, this.protocolState, chunk.params, this.isServer); + var buf = createPacketBuffer(chunk.packetName, this.protocolState, chunk.params, this.isServer); this.push(buf); return cb(); } catch (e) { @@ -200,15 +178,7 @@ class Deserializer extends Transform { } catch (e) { return cb(e); } - if (packet.error) - { - packet.error.packet = packet; - return cb(packet.error) - } - else - { - this.push(packet); - return cb(); - } + this.push(packet); + return cb(); } } diff --git a/test/benchmark.js b/test/benchmark.js index f30ccfb..d1fcd97 100644 --- a/test/benchmark.js +++ b/test/benchmark.js @@ -18,8 +18,7 @@ console.log('Beginning write test'); start = Date.now(); for(i = 0; i < ITERATIONS; i++) { for(j = 0; j < testDataWrite.length; j++) { - var id=mc.packetIds['play']['toServer'][testDataWrite[j].name]; - inputData.push(mc.createPacketBuffer(id, states.PLAY, testDataWrite[j].params, false)); + inputData.push(mc.createPacketBuffer(testDataWrite[j].name, states.PLAY, testDataWrite[j].params, false)); } } console.log('Finished write test in ' + (Date.now() - start) / 1000 + ' seconds'); From b85fa944d742f732af40760d1c9e4fbb8f56f725 Mon Sep 17 00:00:00 2001 From: roblabla Date: Tue, 15 Sep 2015 14:27:46 +0000 Subject: [PATCH 3/4] Remove on(id), on([state;id]) and onRaw --- src/client.js | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/src/client.js b/src/client.js index 899b46b..156d3b1 100644 --- a/src/client.js +++ b/src/client.js @@ -66,30 +66,6 @@ function Client(isServer) { util.inherits(Client, EventEmitter); -// Transform weird "packet" types into string representing their type. Should be mostly retro-compatible -Client.prototype.on = function(type, func) { - var direction = this.isServer ? 'toServer' : 'toClient'; - if(Array.isArray(type)) { - arguments[0] = packetIndexes.packetNames[type[0]][direction][type[1]]; - } else if(typeof type === "number") { - arguments[0] = packetIndexes.packetNames[this.state][direction][type]; - } - EventEmitter.prototype.on.apply(this, arguments); -}; - -Client.prototype.onRaw = function(type, func) { - var arg = "raw."; - if(Array.isArray(type)) { - arg += packetIndexes.packetNames[type[0]][direction][type[1]]; - } else if(typeof type === "number") { - arg += packetIndexes.packetNames[this.state][direction][type]; - } else { - arg += type; - } - arguments[0] = arg; - EventEmitter.prototype.on.apply(this, arguments); -}; - Client.prototype.setSocket = function(socket) { var ended = false; From 537a99562a89d3645f373d84ee178c3b542d3412 Mon Sep 17 00:00:00 2001 From: Romain Beaumont Date: Tue, 15 Sep 2015 20:28:46 +0200 Subject: [PATCH 4/4] use forEach for better speed for containers --- src/datatypes/structures.js | 54 ++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/datatypes/structures.js b/src/datatypes/structures.js index a83aac5..4cb7c04 100644 --- a/src/datatypes/structures.js +++ b/src/datatypes/structures.js @@ -104,26 +104,26 @@ function readContainer(buffer, offset, typeArgs, rootNode) { }; var backupThis = rootNode.this; rootNode.this = results.value; - for(var index in typeArgs) { + typeArgs.forEach((typeArg) => { var readResults; tryCatch(() => { - readResults = this.read(buffer, offset, typeArgs[index].type, rootNode); + readResults = this.read(buffer, offset, typeArg.type, rootNode); + results.size += readResults.size; + offset += readResults.size; + if (typeArg.anon) { + Object.keys(readResults.value).forEach(function(key) { + results.value[key] = readResults.value[key]; + }); + } else + results.value[typeArg.name] = readResults.value; }, (e) => { - if (typeArgs && typeArgs[index] && typeArgs[index].name) - addErrorField(e, typeArgs[index].name); + if (typeArgs && typeArg && typeArg.name) + addErrorField(e, typeArg.name); else addErrorField(e, "unknown"); throw e; }); - results.size += readResults.size; - offset += readResults.size; - if (typeArgs[index].anon) { - Object.keys(readResults.value).forEach(function(key) { - results.value[key] = readResults.value[key]; - }); - } else - results.value[typeArgs[index].name] = readResults.value; - } + }); rootNode.this = backupThis; return results; } @@ -131,20 +131,20 @@ function readContainer(buffer, offset, typeArgs, rootNode) { function writeContainer(value, buffer, offset, typeArgs, rootNode) { var backupThis = rootNode.this; rootNode.this = value; - for(var index in typeArgs) { + typeArgs.forEach((typeArg) => { tryCatch(() => { - if (typeArgs[index].anon) - offset = this.write(value, buffer, offset, typeArgs[index].type, rootNode); + if (typeArg.anon) + offset = this.write(value, buffer, offset, typeArg.type, rootNode); else - offset = this.write(value[typeArgs[index].name], buffer, offset, typeArgs[index].type, rootNode); + offset = this.write(value[typeArg.name], buffer, offset, typeArg.type, rootNode); }, (e) => { - if (typeArgs && typeArgs[index] && typeArgs[index].name) - addErrorField(e, typeArgs[index].name); + if (typeArgs && typeArg && typeArg.name) + addErrorField(e, typeArg.name); else addErrorField(e, "unknown"); throw e; }); - } + }); rootNode.this = backupThis; return offset; } @@ -153,20 +153,20 @@ function sizeOfContainer(value, typeArgs, rootNode) { var size = 0; var backupThis = rootNode.this; rootNode.this = value; - for(var index in typeArgs) { + typeArgs.forEach((typeArg) => { tryCatch(() => { - if (typeArgs[index].anon) - size += this.sizeOf(value, typeArgs[index].type, rootNode); + if (typeArg.anon) + size += this.sizeOf(value, typeArg.type, rootNode); else - size += this.sizeOf(value[typeArgs[index].name], typeArgs[index].type, rootNode); + size += this.sizeOf(value[typeArg.name], typeArg.type, rootNode); }, (e) => { - if (typeArgs && typeArgs[index] && typeArgs[index].name) - addErrorField(e, typeArgs[index].name); + if (typeArgs && typeArg && typeArg.name) + addErrorField(e, typeArg.name); else addErrorField(e, "unknown"); throw e; }); - } + }); rootNode.this = backupThis; return size; }