New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduces middlewares for serving static content. #143

Merged
merged 19 commits into from May 19, 2016

Conversation

Projects
None yet
3 participants
@arteymix
Member

arteymix commented Dec 15, 2015

These are basic middlewares able to serve content from GLib.File and GLib.Resource. It supports ETag.

It could eventually be coupled with a middleware that supports HTTP caching for optimal performances.

With GVFS, it is even possible to serve remote files using various protocols.

There's a few thing to finish, mainly documentation and tests, before it can be merged.

@arteymix arteymix added the feature label Dec 15, 2015

@arteymix arteymix self-assigned this Dec 15, 2015

@arteymix arteymix added this to the 0.3.0 milestone Dec 15, 2015

Introduces middlewares for serving static content.
Support 'ETag' for serving files and resources and raise a
'304 Not Modified' status when appliable.

For 'GLib.File' delivery, the 'ETag' is obtained from the file
attributes.

For 'GLib.Resource' delivery, the 'ETag' is computed with SHA-1 and
stored in a cache to minimize the computational costs.

Replace the resource serving code in the example application and add a
link to a resource being served with the middleware.

Add minimal documentation.
@antono

This comment has been minimized.

Show comment
Hide comment
@antono

antono Dec 16, 2015

Contributor

Cool stuff! :)

Contributor

antono commented Dec 16, 2015

Cool stuff! :)

arteymix added some commits Dec 16, 2015

Improve for static delivery middlewares
Rename 'serve_resource' for 'serve_resources' and change the default
behaviour to serve global resources. A bundle can be provided optionally
after the prefix.

Only close the source stream when splicing a resource, it might be
useful to keep the target stream open for further operations.
Invoke 'next' if an error occured on data lookup
The most reasonable cause of error at this point is the resource not
being available.
Flag to splice the resource asynchronously
Under some circumstances, it might be preferable to serve resource
synchronously.
Use GLib-friendly flags for 'Static.ServeFlags'
Flags are mandatory for 'serve_from_path' and 'serve_from_resources' so
that the user can opt-in for etag and asynchronous delivery.
Support 'Last-Modified' header for resources delivered from path
Provide consistent headers by producing both 'Last-Modified' and 'ETag'
before raising a '304 Not Modified'.
@arteymix

This comment has been minimized.

Show comment
Hide comment
@arteymix

arteymix Dec 17, 2015

Member

Things are getting better: it now supports Last-Modified andCache-Control: public!

I'll finish the tests and start merging features for the 0.3 series.

Member

arteymix commented Dec 17, 2015

Things are getting better: it now supports Last-Modified andCache-Control: public!

I'll finish the tests and start merging features for the 0.3 series.

Generate minimal headers on 304 Not Modified
Favour 'ETag' over 'Last-Modified' when they are both enabled. It's
redundant to produce both, especially since 'ETag' is a much stronger
validator.

Avoid producing 'ETag', 'Last-Modified' or 'Cache-Control' or any
content-related headers when raising a '304 Not Modified' in static
resources.

Set the encoding to 'Soup.Encoding.NONE' to produce response headers
consistent with no content.
@arteymix

This comment has been minimized.

Show comment
Hide comment
@arteymix

arteymix Dec 18, 2015

Member

I think it's a generally good idea to minimize the header size. The only missing thing would be to strip the Content-Type when it's not necessary.

Member

arteymix commented Dec 18, 2015

I think it's a generally good idea to minimize the header size. The only missing thing would be to strip the Content-Type when it's not necessary.

arteymix added a commit that referenced this pull request Dec 19, 2015

Improve initialization in 'Router.handle'
Do not assign the response status to '200 OK', it's already done by
VSGI.

Since 'VSGI.Soup.Response' overwrites the 'status' property, it has to
be set manually in the server handler.

Do not initialize the 'Content-Type' header: under many circumstances
(like #143), it is better to have minimal headers.

The 'Content-Type' should be set explicitly, it's wrong to assume it
defaults to HTML.

Update tests and example to honor the new behaviour.
@arteymix

This comment has been minimized.

Show comment
Hide comment
@arteymix

arteymix Jan 9, 2016

Member

The Content-Type is not set anymore by default.

Member

arteymix commented Jan 9, 2016

The Content-Type is not set anymore by default.

@arteymix

This comment has been minimized.

Show comment
Hide comment
@arteymix

arteymix Jan 29, 2016

Member

It would be nice to support the X-Sendfile header.

Member

arteymix commented Jan 29, 2016

It would be nice to support the X-Sendfile header.

arteymix added some commits Jan 10, 2016

Handle missing permissions as '404 Not Found'
It's not desirable to let the user agent know that a resource exists if
it's not readable.

Add a flag to explicitly generate a '401 Forbidden' if permissions are
missing.
@arteymix

This comment has been minimized.

Show comment
Hide comment
@arteymix

arteymix Jan 30, 2016

Member

Okay, X-Sendfile work :)

I noticed that it's not being supported by lighttpd mod_scgi, but it works fine with mod_fastcgi.

Member

arteymix commented Jan 30, 2016

Okay, X-Sendfile work :)

I noticed that it's not being supported by lighttpd mod_scgi, but it works fine with mod_fastcgi.

@Bob131

This comment has been minimized.

Show comment
Hide comment
@Bob131

Bob131 Feb 10, 2016

Member

On the topic of X-Sendfile support, could an additional path for NGINX's X-Accel-Redirect be added? AFAIK it requires you to know at header-generation time where exactly the NGINX location directive is rooted. In the past I got around this (rather clunkily) just by having that be configurable, but of course that doesn't really fit into a bit field

Member

Bob131 commented Feb 10, 2016

On the topic of X-Sendfile support, could an additional path for NGINX's X-Accel-Redirect be added? AFAIK it requires you to know at header-generation time where exactly the NGINX location directive is rooted. In the past I got around this (rather clunkily) just by having that be configurable, but of course that doesn't really fit into a bit field

@arteymix

This comment has been minimized.

Show comment
Hide comment
@arteymix

arteymix Feb 11, 2016

Member

The X-Accel-Redirect does not have the same semantics: it expects a relative path that the server actually resolve. It's not really possible to change the code in order to the various headers produced along with the resource.

It would require a more specialized middleware, especially since there's much more to X-Accel: https://www.nginx.com/resources/wiki/start/topics/examples/x-accel/

Member

arteymix commented Feb 11, 2016

The X-Accel-Redirect does not have the same semantics: it expects a relative path that the server actually resolve. It's not really possible to change the code in order to the various headers produced along with the resource.

It would require a more specialized middleware, especially since there's much more to X-Accel: https://www.nginx.com/resources/wiki/start/topics/examples/x-accel/

static: Check if the file is locally available for 'X-Sendfile'
Since 'get_path' can be 'null' if the file is not locally available,
'get_uri' is used instead and 'get_basename' to guess the content type.
@arteymix

This comment has been minimized.

Show comment
Hide comment
@arteymix

arteymix Feb 12, 2016

Member

I added a check for file locality prior to produce the X-Sendfile header and for more compatibility with GVFS, get_uri is used instead if the file is missing.

It's really amazing to just think that we could serve files from any supported backend, which can literally be a massive storage server.

Member

arteymix commented Feb 12, 2016

I added a check for file locality prior to produce the X-Sendfile header and for more compatibility with GVFS, get_uri is used instead if the file is missing.

It's really amazing to just think that we could serve files from any supported backend, which can literally be a massive storage server.

Show outdated Hide outdated src/valum-static.vala
static: Serve the global resources with 'resource://' instead
'Static.serve_from_resource' should be exclusively used to serve bundles
that are not registered.
@arteymix

This comment has been minimized.

Show comment
Hide comment
@arteymix

arteymix Feb 19, 2016

Member

@Bob131 We could provide a contrib library for supporting nginx particularities, I think it has a very wide range of useful features

Member

arteymix commented Feb 19, 2016

@Bob131 We could provide a contrib library for supporting nginx particularities, I think it has a very wide range of useful features

@arteymix arteymix referenced this pull request Feb 21, 2016

Open

Recipes for the framework #44

0 of 5 tasks complete

arteymix added some commits May 16, 2016

Merge branch 'middleware' (early part) into 'master'
Rename 'serve_from_path' for 'serve_from_file' as it essentially serve a
'GLib.File' instance.

Remove 'ServeFlags.ASYNC' since async support will be implemented upon
async delegates.

Skip the payload if the method is 'HEAD'.

Update the documentation and examples.

@arteymix arteymix merged commit 3f5c125 into master May 19, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@arteymix arteymix deleted the middleware branch May 19, 2016

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