Python Forum
Thread Rating:
  • 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Refactoring to a class
#1
I've got an old and simple procedural port scanner script that I'm working on refactoring into a class. They are certainly not functioning similarly and I was hoping for some productive feedback. Here is the procedural script ->

import socket
from IPy import IP


def scan(target, port_num=100):
    converted_ip = check_ip(target)
    print('\n' + f'[*] Scanning Target {str(target)}')
    for port in range(1, port_num+1):
        port_scan(converted_ip, port)


def check_ip(ip):
    try:
        IP(ip)
        return ip
    except ValueError:
        return socket.gethostbyname(ip)


def get_banner(s):
    return s.recv(1024)


def port_scan(ipaddress, port):
    try:
        sock = socket.socket()
        sock.settimeout(0.5)
        sock.connect((ipaddress, port))
        try:
            banner = get_banner(sock)
            # I'm not sure why the top one doesn't work
            # print(f'[+] Open Port {str(port)}: {str(banner.decode().strip('\n'))}')
            print(f'[+] Open Port {str(port)}: ' + str(banner.decode().strip('\n')))
        except:
            print(f'[+] Open Port {str(port)}')
    except:
        print(f"Closed port at: {str(port)}")


if __name__ == "__main__":
    targets = input('[?] Enter Target/s to Scan (separate multiple targets with a comma): ')
    ports_to_scan = int(input('[?] How Many Ports Do You Want To Scan: '))

    if ',' in targets:
        for ip_add in targets.split(','):
            scan(ip_add.strip(' '), ports_to_scan)
    else:
        scan(targets, ports_to_scan)
And here is my class refactor attempt ->

import socket
from IPy import IP


def check_ip(ip):
    try:
        IP(ip)
        return ip
    except ValueError:
        return socket.gethostbyname(ip)


def get_banner(s):
    return s.recv(1024)


class Portscanner:

    def __init__(self):
        self.sock = socket.socket()

    def scan(self, target, port_num=100):
        converted_ip = check_ip(target)
        print('\n' + f'[*] Scanning Target {str(target)}')
        for port in range(1, port_num+1):
            self.port_scan(converted_ip, port)

    def port_scan(self, ip, port):
        try:
            self.sock.settimeout(0.5)
            self.sock.connect((ip, port))

            try:
                banner = get_banner(self.sock)
                print(f'[+] Open Port {str(port)}: ' + str(banner.decode().strip('\n')))
            except:
                print(f'[+] Open Port {str(port)}')

        except:
            print(f"Closed port at: {str(port)}")
And then from main.py ->

from portscanner import Portscanner


targets = input('[?] Enter Target/s to Scan (separate multiple targets with a comma): ')
ports_to_scan = int(input('[?] How Many Ports Do You Want To Scan: '))

portscanner = Portscanner()

if ',' in targets:
    for ip_add in targets.split(','):
        portscanner.scan(ip_add.strip(' '), ports_to_scan)
else:
    portscanner.scan(targets, ports_to_scan)
I've been using scanme.nmap.org to test the outcomes. The first code (the procedural way) works fine. It returns the open ports and the banners if any.
The OOP class refactor attempt isn't going so well. It returns every port as closed and obviously doesn't return any banners either.
Hoping some of you fine folks with more experience than me can point out where I need to make corrections and do better.
Thanks
Reply
#2
Why are you making this a class? I don't see anything class-like about it. The only thing making this a class does is something you really don't want it to do; hang onto a socket.

If you really want a class I would do it like this:
class Portscanner:
    def __init__(self, target, port_num):
        ip = check_ip(target)
        for port in range(1, port_num+1):
            try:
                sock = socket.socket()
                sock.settimeout(0.5)
                sock.connect((ip, port))
                try:
                    banner = sock.recv(1024)
                    print(f'[+] Open Port {str(port)}: ' + str(banner.decode().strip('\n')))
                except:
                    print(f'[+] Open Port {str(port)}')
            finally:
                sock.close()

for target in targets.split(',')
    Portscanner(target.strip(), ports_to_scan)
But I would rather do this:
def scan_ports(self, target, port_num):
    ip = check_ip(target)
    for port in range(1, port_num+1):
        try:
            sock = socket.socket()
            sock.settimeout(0.5)
            sock.connect((ip, port))
            try:
                banner = sock.recv(1024)
                print(f'[+] Open Port {str(port)}: ' + str(banner.decode().strip('\n')))
            except:
                print(f'[+] Open Port {str(port)}')
        finally:
            sock.close()

for target in targets.split(',')
    scan_ports(target.strip(), ports_to_scan)
Reply


Forum Jump:

User Panel Messages

Announcements
Announcement #1 8/1/2020
Announcement #2 8/2/2020
Announcement #3 8/6/2020