Bug in send_file with temp files

[b4stien]

Hey there,

I don't know if anyone ran into this problem before because it works "by chance" in py2, but in py3 you can't send temporary file with send_file by passing a fp.

The main problems are here https://github.com/mitsuhiko/flask/blob/master/flask/helpers.py#L477 and there https://github.com/mitsuhiko/flask/blob/master/flask/helpers.py#L496.

Under py2 a tempfile.TemporaryFile is given the name '<fdopen>' whereas under py3 it's an integer.

Python 2.7.9 (default, Dec 15 2014, 10:01:34)
[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.56)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from tempfile import TemporaryFile
>>> fp = TemporaryFile()
>>> fp.name
'<fdopen>'

Python 3.4.3 (default, Mar 23 2015, 04:19:36)
[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.57)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from tempfile import TemporaryFile
>>> fp = TemporaryFile()
>>> fp.name
4

Then when Flask tries to build filename it crashes (trying to concatenate a string and an integer in os.path.join(current_app.root_path, filename)).

Would you consider merging a workaround for this problem? (like setting filename to None if we don't have a string_types for instance)

[untitaker]

The whole feature is apparently supposed to be deprecated, so I don't think a fix is worth it. You're supposed to use attachment_filename anyway.

I want to keep this open as a reminder for deprecation though.

[b4stien]

Thing is, with this function in its current version, even if you pass an attachment_filename you have this problem. Even "worse" in Py2, it doesn't crash because a temp file has a name, but the mimetype guessing fails.

If the whole function is refactored in the next version I agree that it's not worth fixing it, but if it has to stay for longer...

[untitaker]

Yeah I'd say we just refactor the whole thing in 1.0.

[johaven]

Another bug with tempfile.SpooledTemporaryFile when object become a fd (when it exceeds the given size)

   f = tempfile.SpooledTemporaryFile(max_size=rsync.MAX_SPOOL, mode='wb')
   f.write(xxx)
   ......
    rv = send_file(f, as_attachment=True,  attachment_filename=fname.encode('utf-8'))
  File "/usr/lib/python2.7/site-packages/flask/helpers.py", line 542, in send_file
    os.path.getmtime(filename),
  File "/usr/lib64/python2.7/genericpath.py", line 54, in getmtime
    return os.stat(filename).st_mtime
OSError: [Errno 2] No such file or directory: '/var/www/xxx/xxx/<fdopen>'

[ThiefMaster]

Was the temporary file created with delete=True (or rather without delete=False) by any chance?

[johaven]

Spooled has no option delete (https://docs.python.org/2/library/tempfile.html)
I think the object properties are different from file object but detected by flask as such.

[dsully]

I'm looking at this issue at PyCon. Should the deprecation warnings be removed, and the file object functionality stay otherwise?

[ThiefMaster]

+1 for removing name guessing. being able to send a file object needs to stay (but w/o name guessing)

[dsully]

@ThiefMaster What should the behavior be when a file object is passed, but add_etags is True (the default). Should send_file() silently not generate the etag? Or should it raise an exception?

[dsully]

Pull Request #1849

[dsully]

This can be closed now, the PR above has been merged.

[b4stien]

Done, thanks guys 👍