Do not cause errors for unknown files for sendfile

[mitsuhiko]

Unknown files must not raise errors when send with send-file if the
sending happens by filename. This is particularly important for
send from directory which can send up loads of unknown files.

This regressed at one point in the last two weeks. It came up
with woff2 files sent from static folders which are unknown in
the default mimetype database.

---

This change is

[mitsuhiko]

@untitaker can you have a look at this one?

[untitaker]

This is actually intentional behavior as per #1988. The idea was that send_file would never accidentally send a octet-stream when the user simply assumed the mimetype to be in the database. I didn't think about send_from_directory, but I suppose you could just use mimetype.add_type to add the missing mimetype (probably should be documented).

Is this the behavior you can agree with? In any case if you want to change your decision (you approved the mentioned PR), it needs a doc update in upgrading.rst, last paragraph here: https://github.com/pallets/flask/blob/master/docs/upgrading.rst#changes-to-send_file

[untitaker]

Oh, I also remember that I made this behavior more explicit because when we stopped using f.name, there was a risk that, when upgrading Flask, applications suddenly would send octet-stream where Flask previously used a mimetype derived from f.name. A hard error seemed preferrable back then.

[mitsuhiko]

@untitaker we can't force people to add all mimetypes unfortunately. I think the behavior of this pull request is what I like to have. I will update the docs accordingly.

[mitsuhiko]

We can add a new parameter alternatively that controls the fallback.

[mitsuhiko]

In any case the current regression is really bad for a lot of of useful cases where we just don't have a mimetype and also inconsistent with what other frameworks in Python do.

[untitaker]

As said I think silently falling back to octet-stream by default will also introduce regressions in a much more silent way. Nobody said anything about all mimetypes, but adding the ones the application uses is doable for the developer imo. But I don't have a strong opinion on this.

On 11 September 2016 10:29:47 CEST, Armin Ronacher notifications@github.com wrote:

In any case the current regression is really bad for a lot of of useful
cases where we just don't have a mimetype and also inconsistent with
what other frameworks in Python do.

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#2019 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

[mitsuhiko]

@untitaker what is the actual regression if something serves up as octet stream? Most browsers ignore the mimetype anyways for resource loading.

[untitaker]

Serving with an incorrect mimetype is still technically wrong, and that there are only a handful of browsers that break on incorrect mimetypes doesn't make it easier to debug either.

Also you don't even have to use mimetype.add_type, you can also pass f.name as attachment_filename.

On 11 September 2016 11:00:43 CEST, Armin Ronacher notifications@github.com wrote:

@untitaker what is the actual regression if something serves up as
octet stream? Most browsers ignore the mimetype anyways for resource
loading.

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#2019 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

[mitsuhiko]

@untitaker i don't want to reintroduce the handling of f.name anyways, just fall back on the mimetype when a filename is passed.

[untitaker]

Would it make sense to only raise an error if a file-like object is passed and
no mimetype could be determined?

On Sun, Sep 11, 2016 at 02:15:30AM -0700, Armin Ronacher wrote:

@untitaker i don't want to reintroduce the handling of f.name anyways, just fall back on the mimetype when a filename is passed.

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#2019 (comment)

[mitsuhiko]

@untitaker that is basically what the current code is doing.

(And no attachment filename that is. That seems like a sensible behavior to me but i can restrict it to fail for streams specifically)

[untitaker]

I mean:

• Fall back to octet-stream when a filepath is passed
• Raise an error if a fileobject is passed

(and without attachment_filename or explicit mimetype)

On Sun, Sep 11, 2016 at 02:26:12AM -0700, Armin Ronacher wrote:

@untitaker that is basically what the current code is doing.

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#2019 (comment)

[untitaker]

👍 for this PR, had a brainfart about possible silent regression when using send_file(open(...)). We had a discussion on IRC.

What needs an update is upgrading.rst, also the docstring of send_file is a bit problematic and vague wrt error behavior IMO.

[untitaker]

LGTM though I think the docstring of send_file needs work in general (not specific to your PR)