Decode status results into their own object for accessibility & validation

mcstatus/pinger.py

@@ -1,5 +1,7 @@
 import datetime
+import json
 import random
+from six import string_types
 
 from mcstatus.protocol.connection import Connection
 
@@ -31,8 +33,16 @@ class ServerPinger:
 
         response = self.connection.read_buffer()
         if response.read_varint() != 0:
-            raise IOError("Received invalid query response packet.")
+            raise IOError("Received invalid status response packet.")
-        return response.read_utf()
+        try:
+            raw = json.loads(response.read_utf())
+        except ValueError:
+            raise IOError("Received invalid JSON")
+        try:
+            return PingResponse(raw)
+        except ValueError as e:
+            raise IOError("Received invalid status response: %s" % e)
+
 
     def test_ping(self):
         request = Connection()
@@ -47,8 +57,82 @@ class ServerPinger:
             raise IOError("Received invalid ping response packet.")
         received_token = response.read_long()
         if received_token != self.ping_token:
-            raise IOError("Received mangled ping response packet (expected token %d, received %d)" % (self.ping_token, received_token))
+            raise IOError("Received mangled ping response packet (expected token %d, received %d)" % (
+                self.ping_token, received_token))
 
         delta = (received - sent)
         # We have no trivial way of getting a time delta :(
-        return (delta.days * 24 * 60 * 60 + delta.seconds) * 1000 + delta.microseconds / 1000.0
+        return (delta.days * 24 * 60 * 60 + delta.seconds) * 1000 + delta.microseconds / 1000.0
+
+
+class PingResponse:
+    class Players:
+        class Player:
+            def __init__(self, raw):
+                if type(raw) is not dict:
+                    raise ValueError("Invalid player object (expected dict, found %s" % type(raw))
+
+                if "name" not in raw:
+                    raise ValueError("Invalid player object (no 'name' value)")
+                if not isinstance(raw["name"], string_types):
+                    raise ValueError("Invalid player object (expected 'name' to be str, was %s)" % type(raw["name"]))
+                self.name = raw["name"]
+
+                if "id" not in raw:
+                    raise ValueError("Invalid player object (no 'id' value)")
+                if not isinstance(raw["id"], string_types):
+                    raise ValueError("Invalid player object (expected 'id' to be str, was %s)" % type(raw["id"]))
+                self.id = raw["id"]
+
+        def __init__(self, raw):
+            if type(raw) is not dict:
+                raise ValueError("Invalid players object (expected dict, found %s" % type(raw))
+
+            if "online" not in raw:
+                raise ValueError("Invalid players object (no 'online' value)")
+            if type(raw["online"]) is not int:
+                raise ValueError("Invalid players object (expected 'online' to be int, was %s)" % type(raw["online"]))
+            self.online = raw["online"]
+
+            if "max" not in raw:
+                raise ValueError("Invalid players object (no 'max' value)")
+            if type(raw["max"]) is not int:
+                raise ValueError("Invalid players object (expected 'max' to be int, was %s)" % type(raw["max"]))
+            self.max = raw["max"]
+
+            if "sample" in raw:
+                if type(raw["sample"]) is not list:
+                    raise ValueError("Invalid players object (expected 'sample' to be list, was %s)" % type(raw["max"]))
+                self.sample = raw["sample"]
+
+    class Version:
+        def __init__(self, raw):
+            if type(raw) is not dict:
+                raise ValueError("Invalid version object (expected dict, found %s" % type(raw))
+
+            if "name" not in raw:
+                raise ValueError("Invalid version object (no 'name' value)")
+            if not isinstance(raw["name"], string_types):
+                raise ValueError("Invalid version object (expected 'name' to be str, was %s)" % type(raw["name"]))
+            self.name = raw["name"]
+
+            if "protocol" not in raw:
+                raise ValueError("Invalid version object (no 'protocol' value)")
+            if type(raw["protocol"]) is not int:
+                raise ValueError("Invalid version object (expected 'protocol' to be int, was %s)" % type(raw["protocol"]))
+            self.protocol = raw["protocol"]
+
+    def __init__(self, raw):
+        self.raw = raw
+
+        if "players" not in raw:
+            raise ValueError("Invalid status object (no 'players' value)")
+        self.players = PingResponse.Players(raw["players"])
+
+        if "version" not in raw:
+            raise ValueError("Invalid status object (no 'version' value)")
+        self.version = PingResponse.Version(raw["version"])
+
+        if "description" not in raw:
+            raise ValueError("Invalid status object (no 'description' value)")
+        self.description = raw["description"]

mcstatus/protocol/connection.py

@@ -30,6 +30,12 @@ class Connection:
         self.sent = ""
         return result
 
+    def _unpack(self, format, data):
+        return struct.unpack(">" + format, bytes(data))[0]
+
+    def _pack(self, format, data):
+        return struct.pack(">" + format, data)
+
     def read_varint(self):
         result = 0
         for i in range(5):
@@ -58,28 +64,28 @@ class Connection:
         self.write(bytearray(value, 'utf8'))
 
     def read_short(self):
-        return struct.unpack(">h", self.read(2))[0]
+        return self._unpack("h", self.read(2))
 
     def write_short(self, value):
-        self.write(struct.pack(">h", value))
+        self.write(self._pack("h", value))
 
     def read_ushort(self):
-        return struct.unpack(">H", self.read(2))[0]
+        return self._unpack("H", self.read(2))
 
     def write_ushort(self, value):
-        self.write(struct.pack(">H", value))
+        self.write(self._pack("H", value))
 
     def read_long(self):
-        return struct.unpack(">q", self.read(8))[0]
+        return self._unpack("q", self.read(8))
 
     def write_long(self, value):
-        self.write(struct.pack(">q", value))
+        self.write(self._pack("q", value))
 
     def read_ulong(self):
-        return struct.unpack(">Q", self.read(8))[0]
+        return self._unpack("Q", self.read(8))
 
     def write_ulong(self, value):
-        self.write(struct.pack(">Q", value))
+        self.write(self._pack("Q", value))
 
     def read_buffer(self):
         length = self.read_varint()

mcstatus/server.py

@@ -13,10 +13,4 @@ class MinecraftServer:
         connection = TCPSocketConnection((self.host, self.port))
         pinger = ServerPinger(connection, host=self.host, port=self.port, **kwargs)
         pinger.handshake()
-        try:
+        return pinger.read_status(), pinger.test_ping()
-            return {
-                "status": json.loads(pinger.read_status()),
-                "latency": pinger.test_ping(),
-            }
-        except ValueError:
-            raise IOError("The server responded with invalid json")

mcstatus/tests/protocol/test_pinger.py

@@ -1,7 +1,8 @@
 from unittest import TestCase
 
 from mcstatus.protocol.connection import Connection
-from mcstatus.pinger import ServerPinger
+from mcstatus.pinger import ServerPinger, PingResponse
+from mcstatus.server import MinecraftServer
 
 
 class TestServerPinger(TestCase):
@@ -15,10 +16,15 @@ class TestServerPinger(TestCase):
 
     def test_read_status(self):
         self.pinger.connection.receive(bytearray.fromhex("7200707B226465736372697074696F6E223A2241204D696E65637261667420536572766572222C22706C6179657273223A7B226D6178223A32302C226F6E6C696E65223A307D2C2276657273696F6E223A7B226E616D65223A22312E382D70726531222C2270726F746F636F6C223A34347D7D"))
+        status = self.pinger.read_status()
 
-        self.assertEqual(self.pinger.read_status(), '{"description":"A Minecraft Server","players":{"max":20,"online":0},"version":{"name":"1.8-pre1","protocol":44}}')
+        self.assertEqual(status.raw, {"description":"A Minecraft Server","players":{"max":20,"online":0},"version":{"name":"1.8-pre1","protocol":44}})
         self.assertEqual(self.pinger.connection.flush(), bytearray.fromhex("0100"))
 
+    def test_read_status_invalid_json(self):
+        self.pinger.connection.receive(bytearray.fromhex("6D006B7C226465736372697074696F6E223A2241204D696E65637261667420536572766572222C22706C6179657273223A7B226D6178223A32302C226F6E6C696E65223A307D2C2276657273696F6E223A7B226E616D65223A22312E38222C2270726F746F636F6C223A34377D7D09010000000001C54246"))
+        self.assertRaises(IOError, self.pinger.read_status)
+
     def test_read_status_invalid(self):
         self.pinger.connection.receive(bytearray.fromhex("0105"))
 
@@ -41,4 +47,116 @@ class TestServerPinger(TestCase):
         self.pinger.connection.receive(bytearray.fromhex("09010000000000DD7D1C"))
         self.pinger.ping_token = 12345
 
-        self.assertRaises(IOError, self.pinger.test_ping)
+        self.assertRaises(IOError, self.pinger.test_ping)
+
+
+class TestPingResponse(TestCase):
+    def test_raw(self):
+        response = PingResponse({"description":"A Minecraft Server","players":{"max":20,"online":0},"version":{"name":"1.8-pre1","protocol":44}})
+
+        self.assertEqual(response.raw, {"description":"A Minecraft Server","players":{"max":20,"online":0},"version":{"name":"1.8-pre1","protocol":44}})
+
+    def test_description(self):
+        response = PingResponse({"description":"A Minecraft Server","players":{"max":20,"online":0},"version":{"name":"1.8-pre1","protocol":44}})
+
+        self.assertEqual(response.description, "A Minecraft Server")
+
+    def test_version(self):
+        response = PingResponse({"description":"A Minecraft Server","players":{"max":20,"online":0},"version":{"name":"1.8-pre1","protocol":44}})
+
+        self.assertIsNotNone(response.version)
+        self.assertEqual(response.version.name, "1.8-pre1")
+        self.assertEqual(response.version.protocol, 44)
+
+    def test_version_missing(self):
+        self.assertRaises(ValueError, PingResponse, {"description":"A Minecraft Server","players":{"max":20,"online":0}})
+
+    def test_version_invalid(self):
+        self.assertRaises(ValueError, PingResponse, {"description":"A Minecraft Server","players":{"max":20,"online":0},"version":"foo"})
+
+    def test_players(self):
+        response = PingResponse({"description":"A Minecraft Server","players":{"max":20,"online":5},"version":{"name":"1.8-pre1","protocol":44}})
+
+        self.assertIsNotNone(response.players)
+        self.assertEqual(response.players.max, 20)
+        self.assertEqual(response.players.online, 5)
+
+    def test_players_missing(self):
+        self.assertRaises(ValueError, PingResponse, {"description":"A Minecraft Server","version":{"name":"1.8-pre1","protocol":44}})
+
+
+class TestPingResponsePlayers(TestCase):
+    def test_invalid(self):
+        self.assertRaises(ValueError, PingResponse.Players, "foo")
+
+    def test_max_missing(self):
+        self.assertRaises(ValueError, PingResponse.Players, {"online":5})
+
+    def test_max_invalid(self):
+        self.assertRaises(ValueError, PingResponse.Players, {"max":"foo","online":5})
+
+    def test_online_missing(self):
+        self.assertRaises(ValueError, PingResponse.Players, {"max":20})
+
+    def test_online_invalid(self):
+        self.assertRaises(ValueError, PingResponse.Players, {"max":20,"online":"foo"})
+
+    def test_valid(self):
+        players = PingResponse.Players({"max":20,"online":5})
+
+        self.assertEqual(players.max, 20)
+        self.assertEqual(players.online, 5)
+
+    def test_sample(self):
+        players = PingResponse.Players({"max":20,"online":1,"sample":[{"name":'Dinnerbone','id':"61699b2e-d327-4a01-9f1e-0ea8c3f06bc6"}]})
+
+        self.assertIsNotNone(players.sample)
+
+    def test_sample_invalid(self):
+        self.assertRaises(ValueError, PingResponse.Players, {"max":20,"online":1,"sample":"foo"})
+
+
+class TestPingResponsePlayersPlayer(TestCase):
+    def test_invalid(self):
+        self.assertRaises(ValueError, PingResponse.Players.Player, "foo")
+
+    def test_name_missing(self):
+        self.assertRaises(ValueError, PingResponse.Players.Player, {"id":"61699b2e-d327-4a01-9f1e-0ea8c3f06bc6"})
+
+    def test_name_invalid(self):
+        self.assertRaises(ValueError, PingResponse.Players.Player, {"name":{},"id":"61699b2e-d327-4a01-9f1e-0ea8c3f06bc6"})
+
+    def test_id_missing(self):
+        self.assertRaises(ValueError, PingResponse.Players.Player, {"name":"Dinnerbone"})
+
+    def test_id_invalid(self):
+        self.assertRaises(ValueError, PingResponse.Players.Player, {"name":"Dinnerbone","id":{}})
+
+    def test_valid(self):
+        player = PingResponse.Players.Player({"name":'Dinnerbone','id':"61699b2e-d327-4a01-9f1e-0ea8c3f06bc6"})
+
+        self.assertEqual(player.name, "Dinnerbone")
+        self.assertEqual(player.id, "61699b2e-d327-4a01-9f1e-0ea8c3f06bc6")
+
+
+class TestPingResponseVersion(TestCase):
+    def test_invalid(self):
+        self.assertRaises(ValueError, PingResponse.Version, "foo")
+
+    def test_protocol_missing(self):
+        self.assertRaises(ValueError, PingResponse.Version, {"name":"foo"})
+
+    def test_protocol_invalid(self):
+        self.assertRaises(ValueError, PingResponse.Version, {"name":"foo","protocol":"bar"})
+
+    def test_name_missing(self):
+        self.assertRaises(ValueError, PingResponse.Version, {"protocol":5})
+
+    def test_name_invalid(self):
+        self.assertRaises(ValueError, PingResponse.Version, {"name":{},"protocol":5})
+
+    def test_valid(self):
+        players = PingResponse.Version({"name":"foo","protocol":5})
+
+        self.assertEqual(players.name, "foo")
+        self.assertEqual(players.protocol, 5)

mcstatus/tests/test_server.py

@@ -16,16 +16,9 @@ class TestMinecraftServer(TestCase):
 
         with patch("mcstatus.server.TCPSocketConnection") as connection:
             connection.return_value = self.socket
-            info = self.server.ping_server(ping_token=29704774, version=47)
+            info, latency = self.server.ping_server(ping_token=29704774, version=47)
 
         self.assertEqual(self.socket.flush(), bytearray.fromhex("0F002F096C6F63616C686F737463DD01010009010000000001C54246"))
         self.assertEqual(self.socket.remaining(), 0, msg="Data is pending to be read, but should be empty")
-        self.assertEqual(info["status"], {"description":"A Minecraft Server","players":{"max":20,"online":0},"version":{"name":"1.8","protocol":47}})
+        self.assertEqual(info.raw, {"description":"A Minecraft Server","players":{"max":20,"online":0},"version":{"name":"1.8","protocol":47}})
-        self.assertTrue(info["latency"] >= 0)
+        self.assertTrue(latency >= 0)
-
-    def test_ping_server_invalid_json(self):
-        self.socket.receive(bytearray.fromhex("6D006B7C226465736372697074696F6E223A2241204D696E65637261667420536572766572222C22706C6179657273223A7B226D6178223A32302C226F6E6C696E65223A307D2C2276657273696F6E223A7B226E616D65223A22312E38222C2270726F746F636F6C223A34377D7D09010000000001C54246"))
-
-        with patch("mcstatus.server.TCPSocketConnection") as connection:
-            connection.return_value = self.socket
-            self.assertRaises(IOError, self.server.ping_server, ping_token=29704774, version=47)

requirements.txt

@@ -0,0 +1 @@
+six==1.7.3

setup.py

@@ -8,5 +8,6 @@ setup(
     url='https://pypi.python.org/pypi/mcstatus',
     packages=['minecraft_query',],
     description='A library to query Minecraft Servers for their status and capabilities.',
+    install_requires=['six'],
     tests_require=['mock'],
 )