5
\$\begingroup\$

This is one of my first times writing Python. I'm most curious about style and whether while 1: x,y = socket.accept() is an acceptable way to wait for new connections.

#!/usr/bin/pythonw

import socket
import os


class Server:  

  def __init__(self, directory, host="", port=8123):
    print("Server running on port ", port)
    self.directory = directory
    serversocket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    serversocket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
    serversocket.bind((host, port))
    serversocket.listen(5)
    while 1:
      connection, address = serversocket.accept()
      print("Connection received")
      socketfile = connection.makefile("rw")
      request = socketfile.readline()
      print(request)
      url = self.getURL(request)
      print("Serving ", url)
      responseBody = None
      responseCode = 200
      try:
        with open(url, "rb") as requestedFile:
          responseBody = requestedFile.read()
      except FileNotFoundError:
        print("Error 404: File not found")
        responseBody = b"404 Error Not Found"
        responseCode = 404
      connection.sendall(self.writeResponse(url, responseCode, responseBody))
      socketfile.close()
      connection.close()
      print("Connection closed\n")

  def writeResponse(self, url, code, body):
    response = "HTTP/1.1 " + ("200\n" if code is 200 else "404\n")
    response = response + "Cache-Control: no-cache, no-store, must-revalidate\n"
    response = response + "Pragma: no-cache\n"
    response = response + "Expires: 0\n"
    response = response + "Connection: close\n"
    if code is 200:
      response = response + "Content-Length: " + str(os.path.getsize(url)) + "\n"
      if ".css" in url:
        response = response + "Content-Type: text/css; charset=UTF-8\n"
      elif ".js" in url:
        response = response + "Content-Type: application/javascript charset=UTF-8\n"
    response = response + "\n"
    return str.encode(response) + body

  def getURL(self, line):
    first = line.find(" ") + 1
    second = line.find(" ", first + 1)
    line = line[(first + 1):second]
    if line == "" or line == "/":
      return self.directory + "index.html"
    else:
      return self.directory + line



if __name__ == "__main__":
  x = Server("/some/path/")
\$\endgroup\$
4
\$\begingroup\$

Instead of while 1:, it would be more natural to write while True:


Python has an official style guide called PEP8. I suggest to give that a good read and follow the recommendations, especially in terms of indentation and naming rules.


Inside the Server constructor you have a pointless assignment:

responseBody = None

This variable is reassigned later, no need to initialize it.


It's never a good idea to do much in a constructor. Instead of starting a server to accept connections in the constructor, I suggest to move that logic to a method.


Everywhere you write x = x + something, a shorter (and recommended) way is to write x += something.


These conditions seem a bit weak:

if ".css" in url:
    # ...
elif ".js" in url:
    # ...

Are you sure that .css or .js won't appear at unexpected places inside the url? If you expect these to be at the end, it would be better to use the more strict conditions:

if url.endswith(".css"):
    # ...
elif url.endswith(".js"):
    # ...
\$\endgroup\$
0
\$\begingroup\$

I haven't gone through the code throughly but I do have one small gripe - your naming convention does not follow pythons PEP8. For example writeResponse should be write_response. I would recommend taking a look at pythons PEP8 style guide or even a shorthand version of it.

\$\endgroup\$
0
\$\begingroup\$

You seem to have reinvented http.server (Python 3) or SimpleHTTPServer (Python 2).

The HTTP 1.1 specification is vastly more complex than HTTP 1.0. Don't claim to be an HTTP 1.1 server if you aren't implementing any of the HTTP 1.1 features.

getURL() would be more simply written using line.split() or a regular expression match. It fails to do percent-decoding. Note that since you don't validate the URL, it's possible to request any file on the server's filesystem by making a request like GET /../../../../../../../../../../../etc/passwd HTTP/1.0. I consider that to be a security hole.

On supported platforms with Python ≥ 3.3, you can use os.sendfile() to copy a file's contents to a socket.

\$\endgroup\$
  • \$\begingroup\$ This is just for learning purposes, but these are great points! Thank you \$\endgroup\$ – sudo rm -rf slash Feb 24 '16 at 19:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.