patternpythonMinor
File-serving Twisted resource
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
Will
``
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 ""
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 templatesor 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.
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
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:
This is guaranteed to close the file even without reference counting.
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.