Fixed issue where outgoing packet limits would not be enforced, cleaned/added this info to the docs, and added some type hinting

Author: OwenCochell

.gitignore

@@ -23,6 +23,7 @@ __pycache__/
 # Python distribution archive directory/PyInstaller spec files
 
 dist/
+build/
 *.spec
 *.egg-info
 
@@ -34,3 +35,7 @@ dist/
 
 docs/build/
 docs/source/_build
+
+# General dummy files:
+
+test.py

README.md

@@ -116,6 +116,15 @@ if rcon.login("password"):
 
  Be sure to also check out the [mctools PyPi page](https://pypi.org/project/mctools/) for more information.
 
+ # Bug Reports
+
+ If you encounter a bug or any other event that does not seem normal,
+ then please open an issue, or email me personally.
+ I will be sure to get back to you as soon as possible.
+ 
+ Your feedback and reports are appreciated!
+ Your comments and issues are an excellent way to correct issues with mctools.
+
  # Contributing
 
  Pull requests are welcome and encouraged :) ! If you want to see a feature in mctools, a PR will be the quickest 
@@ -126,6 +135,18 @@ if rcon.login("password"):
 
  # Changelog
 
+ ## 1.1.2
+
+ Fixed an issue where RCON command size handling was broken.
+ Before, the remote server would kill the connection if a command is too large(Bigger than 1460 bytes).
+ We now raise a new exception, 'RCONLengthError' and refuse to send the packet if the command is too big,
+ thus keeping the connection alive. You can optionally disable this check, although this is not recommended.
+
+ The documentation has also been updated to make this limitation more clear,
+ as well as correcting some minor errors, mostly correcting examples in the formatting documentation.
+
+ We also added some type hinting, as some IDEs were complaining about type mismatches.
+
  ## 1.1.1
 
  Fixed an issue where clients hang when the connection is closed by the remote host.

docs/source/conf.py

@@ -12,17 +12,19 @@
 #
 import os
 import sys
+
 sys.path.insert(0, os.path.abspath('../..'))
 
+import mctools
 
 # -- Project information -----------------------------------------------------
 
 project = 'Minecraft-Connection-Tools'
-copyright = '2020, Owen Cochell'
+copyright = '2021, Owen Cochell'
 author = 'Owen Cochell'
 
 # The full version, including alpha/beta/rc tags
-release = '1.1.0'
+release = mctools.__version__
 
 
 # -- General configuration ---------------------------------------------------
@@ -58,7 +60,7 @@
                       'github_user': 'Owen-Cochell',
                       'github_repo': 'mctools',
                       'github_button': True,
-                      'github_type': 'watch',
+                      'github_type': 'star',
                       'extra_nav_links': {
                           'Github Page': 'https://github.com/Owen-Cochell/mctools',
                           'PyPi Page': 'https://pypi.org/project/mctools/',

docs/source/format.rst

@@ -146,22 +146,22 @@ Here is an example of a description in ChatObject notation:
 
         {'extra': [{'bold': True,
                     'color': 'yellow',
-                     'text': 'This is bold and yellow!'},
+                    'text': 'This is bold and yellow!'},
 
                     {'color': 'gold',
-                    'text': ' Just gold. New line!\n'},
+                     'text': ' Just gold. New line!\n'},
 
                     {'color': 'white',
-                    'italics': True,
-                    'text': 'We are on a new line, '},
+                     'italics': True,
+                     'text': 'We are on a new line, '},
 
                     {'color': 'green',
-                    'text': 'and we love the color green.'},
+                     'text': 'and we love the color green.'},
 
                     {'color': 'white',
-                    'text': '.'}],
+                     'text': '.'}]},
 
-        'text': ''},
+        'text': ''}
 
 As you can see, this makes reading and parsing the content difficult. ChatObjectFormatter fixes this problem
 by converting the dictionary into a single string, which makes reading and parsing the data much easier.
@@ -177,7 +177,7 @@ Have a look at this example sample player list:
 
 .. code-block:: python
 
-    'players': {'max': 5000,
+    {'players': {'max': 5000,
              'online': 723,
              'sample': [{'id': '00000000-0000-0000-0000-000000000000',
                          'name': 'We are a server.'},
@@ -194,7 +194,7 @@ Have a look at this example sample player list:
                         {'id': '00000000-0000-0000-0000-000000000000',
                          'name': 'Here is one of them:'},
                         {'id': '2ef8ad56-ec35-46e7-b90c-8172386d3fe7',
-                        'name': 'MinecraftPLayer1'}]}
+                        'name': 'MinecraftPLayer1'}]}}
 
 If you look, there is a message encoded in this sample list, with one valid player.
 The message has a null UUID, which is how SampleDescriptionFormatter determines if a user list is actually
@@ -204,11 +204,11 @@ After the formatting operation is complete, the *player* sub-dictionary will loo
 
 .. code-block:: python
 
-    'players': {'max': 5000,
+    {'players': {'max': 5000,
              'online': 723,
              'sample': [{'id': '2ef8ad56-ec35-46e7-b90c-8172386d3fe7',
                         'name': 'MinecraftPLayer1'}],
-             'message': 'We are a server.\n\nCheck out our Twitter!\n\nWe have really great players!\n\nHere is one of them:'}
+             'message': 'We are a server.\n\nCheck out our Twitter!\n\nWe have really great players!\n\nHere is one of them:'}}
 
 Now, the sample only contains valid players, and the message is stored under a separate key named 'message'.
 This allows us to accurately determine who is really playing, and view the message with no extra processing.
@@ -330,7 +330,7 @@ get
 This will return the list of formatters used by FormatterCollection. The information on a formatter is stored in a
 sublist with the following format:
 
-.. code-block:: python
+.. code-block::
 
     [Formatter Instance,
     Commands to match,

docs/source/mcli.rst

@@ -21,7 +21,7 @@ This section defines general usage for mcli.py
     However, if you downloaded the source code or the source distribution without installing,
     the you will call mcli as a python file:
 
-    .. code-block:: python
+    .. code-block::
 
         python3 mcli.py [arguments]
 
@@ -226,25 +226,25 @@ Screenshots
 
 Here are some screenshots of mcli in action:
 
-RCON Usage
-----------
+RCON
+----
 
 Interactive RCON session
 
 .. image:: rcon.png
     :alt: rcon screenshot
 
-Query usage
------------
+Query
+------
 
 Fetching full statistics via Query
 
 .. image:: query.png
     :alt: query screenshot
     :height: 300px
 
-Ping usage
-----------
+Ping 
+-----
 
 Pinging Minecraft server and fetching statistics
 

docs/source/ping.rst

@@ -47,7 +47,7 @@ You can do this like so:
 
     from mctools import PINGClient
 
-    ping = PINGClient('mc.server.net', proto_num=[PROTOCOL NUMBER])
+    ping = PINGClient('mc.server.net', proto_num=PROTOCOL_NUMBER)
 
 This allows PINGClient to emulate certain versions of the Minecraft client. By default, we use protocol number 0,
 which means we are inquiring on which protocol version we should use.
@@ -95,12 +95,12 @@ The contents of the dictionary *should* be in the following format:
         'online': 1,
         'sample': [
             {'id': 'fbf11fd0-5b74-490c-adc4-91febe9de2ae',
-            'name': 'MinecraftPlayer'}]},
-           'message': ''
+            'name': 'MinecraftPlayer'}],
+           'message': ''},
     'time': 0.09879999993245292,
     'version': {
         'name': '1.15.2',
-        'protocol': 578}
+        'protocol': 578},
     'favicon': 'data:image/png;base64,<data>'}
 
 The *description* field is the message of the day.

docs/source/query.rst

@@ -137,7 +137,7 @@ The *plugins* filed contains a list of plugins used.
 This is not used by the vanilla server, however community servers
 such as Bukkit and Spigot use this value. Most servers follow this format:
 
-.. code-block:: python
+.. code-block::
 
     [SERVER_MOD_NAME[: PLUGIN_NAME(; PLUGIN_NAME...)]]
 

docs/source/rcon.rst

@@ -89,7 +89,7 @@ The function will return the response in string format from the server, formatte
 
 .. note::
 
-    You can disable the authentication check by passing 'no_auth=True' to the command function, which will disable
+    You can disable the authentication check by passing 'no_auth=True'  to the command function, which will disable
     the check for this command. Be aware that if the server refuses to serve you, then a RCONAuthenticationError
     exception will be raised.
 
@@ -108,17 +108,58 @@ The command sent will broadcast "Hello RCON!" to every player on the server.
     when issued over RCON, so this is usually a normal operation. It can also mean that the server doesn't understand
     the command issued.
 
-RCON Packet Fragmentation
-_________________________
+RCON Outgoing Packet length
+___________________________
 
-Sometimes, the RCON server will send fragmented packets.
-This is because RCON has a maximum packet size of 4096 bytes.
-RCONClient will automatically handle packet fragmentation for you.
+The RCON Protocol has an outgoing(client to server) packet size limitation of 1460 bytes.
+Taking into account the mandatory information we have to send(request ID, type, padding, ect.),
+the maximum command size that can be sent is 1446 bytes.
+
+This limitation unfortunately has no workaround,
+and is an issue with the RCON protocol, and therefore beyond our control.
+mctools does implement a length check to make sure outgoing packets are not too big.
+
+If an outgoing packet is too big, and the length check is enabled,
+then an 'RCONLengthError' exception will be raised, and the packet will not be sent.
+This ensures that any nasty side effects of going over the outgoing limit will be avoided,
+thus keeping the connection in a stable state.
+
+You can optionally disable the outgoing length check by passing 'length_check=False' to the command method.
+
+.. warning::
+
+    Disabling outgoing length checks is not recommended! Doing so could mess up the state of your client!
+
+Here is an example of disabling outgoing length checks:
 
-If the packet received is 4096 bytes in length, then we will assume the packet is fragmented.
-We send a junk packet to the server, and read packets until the server acknowledges the junk packet.
-RCON ensures that all packets are sent in the order that they are received, meaning that once the server responds to
-the junk packet, then we can be sure that we have all of the relevant packets.
+.. code-block:: python
+
+    # Lets send a HUGE command:
+    # (Assume 'huge_command' is a string containing a command larger than 1446 bytes) 
+
+    resp = rcon.command(huge_command, length_check=False)
+
+This will prevent the 'RCONLengthError' exception from being raised,
+and mctools will send the large packet normally.
+
+If a large packet is sent to the RCON server, then some nasty things could occur.
+The most likely is that the server will forcefully close the connection,
+although other unsavory events could occur.
+This is why we recommend keeping the outgoing length check enabled.
+
+RCON Incoming Packet Fragmentation
+__________________________________
+
+Sometimes, the RCON server will send fragmented packets.
+This is because RCON has an incoming(server to client) maximum packet size of 4096 bytes.
+RCONClient will automatically handle incoming packet fragmentation for you.
+
+If the incoming packet is 4096 bytes in length, then we will assume the packet is fragmented.
+If this is the case, then mctools sends a junk packet to the server, 
+and reads packets until the server acknowledges the junk packet.
+The RCON protocol ensures that all packets are sent in the order that they are received, 
+meaning that once the server responds to the junk packet, 
+then we can be sure that we have all of the relevant packets.
 We then concatenate the packets we received, and return it as one.
 
 However, you can disable the check by passing 'frag_check=False' to the command method.
@@ -135,7 +176,7 @@ Here is an example of disabling RCON packet fragmentation:
 
     resp = rcon.command("help", frag_check=False)
 
-This will return the content of the first 4096 bytes. Any subsequent to 'command' or 'raw_send' will return the rest
+This will return the content of the first 4096 bytes. Any subsequent call to 'command' or 'raw_send' will return the rest
 of the fragmented packets. This means that you will have incomplete content, and subsequent calls will return
 irrelevant information. Unless you have a reason for this, it is recommended to keep packet fragmentation enabled.
 

mctools/__init__.py

@@ -4,5 +4,5 @@
 
 # Define some metadata here:
 
-__version__ = '1.1.1'
+__version__ = '1.1.2'
 __author__ = 'Owen Cochell'

mctools/encoding.py

@@ -6,7 +6,9 @@
 import struct
 import json
 
-from mctools.errors import RCONMalformedPacketError
+from typing import Tuple, Union
+
+from mctools.errors import RCONMalformedPacketError, PINGMalformedPacketError
 
 
 MAP = {''}
@@ -326,7 +328,7 @@ def encode(data):
         return byts
 
     @staticmethod
-    def decode(byts):
+    def decode(byts) -> Tuple[dict, None, str]:
 
         """
         Decodes a ping packet.
@@ -355,19 +357,22 @@ def decode(byts):
 
             # Working with some other packet type:
 
-            return None, None, "pong"
+            return {}, None, "pong"
+
+        raise PINGMalformedPacketError("Invalid packet type! Must be 1 or 0, not {}!".format(pack_type))
 
     @staticmethod
-    def encode_varint(num):
+    def encode_varint(num: int) -> bytes:
 
         """
         Encodes a number into varint bytes.
 
         :param num: Number to encode
+        :type num: int
         :return: Bytes
         """
 
-        byts = b''
+        byts: bytes = b''
         remaining = num
 
         # Iterating over the content, and doing the necessary bitwise stuff
@@ -378,14 +383,16 @@ def encode_varint(num):
 
                 # Found our stuff, exit
 
-                byts = byts + struct.pack("!B", remaining)
-                return byts
+                break
 
             byts = byts + struct.pack("!B", remaining & 0x7F | 0x80)
             remaining >>= 7
 
+        byts = byts + struct.pack("!B", remaining)
+        return byts
+
     @staticmethod
-    def decode_varint(byts):
+    def decode_varint(byts) -> Tuple[int, int]:
 
         """
         Decodes varint bytes into an integer.
@@ -415,7 +422,7 @@ def decode_varint(byts):
         return result, b + 1
 
     @staticmethod
-    def decode_sock(sock):
+    def decode_sock(sock) -> int:
 
         """
         Decodes a var(int/long) of variable length or value.