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

File-serving Twisted resource

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

Problem

I've just written this Twisted resource base class for serving files. It's part of a larger application, this is just for serving its images, JavaScript files, etc.

Is it okay? I wonder if checking for .. in the path is enough -- all the files that exist in self.directory are intended to be served, to anyone, so this should be all that's needed, I think. Should these it be a Unicode string? Should they all be Unicode strings?

Will return open(file).read() be enough? I would think the resource would be closed when the function goes out of scope, but I'm not completely sure.

``
"""
Base class for resources serving files to the browser.

This doesn't use the
Page` class because it doesn't need to set up templates
or anything, just serve the file.
"""
from twisted.web.resource import Resource
import os.path

class File(Resource):
"""
Serves a certain type of file from the server.

It could be images, javascript, etc. It serves files from a single
directory which have a single extension.

members:
content_type -- the HTTP content type of the files to be served
directory -- the local directory the files are stored in
extension -- the extension which the files are saved with.
"""
content_type = None
directory = None
extension = None

def render_GET(self, request):
"""
Serve the requested file.

Makes sure the file exists and the request doesn't contain '..'
to escape the file directory.
"""
file = request.path.strip("/")
file = file.split("/", 1)[1]
file = "{directory}/{filepath}.{extension}".format(
directory=self.directory, filepath=file, extension=self.extension)

request.responseHeaders.setRawHeaders("content-type", [self.content_type])

if ".." in file:
request.setResponseCode(400)
return ""

if not os.path.isfile(file):
request.setResponseCode(404)
return ""

Solution

Firstly, don't try to think of everything that could go wrong. Instead, check for what you know to be valid.

import re
directory, filename = re.match('/([A-Za-z0-9]+)/([A-Za-z0-9]+)/?', alpha).groups()


By using this method to extract the directory and filename, we can be sure that directory and filename contain nothing except letters and numbers which are perfectly safe.

Its also best to use os.path.join to produce folder paths rather then string formatting to put them together.

return open(file).read()


This works, but not if you try and run on IronPython/Jython/PyPy. CPython depends on reference counting which will cause the file to be closed as soon as it returns. A better version is probably:

with open(file) as source:
     return source.read()


This is guaranteed to close the file even without reference counting.

Code Snippets

import re
directory, filename = re.match('/([A-Za-z0-9]+)/([A-Za-z0-9]+)/?', alpha).groups()
return open(file).read()
with open(file) as source:
     return source.read()

Context

StackExchange Code Review Q#2315, answer score: 2

Revisions (0)

No revisions yet.