diff --git a/.coverage b/.coverage new file mode 100644 index 0000000..2770929 Binary files /dev/null and b/.coverage differ diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..b2b8866 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,7 @@ +{ + "python.testing.pytestArgs": [ + "test" + ], + "python.testing.unittestEnabled": false, + "python.testing.pytestEnabled": true +} \ No newline at end of file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cb2c0fd..dd4b587 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -6,12 +6,11 @@ here on GitHub. Please note we have a code of conduct, please follow it in all your interactions with the project. ## Pull Request Process +** All PRs must merge into the dev branch, and not into master**. The dev branch is the only one that can merge into master directly. 1. Ensure any install or build dependencies are removed before the end of the layer when doing a build. -2. Update the README.md with details of changes to the interface, this includes new environment +2. Make sure all corresponding test cases pass. +3. Update the README.md with details of changes to the interface, this includes new environment variables, exposed ports, useful file locations and container parameters. -3. Increase the version numbers in any examples files and the README.md to the new version that this - Pull Request would represent. The versioning scheme we use is [SemVer](http://semver.org/). -4. You may merge the Pull Request in once you have the sign-off of two other developers, or if you - do not have permission to do that, you may request the second reviewer to merge it for you. +4. Version number will be increased only when merging dev into master, so that a version update may carry multiple PRs from various developers. The versioning scheme we use is [SemVer](http://semver.org/). diff --git a/README.md b/README.md index 8692a31..0101415 100644 --- a/README.md +++ b/README.md @@ -71,3 +71,91 @@ which payloads to send to the remote device. For that, we have several classes i `payload_provider` module. You may want to create your own provider by extending `payload_provider.PayloadProvider`. If you are interested in that, you should check the documentation of both `executor` and `payload_provider` module. + +## Code Structure + +### Top Level Directory Layout +Our project directory structure contains all src files in the pythonping folder, test cases in another folder, and helping documentation in on the top level directory. + +``` +. +├── pythonping # Source files +├── test # Automated Testcases for the package +├── CODE_OF_CONDUCT # An md file containing code of conduct +├── CONTRIBUTING # Contributing Guidlins +├── LICENSE # MIT License +├── README.md # An md file +└── setup.py # Instalation +``` + +A UML Diagram of the code structure is below: + +![ER1](https://raw.githubusercontent.com/alessandromaggio/pythonping/master/docs/UML-Diagram.png) + +As per the uml diagram above five distinct classes outside of init exist in this package: Executor, Icmp, Payload Provider, and Utils. Each of them rely on attributes which have been listed as sub-classes for brevities sake. An overview of each class is as follows. + +### Utils +Simply generates random text. See function random_text. + +### Network +Opens a socket to send and recive data. See functions send, recv, and del. + +### Payload Provider +Generates ICMP Payloads with no Headers. It's functionaly a interface. It has three +functions init, iter, and next, which are all implmented by subclasses List, Repeat, and Sweep which store payloads in diffrent lists. + +### ICMP +Generates the ICMP heaser through subclass ICMPType, and various helper functions. + +### Executor +Has various subclasses including Message, Response, Success, and Communicator used for sending icmp packets and collecting data. + +### Init +Uses network, executor, payload_provider and utils.random_text to construct and send ICMP packets to ping a network. + +## Tests +A test package exists under the folder test, and contains a serise of unit tests. Before commiting changes make sure to run the test bench and make sure all corrisponding cases pass. For new functionality new test cases must be added and documented. + +To run testcases we can simply use the ```unitest discover``` utility by running the following command: + +``` +python -m unittest discover +``` + +To run the test cases in a specific file FILE we must run the following command: + +``` +python -m unittest discover -s -p FILE +``` + +Another option is to run the following from the top level directory: + +``` +pytest test +``` + +To test for coverage simply run: + +``` +coverage run -m pytest test +``` + +## Contributing +Before contributing read through the contribution guidlines found the CONTRIBUTING file. + +### Code Style +A few key points when contributing to this repo are as follows: +1. Use tabs over spaces. +2. Format doc strings as such: + ``` + DESCRIPTION + + :param X: DESCRIPTION + :type X: Type + :param Y: DESCRIPTION + :type Y: Type + ``` + Please add doc strings to all functions added. +3. Do not add spaces between docstring and first function line. +4. Do not go over 200 characters per line. +5. When closing multiline items under brackets('()', '[]', ... etc) put the closing bracket on it's own line. \ No newline at end of file diff --git a/docs/UML-Diagram.png b/docs/UML-Diagram.png new file mode 100644 index 0000000..6766f5f Binary files /dev/null and b/docs/UML-Diagram.png differ diff --git a/pythonping/__init__.py b/pythonping/__init__.py index 78cfecb..134a131 100644 --- a/pythonping/__init__.py +++ b/pythonping/__init__.py @@ -1,7 +1,7 @@ import sys +from random import randint from . import network, executor, payload_provider from .utils import random_text -from random import randint # this needs to be available across all thread usages and will hold ints @@ -68,21 +68,24 @@ def ping(target, if df: options = network.Socket.DONT_FRAGMENT - # Fix to allow for pythonping multithreaded usage; - # no need to protect this loop as no one will ever surpass 0xFFFF amount of threads - while True: - # seed_id needs to be less than or equal to 65535 (as original code was seed_id = getpid() & 0xFFFF) - seed_id = randint(0x1, 0xFFFF) - if seed_id not in SEED_IDs: - SEED_IDs.append(seed_id) - break + try: + # Fix to allow for pythonping multithreaded usage; + # no need to protect this loop as no one will ever surpass 0xFFFF amount of threads + while True: + # seed_id needs to be less than or equal to 65535 (as original code was seed_id = getpid() & 0xFFFF) + seed_id = randint(0x1, 0xFFFF) + if seed_id not in SEED_IDs: + SEED_IDs.append(seed_id) + break - comm = executor.Communicator(target, provider, timeout, interval, socket_options=options, verbose=verbose, output=out, - seed_id=seed_id, source=source, repr_format=out_format) + comm = executor.Communicator(target, provider, timeout, interval, socket_options=options, verbose=verbose, output=out, + seed_id=seed_id, source=source, repr_format=out_format) - comm.run(match_payloads=match) + comm.run(match_payloads=match) - SEED_IDs.remove(seed_id) + finally: + if seed_id in SEED_IDs: + SEED_IDs.remove(seed_id) return comm.responses diff --git a/pythonping/executor.py b/pythonping/executor.py index 58bdcf3..085c6b1 100644 --- a/pythonping/executor.py +++ b/pythonping/executor.py @@ -92,35 +92,34 @@ def success(self): def error_message(self): if self.message is None: return 'No response' - else: - if self.message.packet.message_type == 0 and self.message.packet.message_code == 0: - # Echo Reply, response OK - no error - return None - elif self.message.packet.message_type == 3: - # Destination unreachable, returning more details based on message code - unreachable_messages = [ - 'Network Unreachable', - 'Host Unreachable', - 'Protocol Unreachable', - 'Port Unreachable', - 'Fragmentation Required', - 'Source Route Failed', - 'Network Unknown', - 'Host Unknown', - 'Source Host Isolated', - 'Communication with Destination Network is Administratively Prohibited', - 'Communication with Destination Host is Administratively Prohibited', - 'Network Unreachable for ToS', - 'Host Unreachable for ToS', - 'Communication Administratively Prohibited', - 'Host Precedence Violation', - 'Precedence Cutoff in Effect' - ] - try: - return unreachable_messages[self.message.packet.message_code] - except IndexError: - # Should never generate IndexError, this serves as additional protection - return 'Unreachable' + if self.message.packet.message_type == 0 and self.message.packet.message_code == 0: + # Echo Reply, response OK - no error + return None + if self.message.packet.message_type == 3: + # Destination unreachable, returning more details based on message code + unreachable_messages = [ + 'Network Unreachable', + 'Host Unreachable', + 'Protocol Unreachable', + 'Port Unreachable', + 'Fragmentation Required', + 'Source Route Failed', + 'Network Unknown', + 'Host Unknown', + 'Source Host Isolated', + 'Communication with Destination Network is Administratively Prohibited', + 'Communication with Destination Host is Administratively Prohibited', + 'Network Unreachable for ToS', + 'Host Unreachable for ToS', + 'Communication Administratively Prohibited', + 'Host Precedence Violation', + 'Precedence Cutoff in Effect' + ] + try: + return unreachable_messages[self.message.packet.message_code] + except IndexError: + # Should never generate IndexError, this serves as additional protection + return 'Unreachable' # Error was not identified return 'Network Error' @@ -131,28 +130,26 @@ def time_elapsed_ms(self): def legacy_repr(self): if self.message is None: return 'Request timed out' - elif self.success: + if self.success: return 'Reply from {0}, {1} bytes in {2}ms'.format(self.message.source, len(self.message.packet.raw), self.time_elapsed_ms) - else: - # Not successful, but with some code (e.g. destination unreachable) - return '{0} from {1} in {2}ms'.format(self.error_message, self.message.source, self.time_elapsed_ms) + # Not successful, but with some code (e.g. destination unreachable) + return '{0} from {1} in {2}ms'.format(self.error_message, self.message.source, self.time_elapsed_ms) def __repr__(self): if self.repr_format == 'legacy': return self.legacy_repr() if self.message is None: return 'Timed out' - elif self.success: + if self.success: return 'status=OK\tfrom={0}\tms={1}\t\tbytes\tsnt={2}\trcv={3}'.format( self.message.source, self.time_elapsed_ms, len(self.source_request.raw)+20, len(self.message.packet.raw) ) - else: - return 'status=ERR\tfrom={1}\terror="{0}"'.format(self.message.source, self.error_message) + return 'status=ERR\tfrom={1}\terror="{0}"'.format(self.message.source, self.error_message) class ResponseList: """Represents a series of ICMP responses""" @@ -231,23 +228,28 @@ def append(self, value): self.rtt_max = value.time_elapsed if value.time_elapsed < self.rtt_min: self.rtt_min = value.time_elapsed - if value.success: self.stats_packets_returned += 1 + if value.success: + self.stats_packets_returned += 1 if self.verbose: print(value, file=self.output) @property - def stats_packets_lost(self): return self.stats_packets_sent - self.stats_packets_returned + def stats_packets_lost(self): + return self.stats_packets_sent - self.stats_packets_returned @property - def stats_success_ratio(self): return self.stats_packets_returned / self.stats_packets_sent + def stats_success_ratio(self): + return self.stats_packets_returned / self.stats_packets_sent @property - def stats_lost_ratio(self): return 1 - self.stats_success_ratio + def stats_lost_ratio(self): + return 1 - self.stats_success_ratio @property - def packets_lost(self): return self.stats_lost_ratio - + def packets_lost(self): + return self.stats_lost_ratio + def __len__(self): return len(self._responses) diff --git a/pythonping/icmp.py b/pythonping/icmp.py index e060735..2814a99 100644 --- a/pythonping/icmp.py +++ b/pythonping/icmp.py @@ -1,9 +1,5 @@ import os -import socket import struct -import select -import time - def checksum(data): """Creates the ICMP checksum as in RFC 1071 @@ -61,64 +57,64 @@ class DestinationUnreachable(ICMPType): class SourceQuench(ICMPType): type_id = 4 SOURCE_QUENCH = (type_id, 0,) - + class Redirect(ICMPType): type_id = 5 FOR_NETWORK = (type_id, 0,) FOR_HOST = (type_id, 1,) FOR_TOS_AND_NETWORK = (type_id, 2,) FOR_TOS_AND_HOST = (type_id, 3,) - + class EchoRequest(ICMPType): type_id = 8 ECHO_REQUEST = (type_id, 0,) - + class RouterAdvertisement(ICMPType): type_id = 9 ROUTER_ADVERTISEMENT = (type_id, 0,) - + class RouterSolicitation(ICMPType): type_id = 10 ROUTER_SOLICITATION = (type_id, 0) # Aliases ROUTER_DISCOVERY = ROUTER_SOLICITATION ROUTER_SELECTION = ROUTER_SOLICITATION - + class TimeExceeded(ICMPType): type_id = 11 TTL_EXPIRED_IN_TRANSIT = (type_id, 0) FRAGMENT_REASSEMBLY_TIME_EXCEEDED = (type_id, 1) - + class BadIPHeader(ICMPType): type_id = 12 POINTER_INDICATES_ERROR = (type_id, 0) MISSING_REQUIRED_OPTION = (type_id, 1) BAD_LENGTH = (type_id, 2) - + class Timestamp(ICMPType): type_id = 13 TIMESTAMP = (type_id, 0) - + class TimestampReply(ICMPType): type_id = 14 TIMESTAMP_REPLY = (type_id, 0) - + class InformationRequest(ICMPType): type_id = 15 INFORMATION_REQUEST = (type_id, 0) - + class InformationReply(ICMPType): type_id = 16 INFORMATION_REPLY = (type_id, 0) - + class AddressMaskRequest(ICMPType): type_id = 17 ADDRESS_MASK_REQUEST = (type_id, 0) - + class AddressMaskReply(ICMPType): type_id = 18 ADDRESS_MASK_REPLY = (type_id, 0) - + class Traceroute(ICMPType): type_id = 30 INFORMATION_REQUEST = (type_id, 30) @@ -160,7 +156,8 @@ def __init__(self, message_type=Types.EchoReply, payload=None, identifier=None, def packet(self): """The raw packet with header, ready to be sent from a socket""" p = self._header(check=self.expected_checksum) + self.payload - if (self.raw is None): self.raw = p + if self.raw is None: + self.raw = p return p def _header(self, check=0): @@ -171,7 +168,7 @@ def _header(self, check=0): :return: The packed header :rtype: bytes""" # TODO implement sequence number - return struct.pack("bbHHh", + return struct.pack("BBHHH", self.message_type, self.message_code, check, @@ -220,5 +217,5 @@ def unpack(self, raw): self.message_code, \ self.received_checksum, \ self.id, \ - self.sequence_number = struct.unpack("bbHHh", raw[20:28]) + self.sequence_number = struct.unpack("BBHHH", raw[20:28]) self.payload = raw[28:] diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 0000000..e7af550 --- /dev/null +++ b/requirements.txt @@ -0,0 +1 @@ +pexpect==4.0.8 \ No newline at end of file diff --git a/setup.py b/setup.py index e460001..379ca3f 100644 --- a/setup.py +++ b/setup.py @@ -1,10 +1,10 @@ from setuptools import setup -with open('README.md', 'r') as file: +with open('README.md', 'r', encoding='utf-8') as file: long_description = file.read() setup(name='pythonping', - version='1.1.3', + version='1.1.4', description='A simple way to ping in Python', url='https://github.com/alessandromaggio/pythonping', author='Alessandro Maggio', diff --git a/test/test_executor.py b/test/test_executor.py index cfa16d6..0b62a5d 100644 --- a/test/test_executor.py +++ b/test/test_executor.py @@ -257,8 +257,8 @@ def test_no_packets_lost(self): self.assertEqual(rs.stats_packets_sent, rs.stats_packets_returned, 'unable to correctly count sent and returned packets when all responses successful') self.assertEqual( - rs.stats_packets_lost, - 0, + rs.stats_packets_lost, + 0, "Unable to calculate packet loss correctly when all responses successful" ) @@ -271,8 +271,8 @@ def test_all_packets_lost(self): ]) self.assertEqual(rs.stats_packets_returned, 0, 'unable to correctly count sent and returned packets when all responses failed') self.assertEqual( - rs.stats_lost_ratio, - 1.0, + rs.stats_lost_ratio, + 1.0, "Unable to calculate packet loss correctly when all responses failed" ) @@ -286,8 +286,8 @@ def test_some_packets_lost(self): self.assertEqual(rs.stats_packets_sent, 4, 'unable to correctly count sent packets when some of the responses failed') self.assertEqual(rs.stats_packets_returned, 2, 'unable to correctly count returned packets when some of the responses failed') self.assertEqual( - rs.stats_lost_ratio, - 0.5, + rs.stats_lost_ratio, + 0.5, "Unable to calculate packet loss correctly when some of the responses failed" ) diff --git a/test/test_network.py b/test/test_network.py index b7cba82..144a58a 100644 --- a/test/test_network.py +++ b/test/test_network.py @@ -1,11 +1,12 @@ import unittest from pythonping.network import Socket - class UtilsTestCase(unittest.TestCase): """Tests for Socket class""" + + def test_raise_explicative_error_on_name_resolution_failure(self): """Test a runtime error is generated if the name cannot be resolved""" with self.assertRaises(RuntimeError): - Socket('invalid', 'raw') + Socket('invalid', 'raw') \ No newline at end of file diff --git a/test/test_payload_provider.py b/test/test_payload_provider.py index 48a20ef..9cbc474 100644 --- a/test/test_payload_provider.py +++ b/test/test_payload_provider.py @@ -1,10 +1,17 @@ import unittest + +from pexpect import ExceptionPexpect from pythonping import payload_provider class PayloadProviderTestCase(unittest.TestCase): """Tests for PayloadProvider class""" + def test_basic(self): + """Verifies that a provider raises error when calling own functions""" + self.assertRaises(NotImplementedError, payload_provider.PayloadProvider) + + def test_list(self): """Verifies that a list provider generates the correct payloads""" payloads = [