FE legacy ping support

[deathcap]

src/ping.js implements a server list ping using ping and ping_start in the STATUS protocol state, but there is another type of ping initiated by sending the bytes 0xfe 0x01. Sometimes known as the "legacy" ping, but it supported by modern versions of Minecraft, including 1.8.9 and the 1.9 snapshots: http://wiki.vg/Protocol#Legacy_Server_List_Ping

While not technically part of the current protocol, legacy clients may send this packet to initiate Server List Ping, and modern servers should handle it correctly.

node-minecraft-protocol should support FE01 ping, in both the server and client. Server is especially important since third-party ping/status software may still send FE01 pings, due to simplicity and widespread support (vanilla servers support it). Client support would be useful when pinging servers of an unknown version, including those with the modern Netty protocol, or earlier versions (all the way back to Minecraft 1.4.4 release is supported by the FE01 ping).

Example of fe01 ping in nodejs: https://github.com/deathcap/mcping16/blob/master/mcping16.js#L124 - but needs to be cleaned up to use protodef, etc.

[deathcap]

This packet is supported for serialization/deserialization by https://github.com/PrismarineJS/minecraft-data/blob/master/data/1.8/protocol.json#L148:

        "legacy_server_list_ping": {
          "id": "0xfe",
          "fields": [
            {
              "name": "payload",
              "type": "ubyte"
            }
          ]
        }

but nothing currently reads/writes it

[roblabla]

That probably doesn't work. It needs to be sent without the length-prefix and with a byte ID instead of varint ID, no ?

[deathcap]

Yeah I think needs to be special-cased somehow

[rom1504]

Yeah maybe just with a simple if (packet.name=="legacy_server_list_ping"){
/* write directly to this.socket */ } in Client.write

On Wed, Jan 27, 2016, 05:23 deathcap [email protected] wrote:

Yeah I think needs to be special-cased somehow

—
Reply to this email directly or view it on GitHub
#332 (comment)
.

[rom1504]

Ah, not sure how that should be handled by the parsing pipeline though

On Wed, Jan 27, 2016, 08:55 Romain Beaumont [email protected] wrote:

Yeah maybe just with a simple if (packet.name=="legacy_server_list_ping"){
/* write directly to this.socket */ } in Client.write

On Wed, Jan 27, 2016, 05:23 deathcap [email protected] wrote:

Yeah I think needs to be special-cased somehow

—
Reply to this email directly or view it on GitHub
#332 (comment)
.

[deathcap]

Researching the various pings in https://github.com/deathcap/node-minecraft-ping, details in the repo but overall summary of server support:

Minecraft Version	ping_fe01fa	ping_fe01	ping_fe	Netty status ping(*)
1.9	YES	YES	Limited	YES
1.8.9	YES	YES	Limited	YES
1.7.10	YES	YES	Limited	YES
1.6.4	YES	Slow	Limited, Slow	NO
1.5.2	YES	YES	Limited, Slow	NO
1.4.4	YES	YES	Limited, Slow	NO
1.3.2	NO	Limited	Limited	NO
1.2.5	NO	Limited	Limited	NO
earlier	NO	maybe	probably	NO

(*) As implemented in node-minecraft-protocol src/ping.js

(**) Limited = responds but does not return the game/protocol version

What I call ping_fe01fa() (that is, FE01 + FA MC|PingHost) seems to be the best overall, as far as client pinging goes. For the server, just would need to handle the 0xfe "packet" and reply accordingly (technically, check if the next byte is 0x01, if so include the game/protocol version (ping type 1), otherwise return only the motd, online players, max players (ping type 0 - ping_fe/limited), but that's a minor point).

[deathcap]

Released https://www.npmjs.com/package/minecraft-ping, but it is only for the client-side ping.

For server in node-minecraft-protocol, currently gets stuck in the splitter transform. I think we'll need to do something like this:

diff --git a/src/transforms/framing.js b/src/transforms/framing.js
index a4fb0f7..a447137 100644
--- a/src/transforms/framing.js
+++ b/src/transforms/framing.js
@@ -30,6 +30,13 @@ class Splitter extends Transform {
   }
   _transform(chunk, enc, cb) {
     this.buffer = Buffer.concat([this.buffer, chunk]);
+
+    if (this.buffer[0] === 0xfe) {
+      // legacy_server_list_ping packet follows a different protocol format, no varint length
+      this.push(this.buffer);
+      return cb();
+    }
+
     var offset = 0;

     var { value, size, error } = readVarInt(this.buffer, offset) || { error: "Not enough data" };

this gets the legacy server list ping to the deserializer, which recognizes it:

MC-PROTO: 36906 read packet handshaking.legacy_server_list_ping
MC-PROTO: 36906 { payload: 250 }

but it will need to be handled. Also, for par with vanilla, deserialization should be able to read:

• fe
• fe01
• fe01fa...MC|PingHost

Currently it can read the last two, but not the first:

MC-PROTO: 37154 read packet handshaking.legacy_server_list_ping
MC-PROTO: 37154 { payload: 250 }
_transform <Buffer fe>
parsePacketBuffer <Buffer fe>
events.js:141
      throw er; // Unhandled 'error' event
      ^

Error: Deserialization error for handshaking.toServer : Read error for name : Reader returned null : {"type":"varint"}
    at ProtoDef.read (/Users/admin/games/voxeljs/ProtoDef/dist/protodef.js:109:15)
    at ProtoDef.readMapper (/Users/admin/games/voxeljs/ProtoDef/dist/datatypes/utils.js:27:20)
    at ProtoDef.read (/Users/admin/games/voxeljs/ProtoDef/dist/protodef.js:107:42)
    at /Users/admin/games/voxeljs/ProtoDef/dist/datatypes/structures.js:114:32
    at tryCatch (/Users/admin/games/voxeljs/ProtoDef/dist/utils.js:33:12)
    at tryDoc (/Users/admin/games/voxeljs/ProtoDef/dist/utils.js:40:10)
    at /Users/admin/games/voxeljs/ProtoDef/dist/datatypes/structures.js:113:5
    at Array.forEach (native)
    at ProtoDef.readContainer (/Users/admin/games/voxeljs/ProtoDef/dist/datatypes/structures.js:108:12)
    at ProtoDef.read (/Users/admin/games/voxeljs/ProtoDef/dist/protodef.js:46:25)

or

Error: Deserialization error for handshaking.toServer.legacy_server_list_ping.payload : Read error for params.legacy_server_list_ping.payload : Reader returned null : {"type":"ubyte"}

If only 0xfe is received, then the server should (or at least vanilla servers do) return the "ping 0" response, example:

00000000  fe                                               .
    00000000  ff 00 17 00 41 00 20 00  4d 00 69 00 6e 00 65 00 ....A. . M.i.n.e.
    00000010  63 00 72 00 61 00 66 00  74 00 20 00 53 00 65 00 c.r.a.f. t. .S.e.
    00000020  72 00 76 00 65 00 72 00  a7 00 30 00 a7 00 32 00 r.v.e.r. ..0...2.
    00000030  30                                               0

0xff (kick) + 2-byte length + UCS-2 string: motd + \xa7 + current players (decimal string) + \xa7 + max players (decimal string)

If 0xfe is received followed by 0x01 (and then optionally anything else; 1.6.4 sends some MC|PingHost junk, but it does not matter), then the server should send the "ping 1" response:

00000000  fe 01                                            ..
    00000000  ff 00 25 00 a7 00 31 00  00 00 31 00 32 00 37 00 ..%...1. ..1.2.7.
    00000010  00 00 31 00 36 00 77 00  30 00 33 00 61 00 00 00 ..1.6.w. 0.3.a...
    00000020  41 00 20 00 4d 00 69 00  6e 00 65 00 63 00 72 00 A. .M.i. n.e.c.r.
    00000030  61 00 66 00 74 00 20 00  53 00 65 00 72 00 76 00 a.f.t. . S.e.r.v.
    00000040  65 00 72 00 00 00 30 00  00 00 32 00 30          e.r...0. ..2.0

Same 0xff (kick) + 2-byte length + UCS-2 string format, but it begins with \xa7 + digit '1' and has \0-delimited fields:

    if (string[0] == '\xa7') {
      const parts = string.split('\0');
      result.pingVersion = parseInt(parts[0].slice(1));
      result.protocolVersion = parseInt(parts[1]);
      result.gameVersion = parts[2];
      result.motd = parts[3];
      result.playersOnline = parseInt(parts[4]);

[deathcap]

The splitter is easy enough to special-case for 0xfe, but not sure how to special-case in the deserializer.

Currently, 0xfe 0x01 decodes to varint 254, and 0xfe is an incomplete varint. Should legacy ping support be added somewhere in here?

// src/transforms/serializer.js createProtocol
  proto.addType("packet",["container", [
    { "name": "name", "type":["mapper",{"type": "varint" ,
      "mappings":Object.keys(packets).reduce(function(acc,name){
        acc[parseInt(packets[name].id)]=name;
        return acc;
      },{})
    }]},
    { "name": "params", "type": ["switch", {
      "compareTo": "name",
      "fields": Object.keys(packets).reduce(function(acc,name){
        acc[name]="packet_"+name;
        return acc;
      },{})
    }]}
  ]]);

but only for the handshaking state. Tried to add to minecraft-data/data/1.8/protocol.json states > handshaking, but that file does not include the packet length varint, it is defined here in src/transforms/serializer.js, the "packet" data type.

The difficulty is that the first few bytes of the packet can either be a varint length, or a packet identifier byte.. can protodef express this? I know it can switch on another field, but can the field have two different types depending on its value? (if 0xfe, then read rest of bytes as the payload; if anything else, read as a varint length, continue parsing).

It would be be easiest if 0xfe legacy ping handling could bypass the deserializer, but I can't see how to do this.

[rom1504]

It seems the vanilla client uses the legacy ping when trying to ping a server with an other version.

[rom1504]

https://hastebin.com/icawelacec.swift