HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

CLI Command Search Tool - XML Framework

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
searchxmlclicommandframeworktool

Problem

I'm working on the OSCP, and Information Security certificate based around penetration testing. CLI-based tools seem to be the core of most pen-testing assignments, so I'd like to build a tool aiding in command/switch searching.

The goal of the program will to (eventually) load command line arguments by searching in real-time based on arguments, application names, common use-case, and application details. Something like a dynamic Google of the CLI for pen-testing tools.

I'm relatively new to Python. I've never really had anyone look over my code before. I'm hoping that there are not too big of gaps in the logic and syntax. I'm sure there are areas where my code could be simplified. Any suggestions are appreciated.

```
import threading
import xml.etree.ElementTree as et
import os

# Command to clear the CLI in NT-based systems
cls = lambda: os.system('cls')

# Application dictionary data structure for internal data use
app_dict = {}

# Reference data sets for associative commands and arguments
commands = []
command_key = {}
arguments = []
command_pointers = []
argument_pointers = []

# Function to return query results in a consistent format
def return_command(command_pointer):
raw_input('''
Application Name: {}
Arguments: {}
'''.format(command_pointer.name, command_pointer.arguments))

# Base class for a command, identified as the first argument to the CLI
class Command:
def __init__(self, name):
self.name = name
self.arguments = []
commands.append(name)
command_key[name] = self
command_pointers.append(self)

def set_argument(self, number, optional, prefix):
self.arguments.append(number)
self.argument = Argument(self.name, number)
self.argument.create_argument(optional, prefix)

def set_argument_details(self, details, common):
self.argument.set_argument_details(details, common)

# Subsequent CLI arguments following the base command
class Argument:
def __init__(self, pa

Solution

Coding style

There are many coding style issues here. PEP8 is the official Python coding style guide, give it a good read and start following it. There is a command line tool named pep8 which you can install easily with pip install pep8 and run on your code. Try to fix all reported problems.

Common Python practices.

Classes should have CamelCase names, so Command instead of command and Argument instead of argument.

Classes should extend object, so class Command(object): instead of class command:.

You should define all instance attributes in the constructor. For example this is not so good:

def __init__(self, name):
    self.name = name

def set_argument(self, number):
    self.argument = Argument(self.name, number)


You should initialize self.argument in __init__:

def __init__(self, name):
    self.name = name
    self.argument = None

def set_argument(self, number):
    self.argument = Argument(self.name, number)


Class and method decomposition

The way you divided the problem to classes and methods is very strange, confusing and fragile. Consider how you create a Command class:

  • Create an empty Command



  • Set an argument in Command



  • Creates an Argument



  • Set attributes on Argument



  • Set argument details in Command



  • Delegates to Argument



This is really twisted logic. You are piecing together a Command object like a jigsaw puzzle, sticking part by part on it. Ideally, it's best to give a class everything it needs to function correctly in the constructor. You could refactor your code to behave more like this:

  • Create a collection of Arguments that will be part of a Command



  • Create the Command with the collection of Arguments



Something like this would be better:

commands = {}

def return_command(command):
    raw_input(textwrap.dedent('''
        Application Name: {}
        Arguments: {}
        '''.format(command.name, [arg.number for arg in command.arguments])))

class Command:
    def __init__(self, name, arguments):
        self.name = name
        self.arguments = arguments

class Argument:
    def __init__(self, number, optional, prefix, details, common):
        self.number = number
        self.optional = optional
        self.prefix = prefix
        self.details = details
        self.common = common

def parse_arguments(app):
    for details in app:
        argument = Argument(details.attrib['number'], details.attrib['optional'],
                            details.attrib['prefix'],
                            details.attrib.get('details'), details.attrib.get('common'))
        yield argument

def parse_xml(path):
    xmldoc = et.parse(path)
    data = xmldoc.getroot()
    for app in data:
        arguments = [_ for _ in parse_arguments(app)]
        command = Command(app.attrib['name'], arguments)
        commands[command.name] = command

class Search(threading.Thread):
    def __init__(self):
        threading.Thread.__init__(self)

    def run(self):
        while True:
            cls()
            search_key = raw_input('>>> ')
            if search_key in commands:
                return_command(commands[search_key])

parse_xml('terms.xml')
query = Search()
query.run()


That is, the classes should know much about the "outside world". Command should not know more than necessary about an Argument and vice versa. Most important of all, a class should not modify global variables like you did in your version. Notice that I deleted most global variables, left only commands, which is populated and used outside of the Commands class. I strongly recommend you this article:

http://en.wikipedia.org/wiki/Single_responsibility_principle

Code Snippets

def __init__(self, name):
    self.name = name

def set_argument(self, number):
    self.argument = Argument(self.name, number)
def __init__(self, name):
    self.name = name
    self.argument = None

def set_argument(self, number):
    self.argument = Argument(self.name, number)
commands = {}


def return_command(command):
    raw_input(textwrap.dedent('''
        Application Name: {}
        Arguments: {}
        '''.format(command.name, [arg.number for arg in command.arguments])))


class Command:
    def __init__(self, name, arguments):
        self.name = name
        self.arguments = arguments


class Argument:
    def __init__(self, number, optional, prefix, details, common):
        self.number = number
        self.optional = optional
        self.prefix = prefix
        self.details = details
        self.common = common


def parse_arguments(app):
    for details in app:
        argument = Argument(details.attrib['number'], details.attrib['optional'],
                            details.attrib['prefix'],
                            details.attrib.get('details'), details.attrib.get('common'))
        yield argument


def parse_xml(path):
    xmldoc = et.parse(path)
    data = xmldoc.getroot()
    for app in data:
        arguments = [_ for _ in parse_arguments(app)]
        command = Command(app.attrib['name'], arguments)
        commands[command.name] = command


class Search(threading.Thread):
    def __init__(self):
        threading.Thread.__init__(self)

    def run(self):
        while True:
            cls()
            search_key = raw_input('>>> ')
            if search_key in commands:
                return_command(commands[search_key])


parse_xml('terms.xml')
query = Search()
query.run()

Context

StackExchange Code Review Q#60474, answer score: 3

Revisions (0)

No revisions yet.