tags 655154 + upstream
forwarded 655154 si...@budig.de
thanks
Hello,
The attached patch fixes this problem. I have forwarded it to the
upstream maintainer. Here follows the patch description:
,----
| Improve handling of files received with option -U
|
| Instead of storing the downloaded contents into a temporary file
| that is copied by woof to the final destination (current directory),
| the uploaded file is directly written to the current directory, with
| precautions to avoid overwriting existing files.
|
| This is done by subclassing cgi.FieldStorage to override make_file() so
| that it does not create a temporary file when encountering an atomic
| part whose name is "upfile" in the MIME data.
|
| This improvement is particularly welcome when receiving large files,
| since it avoids:
| - a possible failure due to the temporary directory being too small to
| hold the file;
| - a useless copy of the whole file;
| - a possible loss of the whole download in case a failure occurs when
| woof copies the (complete) temporary file to its final destination.
`----
For people who don't want to apply the patch, setting TMPDIR allows a
partial workaround: you may indicate a directory with more space than
/tmp this way, but you will still have to hold two copies of the
downloaded file at the same time (one in TMPDIR and the other in the
directory from which 'woof -U' was started), until woof deletes the
temporary file through garbage collection. Depending on the file size
and the available space, this may or may not be possible.
--
Florent
diff --git a/woof b/woof
index 2abedf0..fde8de1 100755
--- a/woof
+++ b/woof
@@ -124,6 +124,64 @@ class ForkingHTTPServer (BaseHTTPServer.HTTPServer):
self.close_request (request)
+class MyFieldStorage (cgi.FieldStorage):
+ """Subclass of FieldStorage with efficient handling of uploaded files.
+
+ This class behaves the same as FieldStorage except that it handles uploaded
+ files more efficiently: instead of storing the data into a temporary file
+ that is copied by woof to the final destination (current directory),
+ the uploaded file is directly written to the current directory, with
+ precautions to avoid overwriting existing files.
+
+ """
+ # The function signature changed between Python 2 and Python 3; this will
+ # work in all cases.
+ def make_file(self, *args, **kwargs):
+ # The "self.list is None" check is here to prevent misbehaving in case we
+ # receive a multipart whose name happens to be "upfile".
+ if self.list is None and self.name == "upfile":
+ # make_file() was called for an uploaded file. No need to create a
+ # potentially huge temporary file only to copy it to the final
+ # destination. Just create the file in a way that avoids overwriting
+ # existing files.
+ upfilename = self.filename
+
+ if "\\" in upfilename:
+ upfilename = upfilename.split ("\\")[-1]
+
+ upfilename = os.path.basename (self.filename)
+
+ destfile = None
+ for suffix in ["", ".1", ".2", ".3", ".4", ".5", ".6", ".7", ".8",
+ ".9"]:
+ destfilename = os.path.join (upfilename + suffix)
+ try:
+ destfile = os.open (destfilename,
+ os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644)
+ break
+ except OSError, e:
+ if e.errno == errno.EEXIST:
+ continue
+ raise
+
+ # Fallback to tempfile.mkstemp() in case the above did not manage to
+ # create the file.
+ if destfile is None:
+ destfile, destfilepath = tempfile.mkstemp (
+ prefix = upfilename + ".", dir = ".")
+ # 'destfilepath' is a full path, be consistent with the normal case
+ destfilename = os.path.basename (destfilepath)
+
+ msg = "Accepting uploaded file: '%s'" % upfilename
+ if upfilename != destfilename:
+ msg += ", stored as '%s'" % destfilename
+ sys.stderr.write (msg + "...\n")
+
+ return os.fdopen (destfile, "wb")
+ else:
+ return cgi.FieldStorage.make_file (self, *args, **kwargs)
+
+
# Main class implementing an HTTP-Requesthandler, that serves just a single
# file and redirects all other requests to this file (this passes the actual
# filename to the client).
@@ -152,49 +210,26 @@ class FileServHTTPRequestHandler (BaseHTTPServer.BaseHTTPRequestHandler):
# http://mail.python.org/pipermail/python-list/2006-September/402441.html
ctype, pdict = cgi.parse_header (self.headers.getheader ('Content-Type'))
- form = cgi.FieldStorage (fp = self.rfile,
- headers = self.headers,
- environ = {'REQUEST_METHOD' : 'POST'},
- keep_blank_values = 1,
- strict_parsing = 1)
+ form = MyFieldStorage (fp = self.rfile,
+ headers = self.headers,
+ environ = {'REQUEST_METHOD' : 'POST'},
+ keep_blank_values = 1,
+ strict_parsing = 1)
if not form.has_key ("upfile"):
self.send_error (403, "No upload provided")
return
-
+
upfile = form["upfile"]
if not upfile.file or not upfile.filename:
self.send_error (403, "No upload provided")
return
-
- upfilename = upfile.filename
-
- if "\\" in upfilename:
- upfilename = upfilename.split ("\\")[-1]
-
- upfilename = os.path.basename (upfile.filename)
- destfile = None
- for suffix in ["", ".1", ".2", ".3", ".4", ".5", ".6", ".7", ".8", ".9"]:
- destfilename = os.path.join (".", upfilename + suffix)
- try:
- destfile = os.open (destfilename, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0644)
- break
- except OSError, e:
- if e.errno == errno.EEXIST:
- continue
- raise
-
- if not destfile:
- upfilename += "."
- destfile, destfilename = tempfile.mkstemp (prefix = upfilename, dir = ".")
-
- print >>sys.stderr, "accepting uploaded file: %s -> %s" % (upfilename, destfilename)
-
- shutil.copyfileobj (upfile.file, os.fdopen (destfile, "w"))
-
if upfile.done == -1:
self.send_error (408, "upload interrupted")
+ else:
+ # cgi.FieldStorage's constructor left the file open
+ upfile.file.close()
txt = """\
<html>
