Easy file streaming #281

Open
timClicks opened this Issue Jun 15, 2013 · 5 comments

Comments

Projects
None yet
5 participants
@timClicks
Contributor

timClicks commented Jun 15, 2013

I would like to hand the details of streaming files to the client off to the framework. If I could return a file path, Chicago Boss should know what to do.

Here are some proposed controller return values:

{file, Path::string()}
{file, Path::string(), Headers::proplist()}
{file, Path::string(), Headers::proplist(), Options::proplist()}

Some worthwhile features:

  • splits files into some sensible chunk size (can edit with options)
  • automatic compression according to client's Accept-Encoding preferences
  • checks file's metadata for LastModified values
  • supports conditional GET requests

500 errors could be generated in the following cases:

  • undetectable_content_type - system could not determine what the Content-Type to declare the file is
  • various OS/file system errors, such as not having read permissions or does not exist

Some added

@evanmiller

This comment has been minimized.

Show comment
Hide comment
@evanmiller

evanmiller Jun 15, 2013

Contributor

Sounds good to me. I believe SimpleBridge has built-in facilities for sending files that should be easy to hook into. The webserver (Cowboy etc) will handle the chunk sizing, though in practice it will just use sendfile. The webserver should also handle encoding and conditional requests.

BTW the content-type "application/octet-stream" is standard when the file type is not recognized. I am not sure what Cowboy does when it encounters a read error; either 500 or 40X would be sensible.

If you want to take a crack at this check out how CB delivers files from the static directory:

https://github.com/evanmiller/ChicagoBoss/blob/master/src/boss/boss_web_controller.erl#L430

Contributor

evanmiller commented Jun 15, 2013

Sounds good to me. I believe SimpleBridge has built-in facilities for sending files that should be easy to hook into. The webserver (Cowboy etc) will handle the chunk sizing, though in practice it will just use sendfile. The webserver should also handle encoding and conditional requests.

BTW the content-type "application/octet-stream" is standard when the file type is not recognized. I am not sure what Cowboy does when it encounters a read error; either 500 or 40X would be sensible.

If you want to take a crack at this check out how CB delivers files from the static directory:

https://github.com/evanmiller/ChicagoBoss/blob/master/src/boss/boss_web_controller.erl#L430

timClicks added a commit to timClicks/ChicagoBoss that referenced this issue Jun 18, 2013

Add file streaming support; ref #281
This commit adds (buggy) support for streaming a static file
directly from a controller. Three known problems:

 - the status code is not indicated properly, as simple_bridge
   passes this off to the web server
 - hard to know where the docroot is for relative pathnames
 - doesn't seem to support absolute file paths

Still, it's a start.
@evanmiller

This comment has been minimized.

Show comment
Hide comment
@evanmiller

evanmiller Jun 19, 2013

Contributor

The commit looks like a good start. I am not 100% certain but I think the absolute pathname issue is a security "feature" of either SimpleBridge or the web server. As for relative paths I think the only sane thing to do is make them relative to priv/static.

What do you mean "the status code is not indicated properly"? No 304s?

Contributor

evanmiller commented Jun 19, 2013

The commit looks like a good start. I am not 100% certain but I think the absolute pathname issue is a security "feature" of either SimpleBridge or the web server. As for relative paths I think the only sane thing to do is make them relative to priv/static.

What do you mean "the status code is not indicated properly"? No 304s?

@tempaccounterl

This comment has been minimized.

Show comment
Hide comment
@tempaccounterl

tempaccounterl Jun 20, 2013

Contributor

My 2 cents

make them relative to priv/static

IMO this is wrong. this would make these files public for all. priv/static files are usually served directly by nginx.
It would be better to have a special directory for files, served by sendfile, and use relative path in controller response, as it is done in nginx' x-accel-redirect. Option to delete the file after it is sent (regardless of was it sent successfully or not) would be nice too.
Also it would be nice to have an option to directly issue x-accel-redirect/x-send-file header to nginx/apache.

Contributor

tempaccounterl commented Jun 20, 2013

My 2 cents

make them relative to priv/static

IMO this is wrong. this would make these files public for all. priv/static files are usually served directly by nginx.
It would be better to have a special directory for files, served by sendfile, and use relative path in controller response, as it is done in nginx' x-accel-redirect. Option to delete the file after it is sent (regardless of was it sent successfully or not) would be nice too.
Also it would be nice to have an option to directly issue x-accel-redirect/x-send-file header to nginx/apache.

@evanmiller

This comment has been minimized.

Show comment
Hide comment
@evanmiller

evanmiller Jun 20, 2013

Contributor

That makes sense. I am wondering if this should be configurable so you could define a directory outside the application, which would be desirable if you have multiple applications or services involved in file creation. I hadn't thought about deleting but that also makes sense for temporary files.

Contributor

evanmiller commented Jun 20, 2013

That makes sense. I am wondering if this should be configurable so you could define a directory outside the application, which would be desirable if you have multiple applications or services involved in file creation. I hadn't thought about deleting but that also makes sense for temporary files.

@tempaccounterl

This comment has been minimized.

Show comment
Hide comment
@tempaccounterl

tempaccounterl Jun 20, 2013

Contributor

Configurable with some sane defaults, like priv/files/ or even /tmp

Contributor

tempaccounterl commented Jun 20, 2013

Configurable with some sane defaults, like priv/files/ or even /tmp

@ghost ghost assigned zkessin Jan 7, 2014

@danikp danikp modified the milestones: version 1.0, version 0.9 Oct 11, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment