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

Cache-Control headers for static files using ServeFiles #40

Open
einthusan opened this Issue Oct 17, 2014 · 35 comments

Comments

Projects
None yet
9 participants
@einthusan

einthusan commented Oct 17, 2014

I am not sure how to do this... is there support for this? It seems the API does not allow for such thing.

@whit3

This comment has been minimized.

Show comment
Hide comment
@whit3

whit3 Nov 1, 2014

You're right.
ServeFiles doesn't give you access to the http.ResponseWriter for setting his headers.

It would be nice to have a third parameter (like maxAge uint32) for ServeFiles to set the Cache-Control header.

But you can use the same short logic as in the ServeFiles method:

package main

import (
    "github.com/julienschmidt/httprouter"
    "net/http"
)

func main() {
    router := httprouter.New()
    fileServer := http.FileServer(http.Dir("public"))

    router.GET("/static/*filepath", func(w http.ResponseWriter, r *http.Request, p httprouter.Params) {
        w.Header().Set("Vary", "Accept-Encoding")
        w.Header().Set("Cache-Control", "public, max-age=7776000")
        r.URL.Path = p.ByName("filepath")
        fileServer.ServeHTTP(w, r)
    })

    http.ListenAndServe(":8080", router)
}

whit3 commented Nov 1, 2014

You're right.
ServeFiles doesn't give you access to the http.ResponseWriter for setting his headers.

It would be nice to have a third parameter (like maxAge uint32) for ServeFiles to set the Cache-Control header.

But you can use the same short logic as in the ServeFiles method:

package main

import (
    "github.com/julienschmidt/httprouter"
    "net/http"
)

func main() {
    router := httprouter.New()
    fileServer := http.FileServer(http.Dir("public"))

    router.GET("/static/*filepath", func(w http.ResponseWriter, r *http.Request, p httprouter.Params) {
        w.Header().Set("Vary", "Accept-Encoding")
        w.Header().Set("Cache-Control", "public, max-age=7776000")
        r.URL.Path = p.ByName("filepath")
        fileServer.ServeHTTP(w, r)
    })

    http.ListenAndServe(":8080", router)
}
@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Nov 2, 2014

@whit3 Yes, Im already doing something like this for now, but just wanted to raise the issue. Seems like the developer does not have time for this package anymore, not sure if he would even get time to look at a pull request.

einthusan commented Nov 2, 2014

@whit3 Yes, Im already doing something like this for now, but just wanted to raise the issue. Seems like the developer does not have time for this package anymore, not sure if he would even get time to look at a pull request.

@julienschmidt

This comment has been minimized.

Show comment
Hide comment
@julienschmidt

julienschmidt Dec 6, 2014

Owner

Hi @einthusan,
I must admit, that I currently don't have much time for coding as I am studying abroad in the Philippines until next April. This package is still going to be maintained, though.
@whit3's solution is probably the best for now. ServeFiles is only meant as a shortcut. But it might make sense to add another function which allows to pass a Header struct or a handler function as a parameter.

Owner

julienschmidt commented Dec 6, 2014

Hi @einthusan,
I must admit, that I currently don't have much time for coding as I am studying abroad in the Philippines until next April. This package is still going to be maintained, though.
@whit3's solution is probably the best for now. ServeFiles is only meant as a shortcut. But it might make sense to add another function which allows to pass a Header struct or a handler function as a parameter.

@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Dec 7, 2014

@julienschmidt thanks for the response. yes, currently I am doing a work around. great work thus far.

einthusan commented Dec 7, 2014

@julienschmidt thanks for the response. yes, currently I am doing a work around. great work thus far.

@chespinoza

This comment has been minimized.

Show comment
Hide comment
@chespinoza

chespinoza Dec 29, 2014

I need something like that in order to use gzipped assets. Then maybe will be nice to have a more generic way to set it in the API.

chespinoza commented Dec 29, 2014

I need something like that in order to use gzipped assets. Then maybe will be nice to have a more generic way to set it in the API.

@julienschmidt

This comment has been minimized.

Show comment
Hide comment
@julienschmidt

julienschmidt Jan 22, 2015

Owner

My current idea is to provide a second function, which allows to pass a filter / handler / callback function.
Something like the following:

func serveFilesFilter(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
        w.Header().Set("Vary", "Accept-Encoding")
        w.Header().Set("Cache-Control", "public, max-age=7776000")
}

router.ServeFiles_New("/src/*filepath", http.Dir("/var/www"), serveFilesFilter)

Comments? Naming suggestions?

Owner

julienschmidt commented Jan 22, 2015

My current idea is to provide a second function, which allows to pass a filter / handler / callback function.
Something like the following:

func serveFilesFilter(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
        w.Header().Set("Vary", "Accept-Encoding")
        w.Header().Set("Cache-Control", "public, max-age=7776000")
}

router.ServeFiles_New("/src/*filepath", http.Dir("/var/www"), serveFilesFilter)

Comments? Naming suggestions?

@whit3

This comment has been minimized.

Show comment
Hide comment
@whit3

whit3 Jan 22, 2015

A ServeFiles-like function is only used for serving files directly from the provided http.Dir so:

  • This function already writes in the http.ResponseWriter's body : a whole access to it can be misleading and error-prone.
    I would just provide the http.Header (given by the w.Header() function) instead of the whole http.ResponseWriter.
  • I don't see why the *http.Request could be useful:
    • The request body doesn't have an impact over the served file.
    • The request headers can't contain interesting key/values for the response.
      A r.Header.Get("Accept-Encoding") would be interesting if the body was writable, but it's not.
  • I don't see why the whole httprouter.Params could be useful: only the filepath parameter could be used for logging or something like that.

Here is my proposition:

func fileCacheSetter(h http.Header, fp string) {
    log.Printf("Requested file path: %s\n", fp)
    h.Set("Vary", "Accept-Encoding")
    h.Set("Cache-Control", "public, max-age=7776000")
}

router.ServeHeadedFiles("/src/*filepath", http.Dir("/var/www"), fileCacheSetter)

Or, even better (for me), you could just provide a "one-line magic" function with a built-in gzipping and cache headers setting for a given max-age:

router.ServeStaticFiles("/src/*filepath", http.Dir("/var/www"), 7776000)

whit3 commented Jan 22, 2015

A ServeFiles-like function is only used for serving files directly from the provided http.Dir so:

  • This function already writes in the http.ResponseWriter's body : a whole access to it can be misleading and error-prone.
    I would just provide the http.Header (given by the w.Header() function) instead of the whole http.ResponseWriter.
  • I don't see why the *http.Request could be useful:
    • The request body doesn't have an impact over the served file.
    • The request headers can't contain interesting key/values for the response.
      A r.Header.Get("Accept-Encoding") would be interesting if the body was writable, but it's not.
  • I don't see why the whole httprouter.Params could be useful: only the filepath parameter could be used for logging or something like that.

Here is my proposition:

func fileCacheSetter(h http.Header, fp string) {
    log.Printf("Requested file path: %s\n", fp)
    h.Set("Vary", "Accept-Encoding")
    h.Set("Cache-Control", "public, max-age=7776000")
}

router.ServeHeadedFiles("/src/*filepath", http.Dir("/var/www"), fileCacheSetter)

Or, even better (for me), you could just provide a "one-line magic" function with a built-in gzipping and cache headers setting for a given max-age:

router.ServeStaticFiles("/src/*filepath", http.Dir("/var/www"), 7776000)
@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Jan 22, 2015

Please no "one-line magic" functions.. we use the cgo (youtube/vitess) gzip package, not the std go gzip package. What package would httprouter use? better leave that out of the equation. it would be too restrictive to force magic or result in a large API footprint.

einthusan commented Jan 22, 2015

Please no "one-line magic" functions.. we use the cgo (youtube/vitess) gzip package, not the std go gzip package. What package would httprouter use? better leave that out of the equation. it would be too restrictive to force magic or result in a large API footprint.

@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Jan 22, 2015

Things to consider

  • different filetypes may need different expiry values
  • instant cache buster url param "/src/:buster/*filepath" where buster might be app version
  • maybe the need to not serve specific files for users without certain cookies and etc. (auth)

It would be better to use

func serveFilesFilter(w http.ResponseWriter, r *http.Request, ps httprouter.Params)

since it will take care of unforeseeable use cases

einthusan commented Jan 22, 2015

Things to consider

  • different filetypes may need different expiry values
  • instant cache buster url param "/src/:buster/*filepath" where buster might be app version
  • maybe the need to not serve specific files for users without certain cookies and etc. (auth)

It would be better to use

func serveFilesFilter(w http.ResponseWriter, r *http.Request, ps httprouter.Params)

since it will take care of unforeseeable use cases

@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Jan 22, 2015

as for naming, router.ServeFilesFiltered

einthusan commented Jan 22, 2015

as for naming, router.ServeFilesFiltered

@whit3

This comment has been minimized.

Show comment
Hide comment
@whit3

whit3 Jan 22, 2015

We all know there are a lot of different file serving situations:

  • cache strategies
  • GZIP
  • minification
  • compilation (CoffeeScript, SASS, template languages…)
  • on-the-fly image resizing
  • and so on…

We can't provide an omniscient solution, and because of that, any function like ServeFiles (without a full control over the response head and body) will always be somehow "magical" and opinionated.

It's exactly like the Go's built-in http.ServeFile… This function copies the requested file content into the http.ResponseWriter and redirects /index.html to /, with zero configuration.
It's opinionated (but almost useless in production because of the too few features).

So we have 2 options:

  • A very generic files serving func that will give a file reader and a response writer.
    But it will lead to a code similar to the actual workaround and I think it's useless to create a new func for a similar solution.
  • A new/updated ServeFile-like func, but it will require to make some choices and solve the most common need for a classic static content:
    • Vary: Accept-Encoding
    • Cache-Control with a time setting (and maybe adapted to Content-Type)
    • ETag and/or Last-Modified
    • gzipped body (with the Go's compress/gzip, no external dependencies) if Accept-Encoding: gzip

At the end, I think the questions are "What background features this new function needs to provide and what needs to be configurable?" or… "Do we really need a new ServeFilesfunction?".

whit3 commented Jan 22, 2015

We all know there are a lot of different file serving situations:

  • cache strategies
  • GZIP
  • minification
  • compilation (CoffeeScript, SASS, template languages…)
  • on-the-fly image resizing
  • and so on…

We can't provide an omniscient solution, and because of that, any function like ServeFiles (without a full control over the response head and body) will always be somehow "magical" and opinionated.

It's exactly like the Go's built-in http.ServeFile… This function copies the requested file content into the http.ResponseWriter and redirects /index.html to /, with zero configuration.
It's opinionated (but almost useless in production because of the too few features).

So we have 2 options:

  • A very generic files serving func that will give a file reader and a response writer.
    But it will lead to a code similar to the actual workaround and I think it's useless to create a new func for a similar solution.
  • A new/updated ServeFile-like func, but it will require to make some choices and solve the most common need for a classic static content:
    • Vary: Accept-Encoding
    • Cache-Control with a time setting (and maybe adapted to Content-Type)
    • ETag and/or Last-Modified
    • gzipped body (with the Go's compress/gzip, no external dependencies) if Accept-Encoding: gzip

At the end, I think the questions are "What background features this new function needs to provide and what needs to be configurable?" or… "Do we really need a new ServeFilesfunction?".

@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Jan 22, 2015

I would say the question is "Do we really need a new ServeFiles function?" and my answer is yes.

The current one is just too limited, and the workaround is so much more code. Mind you that the workaround is missing some lines of code..

if len(path) < 10 || path[len(path)-10:] != "/*filepath" {
        panic("path must end with /*filepath")
    }

The workaround feels like too much heavy lifting is required, and complicated for beginners to Go. They just want a simple fileserver to begin with, and later be able to expand their feature set as their application grows.

If we make the last param be optional, using the trick below, i think it would be better as it wouldn't need another name and it wouldn't break backwards compatibility...

func ServeFiles(path string, root http.FileSystem, filter ...Handle)

Why can't we just do something like this? It's so much more simpler, especially if an example as such is provided, it's more easily understandable.

package main

import (
    "net/http"

    "github.com/julienschmidt/httprouter"
)

func main() {

    router := httprouter.New()

    // simple file server
    router.ServeFiles("/src/*filepath", http.Dir("/var/www"))

    // custom file server
    router.ServeFiles("/src/*filepath", http.Dir("/var/www"), func(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
        w.Header().Set("Vary", "Accept-Encoding")
        w.Header().Set("Cache-Control", "public, max-age=7776000")
    })

    http.ListenAndServe(":8080", router)

}

einthusan commented Jan 22, 2015

I would say the question is "Do we really need a new ServeFiles function?" and my answer is yes.

The current one is just too limited, and the workaround is so much more code. Mind you that the workaround is missing some lines of code..

if len(path) < 10 || path[len(path)-10:] != "/*filepath" {
        panic("path must end with /*filepath")
    }

The workaround feels like too much heavy lifting is required, and complicated for beginners to Go. They just want a simple fileserver to begin with, and later be able to expand their feature set as their application grows.

If we make the last param be optional, using the trick below, i think it would be better as it wouldn't need another name and it wouldn't break backwards compatibility...

func ServeFiles(path string, root http.FileSystem, filter ...Handle)

Why can't we just do something like this? It's so much more simpler, especially if an example as such is provided, it's more easily understandable.

package main

import (
    "net/http"

    "github.com/julienschmidt/httprouter"
)

func main() {

    router := httprouter.New()

    // simple file server
    router.ServeFiles("/src/*filepath", http.Dir("/var/www"))

    // custom file server
    router.ServeFiles("/src/*filepath", http.Dir("/var/www"), func(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
        w.Header().Set("Vary", "Accept-Encoding")
        w.Header().Set("Cache-Control", "public, max-age=7776000")
    })

    http.ListenAndServe(":8080", router)

}
@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Jan 23, 2015

Now, if you want more features, the package can provide some defaults.

// custom file server
router.ServeFiles("/src/*filepath", http.Dir("/var/www"), httprouter.BasicFilter(3*time.Hour), httprouter.GzipFilter(9), MyLessFilter())

einthusan commented Jan 23, 2015

Now, if you want more features, the package can provide some defaults.

// custom file server
router.ServeFiles("/src/*filepath", http.Dir("/var/www"), httprouter.BasicFilter(3*time.Hour), httprouter.GzipFilter(9), MyLessFilter())
@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Jan 23, 2015

or more neatly written example,

    // custom file server
    router.ServeFiles("/src/*filepath", http.Dir("/var/www"),
        httprouter.BasicFilter(3*time.Hour),
        httprouter.GzipFilter(9),
        MyLessFilter(),
    )

einthusan commented Jan 23, 2015

or more neatly written example,

    // custom file server
    router.ServeFiles("/src/*filepath", http.Dir("/var/www"),
        httprouter.BasicFilter(3*time.Hour),
        httprouter.GzipFilter(9),
        MyLessFilter(),
    )
@whit3

This comment has been minimized.

Show comment
Hide comment
@whit3

whit3 Jan 23, 2015

A patched ServeFiles seems like a good idea:

func ServeFiles(path string, root http.FileSystem, filter ...Handle)

But could you explain where do you write the response body?

'Cause with…

// simple file server
router.ServeFiles("/src/*filepath", http.Dir("/var/www"))

...the body would be written in the httprouter.ServeFiles.

But with…

// custom file server
router.ServeFiles("/src/*filepath", http.Dir("/var/www"), httprouter.BasicFilter(3*time.Hour), httprouter.GzipFilter(9), MyLessFilter())

...the body would be written in the MyLessFilter() and httprouter.GzipFilter(9) filters (and the order is significant). NOT in httprouter.ServeFiles, like previously.
With multiple writing situations, the ServeFiles func will need to always check if the response writer is already used or not.

And if these filters only have the classic 3 arguments (w http.ResponseWriter, r *http.Request, ps httprouter.Params), you will need to manually open the file from the filepath param to process it.
Finally, you don't win so much over the actual workaround.

Also, with multiple filters that pass data one to another (CoffeeScript -> UglifyJS -> GZIP for example), we can't directly use the http.ResponseWriter, we need a middleware system, and "httprouter" becomes "httpmicroframework". :)

whit3 commented Jan 23, 2015

A patched ServeFiles seems like a good idea:

func ServeFiles(path string, root http.FileSystem, filter ...Handle)

But could you explain where do you write the response body?

'Cause with…

// simple file server
router.ServeFiles("/src/*filepath", http.Dir("/var/www"))

...the body would be written in the httprouter.ServeFiles.

But with…

// custom file server
router.ServeFiles("/src/*filepath", http.Dir("/var/www"), httprouter.BasicFilter(3*time.Hour), httprouter.GzipFilter(9), MyLessFilter())

...the body would be written in the MyLessFilter() and httprouter.GzipFilter(9) filters (and the order is significant). NOT in httprouter.ServeFiles, like previously.
With multiple writing situations, the ServeFiles func will need to always check if the response writer is already used or not.

And if these filters only have the classic 3 arguments (w http.ResponseWriter, r *http.Request, ps httprouter.Params), you will need to manually open the file from the filepath param to process it.
Finally, you don't win so much over the actual workaround.

Also, with multiple filters that pass data one to another (CoffeeScript -> UglifyJS -> GZIP for example), we can't directly use the http.ResponseWriter, we need a middleware system, and "httprouter" becomes "httpmicroframework". :)

@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Jan 23, 2015

Let me know if you need an example or not, but let me try to be quick about it.

You would pass in a "fake" w which is just a bytes.buffer. Each filter would be responsible for draining the fake w using io.Copy into a temp buffer, and doing their part of the work on it, and then performing another io.Copy to re-fill the fake w.

This passing around of the fake w should allow the package to regain control and ensure that only it has the ability to write to the actual client using the "real" w.

Yes, your correct this means we can't use http.ResponseWriter directly, but it was you who suggested that we have features like Gzip, so eventually even your proposal would lead to the same pattern.

About this becoming a micro framework, I didn't request for built-in features, I just need something so that I can write my own micro framework, and what other way to write my own micro framework than use the technique i suggested?

einthusan commented Jan 23, 2015

Let me know if you need an example or not, but let me try to be quick about it.

You would pass in a "fake" w which is just a bytes.buffer. Each filter would be responsible for draining the fake w using io.Copy into a temp buffer, and doing their part of the work on it, and then performing another io.Copy to re-fill the fake w.

This passing around of the fake w should allow the package to regain control and ensure that only it has the ability to write to the actual client using the "real" w.

Yes, your correct this means we can't use http.ResponseWriter directly, but it was you who suggested that we have features like Gzip, so eventually even your proposal would lead to the same pattern.

About this becoming a micro framework, I didn't request for built-in features, I just need something so that I can write my own micro framework, and what other way to write my own micro framework than use the technique i suggested?

@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Jan 23, 2015

Oh and your right about order being important, my example is wrong just for the record, it does gzip and cache before less filter lol.

einthusan commented Jan 23, 2015

Oh and your right about order being important, my example is wrong just for the record, it does gzip and cache before less filter lol.

@whit3

This comment has been minimized.

Show comment
Hide comment
@whit3

whit3 Jan 23, 2015

OK, I see.

But my proposal was to just embed gzipping (and cache headers) inside the httprouter package.
And for that, there is no need to expose any writer.
This pattern would simply lead to:

// 3 simple arguments and all the work in julienschmidt/httprouter:
//
//                      URL pattern          Source directory     Max-age
router.ServeStaticFiles("/static/*filepath", http.Dir("/public"), 3*time.Hour)

It's opinionated (but like I said, the http.ServeFileand httprouter.ServeFilesthemselves are already opinionated: they suppose that index.html is the root and that you don't want cache, GZIP or others).

In summary:

  • You want a middleware system.
    But in this case, why not to apply this feature to the entire router and not just the ServeFilesfunc?
    The "problem" is that middleware capability transforms any router into a (micro)framework, like Martini, Goji or Gin (himself based on httprouter).
  • I want a simple ServeStaticFiles func.
    It's perfect to serve the assets of a simple website, with a single line of code.
    For more specific needs, I would use the workaround, or write my own middleware package on top of httprouter, or use an existent framework.

httprouter is described as "a lightweight high performance HTTP request router".
A middleware capability would break that concept.


I just need something so that I can write my own micro framework, and what other way to write my own micro framework than use the technique i suggested?

Just write it on top of httprouter, like Gin has done. No?

whit3 commented Jan 23, 2015

OK, I see.

But my proposal was to just embed gzipping (and cache headers) inside the httprouter package.
And for that, there is no need to expose any writer.
This pattern would simply lead to:

// 3 simple arguments and all the work in julienschmidt/httprouter:
//
//                      URL pattern          Source directory     Max-age
router.ServeStaticFiles("/static/*filepath", http.Dir("/public"), 3*time.Hour)

It's opinionated (but like I said, the http.ServeFileand httprouter.ServeFilesthemselves are already opinionated: they suppose that index.html is the root and that you don't want cache, GZIP or others).

In summary:

  • You want a middleware system.
    But in this case, why not to apply this feature to the entire router and not just the ServeFilesfunc?
    The "problem" is that middleware capability transforms any router into a (micro)framework, like Martini, Goji or Gin (himself based on httprouter).
  • I want a simple ServeStaticFiles func.
    It's perfect to serve the assets of a simple website, with a single line of code.
    For more specific needs, I would use the workaround, or write my own middleware package on top of httprouter, or use an existent framework.

httprouter is described as "a lightweight high performance HTTP request router".
A middleware capability would break that concept.


I just need something so that I can write my own micro framework, and what other way to write my own micro framework than use the technique i suggested?

Just write it on top of httprouter, like Gin has done. No?

@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Jan 24, 2015

@whit3 lol... okay well to be honest, I have been working on a closed source full-featured go web framework for the past 1.5 year, and we did some benchmarks on the gzip packages available, and the cgo based package by youtube/vitess project was amazingly better. Hence, I got convinced into making sure you didn't get to put go's standard gzip package in here. But of-course, a simple web application wouldn't care and probably just needs something that works out of the box without complications.

I actually don't have any problems at all using the workaround as that is what my web framework is doing, and there wouldn't be much of an impact to me even if this issue wasn't closed. However, it would affect others that need a simple solution, so I'd admit defeat only on that point.

Aside from that, I've been thinking if it would make sense to open-source the framework that I have been working on as a team, but not sure the current Go crowd would understand why there has to be another web framework among many others. But that's beyond the scope of this issue. :)

You win!

By the way, you can probably do something like this.

type StaticFileOption struct {
    MimeType    string
    CacheExpire time.Duration
    GzipLevel   int
}

And here is the examples

// defaults the cache expiry to what google page speed thinks is acceptable (i think 7 days)
// defaults gzip level to 1
router.ServeStaticFiles("/static/*filepath", http.Dir("/public"))
// for more control, you can do this
// but mime types that are not defined use default behaviour
router.ServeStaticFiles("/static/*filepath", http.Dir("/public"),
        StaticFileOption{MimeType: "text/css", CacheExpire: 24 * time.Hour},
        StaticFileOption{MimeType: "text/javascript", CacheExpire: 1 * time.Hour},
    )

Just another possible way of doing it..

einthusan commented Jan 24, 2015

@whit3 lol... okay well to be honest, I have been working on a closed source full-featured go web framework for the past 1.5 year, and we did some benchmarks on the gzip packages available, and the cgo based package by youtube/vitess project was amazingly better. Hence, I got convinced into making sure you didn't get to put go's standard gzip package in here. But of-course, a simple web application wouldn't care and probably just needs something that works out of the box without complications.

I actually don't have any problems at all using the workaround as that is what my web framework is doing, and there wouldn't be much of an impact to me even if this issue wasn't closed. However, it would affect others that need a simple solution, so I'd admit defeat only on that point.

Aside from that, I've been thinking if it would make sense to open-source the framework that I have been working on as a team, but not sure the current Go crowd would understand why there has to be another web framework among many others. But that's beyond the scope of this issue. :)

You win!

By the way, you can probably do something like this.

type StaticFileOption struct {
    MimeType    string
    CacheExpire time.Duration
    GzipLevel   int
}

And here is the examples

// defaults the cache expiry to what google page speed thinks is acceptable (i think 7 days)
// defaults gzip level to 1
router.ServeStaticFiles("/static/*filepath", http.Dir("/public"))
// for more control, you can do this
// but mime types that are not defined use default behaviour
router.ServeStaticFiles("/static/*filepath", http.Dir("/public"),
        StaticFileOption{MimeType: "text/css", CacheExpire: 24 * time.Hour},
        StaticFileOption{MimeType: "text/javascript", CacheExpire: 1 * time.Hour},
    )

Just another possible way of doing it..

@whit3

This comment has been minimized.

Show comment
Hide comment
@whit3

whit3 Jan 24, 2015

[…] the cgo based package by youtube/vitess project was amazingly better. Hence, I got convinced into making sure you didn't get to put go's standard gzip package in here.

Best GZIP from third-party package vs. basic GZIP from standard library…
For the actual usage of ServeFiles, no matter, it can't go wrong.

By the way, you can probably do something like this.

type StaticFileOption struct {
    MimeType    string
    CacheExpire time.Duration
    GzipLevel   int
}

Why not, but this structure (which is a good idea) should be named StaticFilesOptions and be part of httprouter:

router.ServeStaticFiles("/static/*filepath", http.Dir("/public"),
    &httprouter.StaticFilesOptions{
        MIMEType:    "text/css",
        CacheExpire: 24 * time.Hour,
    },
    &httprouter.StaticFilesOptions{
        MimeType:    "text/javascript",
        CacheExpire: 1 * time.Hour,
    },
)

Aside from that, I've been thinking if it would make sense to open-source the framework that I have been working on as a team, but not sure the current Go crowd would understand why there has to be another web framework among many others.

I'm also developing 2 frameworks (one for REST APIs, another for full websites) with very modular components.

Let the things happen! Nobody refuses new ideas and visions. :)

But I think we are much to develop routers, custom frameworks and other stuff around net/http and I feel like open source frameworks in Go are more like sources of inspiration or temporary tools.
Not necessarily because of the quality of these packages, but because that the need for light and custom solutions quickly arise with a language like Go and its stdlib.

whit3 commented Jan 24, 2015

[…] the cgo based package by youtube/vitess project was amazingly better. Hence, I got convinced into making sure you didn't get to put go's standard gzip package in here.

Best GZIP from third-party package vs. basic GZIP from standard library…
For the actual usage of ServeFiles, no matter, it can't go wrong.

By the way, you can probably do something like this.

type StaticFileOption struct {
    MimeType    string
    CacheExpire time.Duration
    GzipLevel   int
}

Why not, but this structure (which is a good idea) should be named StaticFilesOptions and be part of httprouter:

router.ServeStaticFiles("/static/*filepath", http.Dir("/public"),
    &httprouter.StaticFilesOptions{
        MIMEType:    "text/css",
        CacheExpire: 24 * time.Hour,
    },
    &httprouter.StaticFilesOptions{
        MimeType:    "text/javascript",
        CacheExpire: 1 * time.Hour,
    },
)

Aside from that, I've been thinking if it would make sense to open-source the framework that I have been working on as a team, but not sure the current Go crowd would understand why there has to be another web framework among many others.

I'm also developing 2 frameworks (one for REST APIs, another for full websites) with very modular components.

Let the things happen! Nobody refuses new ideas and visions. :)

But I think we are much to develop routers, custom frameworks and other stuff around net/http and I feel like open source frameworks in Go are more like sources of inspiration or temporary tools.
Not necessarily because of the quality of these packages, but because that the need for light and custom solutions quickly arise with a language like Go and its stdlib.

@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Jan 25, 2015

@whit3 I was using go playground to ensure the code was correct, but forgot to put back httprouter. I actually did mean it the way you re-wrote the code (struct inside httprouter).

Let the things happen! Nobody refuses new ideas and visions. :)

The documentation part is being a killer.. and it's still not completed to 1.0 status
You can see it in action at http://wojka.com (mobile only website but still works on desktop of-course)..

I'm also developing 2 frameworks (one for REST APIs, another for full websites) with very modular components.

Is your framework that is geared towards full websites open sourced? If so, any links to it?

einthusan commented Jan 25, 2015

@whit3 I was using go playground to ensure the code was correct, but forgot to put back httprouter. I actually did mean it the way you re-wrote the code (struct inside httprouter).

Let the things happen! Nobody refuses new ideas and visions. :)

The documentation part is being a killer.. and it's still not completed to 1.0 status
You can see it in action at http://wojka.com (mobile only website but still works on desktop of-course)..

I'm also developing 2 frameworks (one for REST APIs, another for full websites) with very modular components.

Is your framework that is geared towards full websites open sourced? If so, any links to it?

@whit3

This comment has been minimized.

Show comment
Hide comment
@whit3

whit3 Jan 25, 2015

The documentation part is being a killer…

Always! :)

Is your framework that is geared towards full websites open sourced?

I'm only at the beginning (I develop full-time in Go only since october 2014).

I'm actually using httprouter and not very DRY code.
Too much logic is duplicated between my web projects and none of the existent Go (micro)frameworks can satisfy me (for various reasons like no idiomatic code, bad var/func/struct/interface naming, lack of modularity, wrong balance between performance and abstraction, and many others).

That's why I will take time to create the base packages (router, middleware handler, authentication, validations, i18n, CDN-compatible assets management, templates, cookies & sessions, simple real-time with WebSockets, and some others).
With these independent packages, I'll compound my 2 homogenous frameworks.

It's a lot of work but when all is done (tested, used in production and documented) it will be open sourced (this year normally, with another GitHub account).


Anyway, what do you think about the actual propositions for this issue @julienschmidt?

whit3 commented Jan 25, 2015

The documentation part is being a killer…

Always! :)

Is your framework that is geared towards full websites open sourced?

I'm only at the beginning (I develop full-time in Go only since october 2014).

I'm actually using httprouter and not very DRY code.
Too much logic is duplicated between my web projects and none of the existent Go (micro)frameworks can satisfy me (for various reasons like no idiomatic code, bad var/func/struct/interface naming, lack of modularity, wrong balance between performance and abstraction, and many others).

That's why I will take time to create the base packages (router, middleware handler, authentication, validations, i18n, CDN-compatible assets management, templates, cookies & sessions, simple real-time with WebSockets, and some others).
With these independent packages, I'll compound my 2 homogenous frameworks.

It's a lot of work but when all is done (tested, used in production and documented) it will be open sourced (this year normally, with another GitHub account).


Anyway, what do you think about the actual propositions for this issue @julienschmidt?

@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Jan 26, 2015

none of the existent Go (micro)frameworks can satisfy me (for various reasons like no idiomatic code, bad var/func/struct/interface naming, lack of modularity, wrong balance between performance and abstraction, and many others).

Seriously, I see the EXACT same problem. I also put readability (idiomatic Go & var/func/struct/interface naming) at the top of the list. It's an event based framework, and uses web-sockets as default and fall-back as Ajax. Both client side (javascript) and back-end code is extremely easy to read.

But why re-write code when some great code exists?

  • router (using httprouter package)
  • template (std go template package)
  • cookies & session (using only the securecookie package from gorilla)
  • websockets (websockets package by gorilla)

@whit3 can you send me an email? I'd like to discuss if your down for that. "einthusan @ paperboardinc.com"

einthusan commented Jan 26, 2015

none of the existent Go (micro)frameworks can satisfy me (for various reasons like no idiomatic code, bad var/func/struct/interface naming, lack of modularity, wrong balance between performance and abstraction, and many others).

Seriously, I see the EXACT same problem. I also put readability (idiomatic Go & var/func/struct/interface naming) at the top of the list. It's an event based framework, and uses web-sockets as default and fall-back as Ajax. Both client side (javascript) and back-end code is extremely easy to read.

But why re-write code when some great code exists?

  • router (using httprouter package)
  • template (std go template package)
  • cookies & session (using only the securecookie package from gorilla)
  • websockets (websockets package by gorilla)

@whit3 can you send me an email? I'd like to discuss if your down for that. "einthusan @ paperboardinc.com"

@fiorix

This comment has been minimized.

Show comment
Hide comment
@fiorix

fiorix Jan 26, 2015

I also agree 100% and would be interested in having an open discussion about this. Keep me in the loop.

Sent from the future

On Jan 25, 2015, at 8:25 PM, Einthusan Vigneswaran notifications@github.com wrote:

none of the existent Go (micro)frameworks can satisfy me (for various reasons like no idiomatic code, bad var/func/struct/interface naming, lack of modularity, wrong balance between performance and abstraction, and many others).

Seriously, I see the EXACT same problem. I also put readability (idiomatic Go & var/func/struct/interface naming) at the top of the list. It's an event based framework, and uses web-sockets as default and fall-back as Ajax. Both client side (javascript) and back-end code is extremely easy to read.

But why re-write code when some great code exists?

router (using httprouter package)
template (std go template package)
cookies & session (using only the securecookie package from gorilla)
websockets (websockets package by gorilla)
@whit3 can you send me an email? I'd like to discuss if your down for that. "einthusan @ paperboardinc.com"


Reply to this email directly or view it on GitHub.

fiorix commented Jan 26, 2015

I also agree 100% and would be interested in having an open discussion about this. Keep me in the loop.

Sent from the future

On Jan 25, 2015, at 8:25 PM, Einthusan Vigneswaran notifications@github.com wrote:

none of the existent Go (micro)frameworks can satisfy me (for various reasons like no idiomatic code, bad var/func/struct/interface naming, lack of modularity, wrong balance between performance and abstraction, and many others).

Seriously, I see the EXACT same problem. I also put readability (idiomatic Go & var/func/struct/interface naming) at the top of the list. It's an event based framework, and uses web-sockets as default and fall-back as Ajax. Both client side (javascript) and back-end code is extremely easy to read.

But why re-write code when some great code exists?

router (using httprouter package)
template (std go template package)
cookies & session (using only the securecookie package from gorilla)
websockets (websockets package by gorilla)
@whit3 can you send me an email? I'd like to discuss if your down for that. "einthusan @ paperboardinc.com"


Reply to this email directly or view it on GitHub.

@chakrit

This comment has been minimized.

Show comment
Hide comment
@chakrit

chakrit Feb 15, 2015

Just want to weight in that I like @einthusan 's solution if only because having proper composition is much more future proof than any magic code. The minimalistic argument also do not hold as I don't see any difference in minimality between the 2 approaches, in fact, the magic approach is less minimal since that'd mean the ServeFiles function would be doing more than just serving files. It will become a black box that you cannot change and then if you really need to change it, then you'd be required to fork this project or re-implement everything instead of just swapping out the buggy part.

I don't think it's a good idea to idiot-proof an API but lose composition in the process. Sensible defaults is not that hard use even for newbie. I'd say that if anyone can't figure out the how of einthusan's approach he/she shouldn't be touching any routing code at all.

And that the opinionated solution is very un go-like, but that might be just me.

chakrit commented Feb 15, 2015

Just want to weight in that I like @einthusan 's solution if only because having proper composition is much more future proof than any magic code. The minimalistic argument also do not hold as I don't see any difference in minimality between the 2 approaches, in fact, the magic approach is less minimal since that'd mean the ServeFiles function would be doing more than just serving files. It will become a black box that you cannot change and then if you really need to change it, then you'd be required to fork this project or re-implement everything instead of just swapping out the buggy part.

I don't think it's a good idea to idiot-proof an API but lose composition in the process. Sensible defaults is not that hard use even for newbie. I'd say that if anyone can't figure out the how of einthusan's approach he/she shouldn't be touching any routing code at all.

And that the opinionated solution is very un go-like, but that might be just me.

@Puffton

This comment has been minimized.

Show comment
Hide comment
@Puffton

Puffton Jul 3, 2015

Related question: I just implemented gzip compression in my project, but since I use both httprouter.Handle and http.Handler (static files, r.NotFound), I had to duplicate the code a bit:

    func C(h httprouter.Handle) httprouter.Handle {
        return func(w http.ResponseWriter, r *http.Request, p httprouter.Params) {
            w.Header().Add("Vary", "Accept-Encoding")
            if !strings.Contains(r.Header.Get("Accept-Encoding"), "gzip") {
                h(w, r, p)
                return
            }
            w.Header().Set("Content-Encoding", "gzip")
            gz := gzip.NewWriter(w)
            defer gz.Close()
            h(&gzipResponseWriter{Writer: gz, ResponseWriter: w}, r, p)
        }
    }

    func Compress(h http.Handler) http.Handler {
        return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
            w.Header().Add("Vary", "Accept-Encoding")
            if !strings.Contains(r.Header.Get("Accept-Encoding"), "gzip") {
                h.ServeHTTP(w, r)
                return
            }
            w.Header().Set("Content-Encoding", "gzip")
            gz := gzip.NewWriter(w)
            defer gz.Close()
            h.ServeHTTP(&gzipResponseWriter{Writer: gz, ResponseWriter: w}, r)
        })
    }

Is there any way I could make this look prettier given the current circumstances?

Puffton commented Jul 3, 2015

Related question: I just implemented gzip compression in my project, but since I use both httprouter.Handle and http.Handler (static files, r.NotFound), I had to duplicate the code a bit:

    func C(h httprouter.Handle) httprouter.Handle {
        return func(w http.ResponseWriter, r *http.Request, p httprouter.Params) {
            w.Header().Add("Vary", "Accept-Encoding")
            if !strings.Contains(r.Header.Get("Accept-Encoding"), "gzip") {
                h(w, r, p)
                return
            }
            w.Header().Set("Content-Encoding", "gzip")
            gz := gzip.NewWriter(w)
            defer gz.Close()
            h(&gzipResponseWriter{Writer: gz, ResponseWriter: w}, r, p)
        }
    }

    func Compress(h http.Handler) http.Handler {
        return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
            w.Header().Add("Vary", "Accept-Encoding")
            if !strings.Contains(r.Header.Get("Accept-Encoding"), "gzip") {
                h.ServeHTTP(w, r)
                return
            }
            w.Header().Set("Content-Encoding", "gzip")
            gz := gzip.NewWriter(w)
            defer gz.Close()
            h.ServeHTTP(&gzipResponseWriter{Writer: gz, ResponseWriter: w}, r)
        })
    }

Is there any way I could make this look prettier given the current circumstances?

@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Jul 3, 2015

why do you need to use two different ones.. why not just httprouter.Handle?

einthusan commented Jul 3, 2015

why do you need to use two different ones.. why not just httprouter.Handle?

@Puffton

This comment has been minimized.

Show comment
Hide comment
@Puffton

Puffton Jul 3, 2015

That's what I thought at first, too, but I couldn't seem to pass a http.FileServer to it as such:

r.NotFound = C(http.FileServer(http.Dir("../static"))).ServeHTTP

Puffton commented Jul 3, 2015

That's what I thought at first, too, but I couldn't seem to pass a http.FileServer to it as such:

r.NotFound = C(http.FileServer(http.Dir("../static"))).ServeHTTP
@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Jul 3, 2015

use an anonymous func wrapper inline.

einthusan commented Jul 3, 2015

use an anonymous func wrapper inline.

@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Jul 3, 2015

Or... can you tell me what your case scenario is?... what are you trying to accomplish in the bigger picture?

einthusan commented Jul 3, 2015

Or... can you tell me what your case scenario is?... what are you trying to accomplish in the bigger picture?

@Puffton

This comment has been minimized.

Show comment
Hide comment
@Puffton

Puffton Jul 3, 2015

I'm fairly new to Go, so I'm sorry if it's an easy problem that I'm over-complicating. Both input and output types of C are httprouter.Handle, so I would need both an inline func wrapper in the call to C (http.Handler -> httprouter.Handle) and then wrap the call to C itself (httprouter.Handle -> http.HandleFunc)? If I return httprouter.Handle, I can't use ServeHTTP, right?

Puffton commented Jul 3, 2015

I'm fairly new to Go, so I'm sorry if it's an easy problem that I'm over-complicating. Both input and output types of C are httprouter.Handle, so I would need both an inline func wrapper in the call to C (http.Handler -> httprouter.Handle) and then wrap the call to C itself (httprouter.Handle -> http.HandleFunc)? If I return httprouter.Handle, I can't use ServeHTTP, right?

@einthusan

This comment has been minimized.

Show comment
Hide comment
@einthusan

einthusan Jul 3, 2015

@Puffton I tried all sorts of way to make the code simpler... it just doesn't work... your original solution is the best it can be. I really don't like this new change where the NotFound was changed from httprouter.Handle to http.Handle... @julienschmidt can you explain what this is about... it makes really bad code from my standpoint.

einthusan commented Jul 3, 2015

@Puffton I tried all sorts of way to make the code simpler... it just doesn't work... your original solution is the best it can be. I really don't like this new change where the NotFound was changed from httprouter.Handle to http.Handle... @julienschmidt can you explain what this is about... it makes really bad code from my standpoint.

@chuckhacker

This comment has been minimized.

Show comment
Hide comment
@chuckhacker

chuckhacker Jun 7, 2017

Still an issue in 2017 -- for all practical use cases, I cannot simply use http.FileServer as is because it is woefully inadequate, I always have to write my own three-liner "magic function" (as they are called here) because I want to do something "extreme" like set a custom header for every response.

This is not good software design, and I question whether as a primarily offline-only native software developer, I have the ability to write code in my own custom HTTP response handlers that is even nearly as secure or robust as the original implementation written by the library authors themselves.

chuckhacker commented Jun 7, 2017

Still an issue in 2017 -- for all practical use cases, I cannot simply use http.FileServer as is because it is woefully inadequate, I always have to write my own three-liner "magic function" (as they are called here) because I want to do something "extreme" like set a custom header for every response.

This is not good software design, and I question whether as a primarily offline-only native software developer, I have the ability to write code in my own custom HTTP response handlers that is even nearly as secure or robust as the original implementation written by the library authors themselves.

@prologic

This comment has been minimized.

Show comment
Hide comment
@prologic

prologic Jul 15, 2017

WoW Just ran into this too; the lack of appropriate cache control headers and gzip is a bit disheartening :/ now I have to figure out what magic everyone else has done to get around this lacking builtin support!

prologic commented Jul 15, 2017

WoW Just ran into this too; the lack of appropriate cache control headers and gzip is a bit disheartening :/ now I have to figure out what magic everyone else has done to get around this lacking builtin support!

@prologic

This comment has been minimized.

Show comment
Hide comment
@prologic

prologic commented Apr 8, 2018

Bump

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