Basic web server in Python 3

Question

This project is intended to function as a basic web server with a command-line interface for easy use. It's written in Python 3, with the help of the cmd module for the CLI itself and the _thread module (I know, I know) to run the listening/responding process without interfering with the CLI.

I am wondering (with the exception of _thread, which I do intend to fix very soon) how well it conforms to best practices and how readable it is. Performance is not a big issue.

import socket
import cmd
import os.path
import _thread  # TODO: Switch from _thread to threading
import json

version = "0.3.0"
settings = {}
serverSocket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
serverSocket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)


def loadSettings():
    global settings
    with open('.linprconfig', 'r') as configFile:
        settings = json.load(configFile)


def makeErrorResponse(errorNumber):
    with open("htdocs/error/default.html") as defaultErrorTemplateFile:
        defaultErrorTemplate = defaultErrorTemplateFile.readlines()
    formattedErrorTemplate = ""
    for line in defaultErrorTemplate:
        formattedErrorTemplate += (line.replace("{}", str(errorNumber)))
    return "HTTP/1.0 " + str(errorNumber) + "\n\n", formattedErrorTemplate,


def makeResponse(filename):
    if filename == "/": filename = "/index.html"

    try:
        with open(os.path.join(settings["general"]["contentRoot"], filename[1:])) as requestedFile:  # slice removes leading /
            try:
                return 'HTTP/1.0 200 OK\n\n', requestedFile.read(),
            except UnicodeDecodeError:  # it's not a text-based file
                requestedFile = open(os.path.join(settings["general"]["contentRoot"], filename[1:]), 'rb')  # reopen as binary
                return 'HTTP/1.0 200 OK\n\n', requestedFile.read(),
    except (FileNotFoundError, IsADirectoryError):
        return makeErrorResponse(404)


def listen():
    try:
        while True:
            clientConnection, clientAddress = serverSocket.accept()

            # Get the client request and split it up
            request = clientConnection.recv(1024).decode()

            try:
                headers = request.split('\n')

                # Parse the request
                command = headers[0]
                if command.strip(" ") == "": continue
                responseHeaders = makeResponse(command.split()[1])[0]
                responseContent = makeResponse(command.split()[1])[1]

                clientConnection.sendall(responseHeaders.encode())
                try:
                    clientConnection.sendall(responseContent.encode())
                except AttributeError:  # it's a bytes object, so no .encode() method
                    clientConnection.sendall(responseContent)
                clientConnection.close()
            except Exception as e:
                print("Error caught and 500 returned: "+ str(e))
                responseHeaders = makeErrorResponse(500)[0]
                responseContent = makeErrorResponse(500)[1]
                clientConnection.sendall(responseHeaders.encode())
                try:
                    clientConnection.sendall(responseContent.encode())
                except AttributeError:  # it's a bytes object, so no .encode() method
                    clientConnection.sendall(responseContent)
                clientConnection.close()
    except (KeyboardInterrupt, EOFError, SystemExit):
        pass  # this hides ugly messages on exit



class CommandLine(cmd.Cmd):

    def preloop(self):
        self.host = None
        self.port = None
        self.prompt = "LINPR " + version + " >"

    def do_startup(self, s):
        s = s.split(" ")

        try:
            self.host = s[0]
            self.port = s[1]
        except IndexError:
            self.host = "0.0.0.0"
            self.port = 80

        serverSocket.bind((self.host, int(self.port)))


    def help_startup(self):
        print("Bind to a user-specified host and port. "
              "When host and port are unsupplied, use the defaults 0.0.0.0 and 80.")


    def do_listen(self, s):
        print("Listening on port " + str(self.port))
        serverSocket.listen(1)
        _thread.start_new_thread(listen, ())


    def help_listen(self):
        print("Listen for any incoming connections and automatically respond to them with the requested resource.")


    def do_exit(self, s):
        print("Exiting...")
        try:
            serverSocket.close()
        except NameError:  # server_socket hasn't been created yet
            pass
        return True


    def help_exit(self):
        print("Exits LINPR and shuts down the server.")

    def emptyline(self):
        pass

loadSettings()
interpreter = CommandLine()
try:
    interpreter.cmdloop(intro="Welcome to LINPR " + version + ". Type 'help' for help, or 'help cmd' for info on the 'cmd' command.")
except (KeyboardInterrupt, EOFError, SystemExit):
    print("\n\nUse 'exit' to exit.")
    CommandLine.do_exit(interpreter, "")

Answer 1

1. You should strive to write PEP-8 compliant code.

   • Function and variable names should follow the snake_case naming convention. Constants should be in ALL_CAPS.

   • There should be a single blank line in between class methods, with no initial blank line between the class signature and the first method definition.

2. There's usually no good reason to put an if-statement and its body on the same line.

3. Global variables can quickly clutter the namespace and become hard to follow around because they can be accessed and modified from any part of the program. This works for very simple scripts and variables that truly do not belong to any object, but those are not valid excuses here.

   To avoid using global variables, you can bind them to a class (instance) or simply pass them around as arguments to functions. This is also where you should be making the decision to go for a more object-oriented approach, or a traditional 'procedural' one.

4. Debugging messages should be written to stderr, not stdout. You can use sys.stderr for that purpose.

5. String concatenation is hard to read and slow. Use string formatting instead. If you have Python 3.6, you can use f-strings (see the quite unwieldy PEP-498), or use the backwards compatible str.format():

   self.prompt = "LINPR {} >".format(version)
   

6. In listen(), you don't need to call makeErrorResponse(500) twice. Simply store the result in a variable:

   error_response = makeErrorResponse(500)
   # ...
   

   or unpack immediately:

   headers, content = makeErrorResponse(500)
   

   You could do the same tuple unpacking trick in CommandLine.do_startup():

   self.host, self.port = s
   

7. This line:

   request = clientConnection.recv(1024).decode()
   

   does not do what you think it does. HTTP requests vary in size, so a single request will rarely be 1024 bytes (unless you implement padding, but really?). You'll have to add some kind of message system. This is typically done by either using unique delimiters, or by prefixing the data with its length.

8. Catching Exception is too broad. What could go wrong here? String splitting won't fail here, nor will any of the code below (at least up till the next try). Since you already handle the AttributeError case, why is the outer try / except in there at all?

9. Come to think of it, why would you get an AttributeError there? In makeErrorResponse(), this:

   formattedErrorTemplate = ""
   

   assures that the return type is a (string, string) tuple. There's just no way that would magically turn into a bytes object.

10. The HTTP line break is CRLF; Carriage Return + Line Feed. RFC 2616, section 2.2:

    HTTP/1.1 defines the sequence CR LF as the end-of-line marker for all protocol elements except the entity-body (see appendix 19.3 for tolerant applications).

    So this:

    return 'HTTP/1.0 200 OK\n\n'
    

    is wrong, and should be:

    return 'HTTP/1.0 200 OK\r\n'
    

11. Why this:

    CommandLine.do_exit(interpreter, "")
    

    instead of:

    interpreter.do_exit("")
    

12. The s argument to CommandLine.do_exit() is never actually used, so could be removed.

Answer 2

Direction and Architecture

If applicable to your use case, I'd first see if the functionality and performance of the existing module http.server is sufficient, and if yes - would just use it.

If it is established that a custom server implementation is required (then it would be interesting to know the reason for a more focused review), I would consider using multiprocessing over threading, as with a very similar API the former is not subject to the contention effects of the Global Interpreter Lock.

Lastly, if I were a perfectionist, I would have tried to create a single threaded asynchronous server with the asyncio module (like this project did). It requires some mind stretching to get around the syntax of async coroutines, but gives the maximum amount of fun to work with.

Implementation

I would try to factor out a Handler layer (with the default implementation just serving static files). This should make writing tests easier and provide generally useful level of indirection for authz, audit, timing, etc.