Query info packet 0 size strings fix

sampy/__main__.py

@@ -6,12 +6,15 @@ import textwrap
 
 def main(args: argparse.Namespace) -> int:
     from sampy.config import Config
+
+    config = Config(*args.config, logging_level=args.log)
+
     from sampy.network.protocol import Protocol
     from sampy.server import InteractiveServer
 
     server = InteractiveServer(
         Protocol,
-        config=Config(*args.config, logging_level=args.log),
+        config=config,
     )
     server.start()
 

sampy/network/query.py

@@ -9,11 +9,13 @@ if TYPE_CHECKING:
     from sampy.server import Server
 
 
+# Holds a dict of handlers for different packet IDs. Since every query packet id is a single byte, we use an int here.
 HANDLERS: Dict[int, Callable[[Server, bytes, Tuple[str, int]], bool]] = {}
 
 
-def handler(opcode: bytes):
-    if len(opcode) > 1:
+# Decorator that adds the function to the HANDLERS dict.
+def handler(packet_id: bytes):
+    if len(packet_id) > 1:
         raise Exception("Query opcode length cannot be bigger then 1")
 
     def outer(func):
@@ -23,14 +25,45 @@ def handler(opcode: bytes):
         ):
             return func(server, packet, addr, *args, **kwargs)
 
-        HANDLERS[opcode[0]] = func
-        logging.debug("Added query handler:", opcode, func)
+        HANDLERS[packet_id[0]] = func
+        logging.debug("Added Query handler: %s -> %s" % (packet_id, func))
         return inner
 
     return outer
 
 
 class Query:
+    """Query handler
+
+    Reference: https://team.sa-mp.com/wiki/Query_Mechanism.html (Unable to archive due to https://sa-mp.com/robots.txt disallowing ia_archiver)
+
+    The Query protocol is *mostly* used for the samp server browser and other systems that queries for server info.
+    The exception to this is would be the "player_details"(d) packet on version 0.2.x and below where an ingame player pressing TAB would use this packet.
+
+    Structure:
+        - "SAMP" header (4 bytes)
+        - Server's ipv4 (4 bytes)
+        - Server's port (2 bytes)
+        - Packet type (1 byte)
+        - Packet data
+    Note that the server and client will both use the same first 4 parts when communicating.
+    Not all packets have packet data, and most packets doesn't have data when the client sends it as its a "request".
+    The server will always have packet data attached.
+
+    List of packets:
+        - i: Information packet. This includes hostname, players (online/max), mode, language and whether password is required.
+        - r: Rules packet. This is a list of "rules". The name "rules" is subjective in this case, as most are general optional information.
+        - c: Client list packet. This is a list of players and scores. Players being just their username and score a number.
+        - d: Detailed player list packet. Extends the client list packet with player id and ping.
+        - p: Ping packet. A client uses this packet with 4 random bytes and measures how long it takes before it gets the same packet back.
+        - x: Remote console packet. A client can send and receive anything on this packet. Usually used for remote console.
+
+    Additional findings:
+        - On info packet, strings can not be of size 0. This will make parsing of the rest of the packet fail.
+            - This is strange due to how we are sending the length of the string first, which should allow this...
+            - Fix: If string size is 0, we replace the string with a single space (Considering using a NULL byte, but unsure if this could cause other issues)
+    """
+
     HEADER_LENGTH = 11
 
     @staticmethod
@@ -38,35 +71,42 @@ class Query:
         if len(packet) < 11:  # Packet is too small
             return False
 
-        magic, _ip, _port, opcode = struct.unpack(
+        magic, _ip, _port, packet_id = struct.unpack(
             b"<4sIHB", packet[: Query.HEADER_LENGTH]
         )  # Unpack packet
 
         if magic != b"SAMP":  # Validate magic
             return False
 
-        return HANDLERS.get(opcode, lambda *_: False)(server, packet, addr)
+        return HANDLERS.get(packet_id, lambda *_: False)(server, packet, addr)
 
     @handler(b"i")
     @staticmethod
     def info(server: Server, packet: bytes, addr: Tuple[str, int]) -> bool:
         packet = packet[: Query.HEADER_LENGTH]  # Discard additional data if passed
 
-        len_hostname = len(server.config.hostname)
-        len_mode = len(server.config.mode)
-        len_language = len(server.config.language)
+        hostname = server.config.hostname.encode()
+        mode = server.config.mode.encode()
+        language = server.config.language.encode()
+
+        if len(hostname) == 0:
+            hostname = b" "
+        if len(mode) == 0:
+            mode = b" "
+        if len(language) == 0:
+            language = b" "
 
         packet += struct.pack(
-            b"<?HHI%dsI%dsI%ds" % (len_hostname, len_mode, len_language),
+            b"<?HHI%dsI%dsI%ds" % (len(hostname), len(mode), len(language)),
             len(server.config.password) != 0,
             len(server.players),
             server.config.max_players,
-            len_hostname,
-            server.config.hostname.encode(),
-            len_mode,
-            server.config.mode.encode(),
-            len_language,
-            server.config.language.encode(),
+            len(hostname),
+            hostname,
+            len(mode),
+            mode,
+            len(language),
+            language,
         )
         server.sendto(packet, addr)
         return True