[Cyril]

Okay so, I asked on the Nodejs IRC, and very helpful frenchman helped me out. This code now works:

var net = require('net'),
	    events = require('events'),
	    util = require('util');
	
	function Connection(settings) {
	  events.EventEmitter.call(this);
	  var self = this;
	
	  this.socket = net.createConnection(settings.port, settings.host);
	  this.socket.setNoDelay(true);
	  this.status = 'connecting';
	
	  this.socket.on('connect', function() {
	    console.log('connected');
	    console.log('writing');
	    
	    var auth = "ds;localhost:25565";
	    var b = new Buffer(auth.length + 1);
	    b[0] = 2;
	    b.write(auth, 1);
	    
	    self.socket.write(b, function() {
	      console.log('flushed');
	    });
	    self.status = 'send-2';
	  });
	
	  function handleByte(B) {
	    switch(self.status) {
	      case 'send-2':
	        if(b !== 0x2)
	          return false;
	
	        self.socket.write(new Buffer('01', 'hex'), function() {
	          console.log('flushed');
	        });
	        self.status = 'send-1';
	
	        return true;
	
	      case 'send-1':
	        if(b !== 0x1)
	          return false;
	
	        self.status = 'recv-0d';
	
	        return true;
	
	      case 'recv-0d':
	        if(b !== 0x1)
	          return false;
	
	        self.emit('authenticated');
	
	        return true;
	
	      default:
	        return false;
	    }
	  }
	
	  this.socket.on('data', function(chunk) {
	    console.log('got data: ' + chunk);
	    while(chunk.length > 0) {
	      if(!handleByte(chunk[0])) {
	        console.error('error in ' + self.status);
	        process.exit(1);
	      }
	
	      chunk = chunk.slice(1);
	    }
	  });
	}
	
	util.inherits(events.EventEmitter, Connection);
	
	exports.Connection = Connection;
	
	var server = new Connection({ port: 25565, host: "localhost"});

Now, the problem is, when it connects to the server (minecraft server), I get this in my server console:

ava.io.IOException: Received string length longer than maximum allowed (26217 > 64)
	at lx.a(SourceFile:197)
	at ny.a(SourceFile:21)
	at lx.a(SourceFile:155)
	at qq.h(SourceFile:189)
	at qq.c(SourceFile:9)
	at zt.run(SourceFile:76)
2012-03-29 17:48:00 [INFO] /127.0.0.1:57231 lost connection

Okay. So what do I do? I'm guessing it has to do with this part of my code:

var auth = "ds;localhost:25565";
	    var b = new Buffer(auth.length + 1);
	    b[0] = 2;
	    b.write(auth, 1);
	    
	    self.socket.write(b, function() {

If I run Wireshark on localhost, and connect using the official client, I should be expecting this:

00000000  02 00 1e 00 66 00 69 00  72 00 65 00 63 00 61 00 ....f.i. r.e.c.a.
00000010  74 00 64 00 65 00 73 00  69 00 67 00 6e 00 73 00 t.d.e.s. i.g.n.s.
00000020  3b 00 6c 00 6f 00 63 00  61 00 6c 00 68 00 6f 00 ;.l.o.c. a.l.h.o.
00000030  73 00 74 00 3a 00 32 00  35 00 35 00 36 00 35    s.t.:.2. 5.5.6.5

However, my code generates this:

00000000  02 66 69 72 65 63 61 74  64 65 73 69 67 6e 73 3b .ds ds;
00000010  6c 6f 63 61 6c 68 6f 73  74 3a 32 35 35 36 35    localhos t:25565

What do I have to do? If you look at the TCP dump, the actual data is the same, but the official client sticks NULL between each character, hence why it's twice as long. (And for some reason, it also adds "30" before starting the string part -> 1e = 30)

[Kyek]

Ooo, I *like* this project :D I used to do this crap to a game called Furcadia back in the day. Awesome stuff.

Anyway! I haven't worked a *lot* with Buffers in Node, but rather than writing the whole string to the buffer, just loop through each character and write in a NULL character (like this, if you want to use a string: "\0") before it. Easy peasey

[Cyril]

Yeaaaah, I *may* be planning to create an army of robots to take over a friend's Minecraft server xD

So you mean, instead of doing this:

buffer.write(string);

I should do

// for loop
    buffer[i] = NULL;
    // find the next character or something
    buffer.write(character, i);

? (or something along those lines?)

[Kyek]

Cyril, on 29 March 2012 - 01:38 PM, said:

Yeaaaah, I *may* be planning to create an army of robots to take over a friend's Minecraft server xD

So you mean, instead of doing this:

buffer.write(string);

I should do

// for loop
    buffer[i] = NULL;
    // find the next character or something
    buffer.write(character, i);

? (or something along those lines?)

I don't know, that method of access is scary. Buffers were meant to manage their own data. I'd do it more like this:

var str="frostydesigns;localhost:25565",
    len = str.length,
    buffer = new Buffer(len * 2 + 3);
buffer.writeUInt8(0x02, 0);
buffer.writeUInt8(0x00, 1);
buffer.writeUInt8(len, 2); // That 1e? It's the length of your string.  30 characters.
for(var i = 0; i < len; i++) {
    buffer.writeUInt8(0x00, i * 2 + 3); // *2 since this loop writes 2 bytes, +3 because we've already written bytes 0-2.
    buffer.write(str[i], i * 2 + 4, 1);
}

That should do it

[Cyril]

Thanks, that works great

Just one more question, now. How should I structure this?
I need to have various "actions", which contain multiple exchanges. For example, the login action contains (as I already said above);

-> Send server 0x02
<- Receive 0x02
-> Send server 0x01
<- Receive 0x01
<- Receive 0x0D

However, that means writing to the socket a few times -- and I'm going to have many functions like that.

Here's the code I have at the moment, which only sends the first 0x02:

var net = require("net");

function Server(info) {

	this.socket = net.createConnection(info.port, info.host);
	this.socket.setNoDelay(true);
	
	this.port = info.port;
	this.host = info.host;
	
	this.hex = {
		handshake: 0x02
	};
	
	this.socket.on("data", this.dataHandler);
	this.socket.on("connect", function() {
		console.log("[->] Connected to " + info.host + ":" + info.port);
	});
}

Server.prototype.login = function(username) {
	var string = username+";"+this.host+":"+this.port;
	
	this.send(this.hex.handshake, string, function() {
		console.log("[->] Sent handshake: " + string);
	});
}

Server.prototype.dataHandler = function(data) {
	console.log("[<-] Received: " + data[0]);
}

Server.prototype.send = function(hex, str, callback) {

	var buffer = new Buffer(str.length * 2 + 3);
	    buffer.writeUInt8(hex, 0);
	    buffer.writeUInt8(0x00, 1);
	    buffer.writeUInt8(str.length, 2);
	
	for(var i = 0; i < str.length; i++) {
		buffer.writeUInt8(0x00, i * 2 + 3);
		buffer.write(str[i], i * 2 + 4, 1);
	}

	this.socket.write(buffer, callback);
}

var conn = new Server({ host: "localhost", port: 25565 });
    conn.login("ds");

Maybe I should create a seperate function from Server, for example Player, which would contain all the player-related actions such as logging in, etc?
If I do that, how will I structure it *inside* the methods anyway? How do I wait for each reply, and send the next one I should?

[Kyek]

Have I mentioned I love this project? Because I love this project.

I wish I were more familiar with Minecraft (specifically, with the protocol schema), because I can't suggest a firm way to do this. I can only really tell you how I'd start.

Your server object is awesome. As-is, you could turn your dataHandler function into a giant state machine, and do everything you need right in this class. But I'm going to assume you understand enough of the protocol to pull out data about the bot's surrounding's: Players, with their names at minimum, and ideally something about their positioning. Maybe information about the world map. NPCs too, if you know what the TCP stream looks like when an animal walks by or something.

If I were writing this, I'd take all of those pieces and make them into classes, and then store instances in the Server object. So the Server object would have an array of players, an array of NPCs, and maybe a World object that tracks information about the world map. Each of these objects, then, will have a function called ingest(data). So what happens is, the server sends data. Server.dataHandler is responsible for collecting the entire packet. I'm assuming, here, that the structure of packets that Minecraft sends to you is the exact same format as what you send to them: an instruction code, followed by a null byte, the length of the packet, followed by a null byte, and then the packet data (with null bytes between each character). Which means, your dataHandler can read in a full packet, and know exactly when that packet ends.

Now, Server.dataHandler has the packet, and it looks at the contents and decides what it's for. Does it refer to player X? Find player X in the Players array, and send the packet to him. Did a pig appear on the map? Create a new NPC object for the Server's NPC array, and send it that packet. You get the idea. Now, dataHandler isn't responsible for fully "understanding" the contents of the packet, nor is it responsible for reacting to it. You get to keep all of your "reaction" code separated into the objects that the actions refer to.

So that's where I'd start with code organization, but you know more about the protocol structure than I do -- so feel free to tweak or ignore all of the above if it doesn't really make sense

[Cyril]

Thanks, what I've settled with (and what works really great) is that I take the first "command" byte from the reply, and grab a corresponding function from an "events" objet. For example:

this.events = {
		0x02: this.login.bind(this),
		0x01: this.born.bind(this)
	};

So that works great. Problem now, is that the type of request is totally different from the first step I had to do.
It's not just 0x01 I have to send, it's 0x01, a 4 byte long 1D (the protocol version), then the username again, and finally a bunch of unused values, but which have to be there.

That means that simply writing a NULL between each character, etc. won't work as planned (with your function).
So, I rewrote it to take data from an array. Here's what I've got:

Server.prototype.send = function(data, callback) {
	
	var length = 0;
	for (var i = 0; i < data.length; i++) {
		if (typeof data[i] != "number")
			for (var y = 0; y < data[i].length; y++)
				length++;
		else
			length++;
	}
	
	var buffer = new Buffer(length * 2 + 3);
	var bufferIndex = 1;
	
	buffer.writeUInt8(data[0], 0);
	for (var i = 1; i < data.length; i++) {
		if (typeof data[i] != "number") {
			for (var y = 0; y < data[i].length; y++) {
				buffer.writeUInt8(0x00, bufferIndex++);
				buffer.write(data[i][y], bufferIndex++, 1);
			}
		} else {
			buffer.writeUInt8(0x00, bufferIndex++);
			buffer.writeUInt8(data[i], bufferIndex++);
		}
	}
	
	if (callback == undefined) callback = function() {};

	this.socket.write(buffer, callback);

};

this.server.send([0x02, string.length, string]);

First of all, this code is inefficient as fuck. What can I do to make it faster? I mean, I've got an if statement inside a for loop, and a second for loop inside that. Twice.

Spoiler

Secondly, while this works fine, it adds some data at the end, for some reason, and I haven't got a clue where it comes from. This is what I'm expecting (same as above):

00000000  02 00 1e 00 66 00 69 00  72 00 65 00 63 00 61 00 ....f.i. r.e.c.a.
00000010  74 00 64 00 65 00 73 00  69 00 67 00 6e 00 73 00 t.d.e.s. i.g.n.s.
00000020  3b 00 6c 00 6f 00 63 00  61 00 6c 00 68 00 6f 00 ;.l.o.c. a.l.h.o.
00000030  73 00 74 00 3a 00 32 00  35 00 35 00 36 00 35    s.t.:.2. 5.5.6.5

But this is what I get:

00000000  02 00 1e 00 66 00 69 00  72 00 65 00 63 00 61 00 ....f.i. r.e.c.a.
00000010  74 00 64 00 65 00 73 00  69 00 67 00 6e 00 73 00 t.d.e.s. i.g.n.s.
00000020  3b 00 6c 00 6f 00 63 00  61 00 6c 00 68 00 6f 00 ;.l.o.c. a.l.h.o.
00000030  73 00 74 00 3a 00 32 00  35 00 35 00 36 00 35 00 s.t.:.2. 5.5.6.5.
00000040  01 f1 08                                         ...

Any ideas for that?

Edit: Okay, fixed that. Changed the +3 to -1, because the +3 didn't apply anymore. Any ideas for making the looping more efficient?

[Kyek]

Let's do this :D

	var length = 0;
	for (var i = 0; i < data.length; i++) {
		if (typeof data[i] != "number")
			for (var y = 0; y < data[i].length; y++)
				length++;
		else
			length++;
	}

Dude this is 100% crazy xD. Rewrite to:

var length = 0;
for (var i = 0, len = data.length; i < len; i++) {
    switch (typeof data[i]) {
        case 'string': length += data[i].length; break;
        case 'number': length++; break;
        // You can add additional types here if you ever need them
    }
}

Beyond that, the rest of your code is pretty sane. It seems crazy to have two nested loops like that, but when you have to iterate over every character of a string, it's a necessary evil. The only other thing you could do is a regex replace on each string, replacing

(.)

with

\0$1

. \0 is a "null" character (meaning it's 0x00), so with one line of code you'll successfully put a 0x00 before every character in the string. That way, you no longer need a loop -- you just write the string to the buffer. But your loop may actually be faster than that, so it's worth a benchmark!

[callumacrae]

var length = 0;
for (var i = 0, len = data.length; i < len; i++) {
    length += (typeof data[i] === 'number') ? 1 : data[i].length;
}

Kyek, your code is ugly!

EDIT:

Micro-optimisations suck.

var length = 0;
for (var i = 0; i < data.length; i++) {
    length += (typeof data[i] === 'number') ? 1 : data[i].length;
}

[Cyril]

Okay, I'll change that a bit later. Thanks Here goes another problem!
So, I managed to send a proper request this time, but instead of receiving back a 0x01, which means my login was accepted, I get a 0xFF -> I was kicked. So, that means one of my fields is invalid.

Here are the docs I'm working with: http://wiki.vg/Proto...uest_.280x01.29
(mind you, I still have to mess around quite a lot in wireshark just to understand what some values should be. Kyek, I know you love going straight into the bytes, but I'm just beginning A resource to work with is really helpful as well -- and don't worry, I'll do it "properly" some day as well )

And here's the line (the rest of the code is the same as above, haven't updated it yet):

this.server.send([0x01,                  // command
                  0x00,                  // uh, there was an extra NULL in the wiresharp dump, so...
                  0x1d,                  // protocol version (29)
                  this.name.length,      // username length
                  this.name,             // my username
                  0x00, 0x00, 0x00, 0x00, 0x00, 0x00    // unused
                 ]);

The wireshark dump looks fine, and corresponds to the official minecraft one, yet I get rejected :/
(Just in case someone wants to have a look, my full code: http://i.cntd.in/1N2r1x1i0i1Z1a1A3j1x)

[Kyek]

callumacrae, on 30 March 2012 - 01:05 PM, said:

var length = 0;
for (var i = 0, len = data.length; i < len; i++) {
    length += (typeof data[i] === 'number') ? 1 : data[i].length;
}

Kyek, your code is ugly!

You're assuming that, if the element isn't a number, it's a string. That's a big assumption, and it doesn't allow for easy maintenance or expansion.

Quote

EDIT:

Micro-optimisations suck.

var length = 0;
for (var i = 0; i < data.length; i++) {
    length += (typeof data[i] === 'number') ? 1 : data[i].length;
}

LOL, you're going to get on me for removing the length calculation from a loop (which has an easily measurable performance impact) but yet you use === instead of ==, whose performance impact is much less? xD

@Cyril: That *is* an interesting problem. You're right, I'd probably go straight to the bytes for this one to see what all's different and why, but if I get a chance later I'll look through the docs. Keep in mind that something else could be happening behind the scenes, too -- it could expect a secondary connection of some sort, even.

[callumacrae]

Kyek, on 30 March 2012 - 01:52 PM, said:

LOL, you're going to get on me for removing the length calculation from a loop (which has an easily measurable performance impact) but yet you use === instead of ==, whose performance impact is much less? xD

Coding standards, sorry ;-)

[Cyril]

The thing that's weird, is that my version and the official version both send exactly the same thing (I think the official client pads it with an extra NULL at the end, but I tried, and it made it error out -- took it out, no error.

Quote

If the version is outdated or any field is invalid, the server will disconnect the client with a kick.

I took the protocol version straight from the official client dump, so that can't be the problem.
Maybe it's because the different fields are of different types? It says that one is an empty string, two ints and unsigned bytes...

[Kyek]

Cyril, on 30 March 2012 - 02:04 PM, said:

The thing that's weird, is that my version and the official version both send exactly the same thing (I think the official client pads it with an extra NULL at the end, but I tried, and it made it error out -- took it out, no error.



I took the protocol version straight from the official client dump, so that can't be the problem.
Maybe it's because the different fields are of different types? It says that one is an empty string, two ints and unsigned bytes...

Types shouldn't matter if the hex matches. Call it whatever you want, but bits are bits. If they match, they match.

If you're diffing the client version with the bot version and getting equal results, the only thing that can differ in that scenario is time. If you're able, run the client while watching the wireshark output (or use wireshark to replay it in real time) and see if there are any delays that you don't have, or vice versa. I'd also normally say to pay attention to any additional incoming/outgoing connections (even UDP) during this stage, but I'd imagine the docs you're looking at would probably mention if that was a requirement.

[callumacrae]

I haven't really read most of this thread, but are you getting the EOLs right?

[Kyek]

Another thing to check -- sample the login process from the client a few times over the course of at least an hour, and compare them to each other. If something changes, then time is playing into it there as well.

[Cyril]

I've looked at the bytes enough, and the docs don't metion it, so I know there isn't any time related things, or external requests. The only difference between the two TCP dumps is that mine is missing one NULL byte at the end (like, on the last line, there are 15 bytes, not 16). However, if I add that extra byte manually, it errors out with a protocol error (server side).

[Kyek]

Any differences in the packet headers in Wireshark?

[Cyril]

Okay, so I'm off for today, so I'll test it out tomorrow, and tell you the result.

In the mean time, for people who don't understand what I'm planning for, this image should sum it up: