From baa38eea7a848ce693d8187e1a7a3ddb6a7fa55a Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Mon, 25 May 2020 08:51:27 +0200 Subject: [PATCH] lib/assets: Allow assets to remain uncompressed (#6661) --- cmd/strelaypoolsrv/main.go | 8 ++---- lib/api/api.go | 2 +- lib/api/api_statics.go | 13 ++++----- lib/api/api_test.go | 13 ++++++--- lib/api/auto/auto_test.go | 5 +++- lib/assets/assets.go | 25 +++++++++++------- lib/assets/assets_test.go | 33 ++++++++++++++++++----- script/genassets.go | 54 ++++++++++++++++++++++++++++---------- 8 files changed, 104 insertions(+), 49 deletions(-) diff --git a/cmd/strelaypoolsrv/main.go b/cmd/strelaypoolsrv/main.go index 32a3edbc3..5ec000fa2 100644 --- a/cmd/strelaypoolsrv/main.go +++ b/cmd/strelaypoolsrv/main.go @@ -268,17 +268,13 @@ func handleAssets(w http.ResponseWriter, r *http.Request) { path = "index.html" } - content, ok := auto.Assets()[path] + as, ok := auto.Assets()[path] if !ok { w.WriteHeader(http.StatusNotFound) return } - assets.Serve(w, r, assets.Asset{ - ContentGz: content, - Filename: path, - Modified: time.Unix(auto.Generated, 0).UTC(), - }) + assets.Serve(w, r, as) } func handleRequest(w http.ResponseWriter, r *http.Request) { diff --git a/lib/api/api.go b/lib/api/api.go index 4f4e54bd2..dd76e50df 100644 --- a/lib/api/api.go +++ b/lib/api/api.go @@ -1267,7 +1267,7 @@ func (s *service) getEventSub(mask events.EventType) events.BufferedSubscription func (s *service) getSystemUpgrade(w http.ResponseWriter, r *http.Request) { if s.noUpgrade { - http.Error(w, upgrade.ErrUpgradeUnsupported.Error(), 500) + http.Error(w, upgrade.ErrUpgradeUnsupported.Error(), http.StatusServiceUnavailable) return } opts := s.cfg.Options() diff --git a/lib/api/api_statics.go b/lib/api/api_statics.go index d0cdd6f74..ce8fe2c62 100644 --- a/lib/api/api_statics.go +++ b/lib/api/api_statics.go @@ -24,7 +24,7 @@ const themePrefix = "theme-assets/" type staticsServer struct { assetDir string - assets map[string]string + assets map[string]assets.Asset availableThemes []string mut sync.RWMutex @@ -118,7 +118,7 @@ func (s *staticsServer) serveAsset(w http.ResponseWriter, r *http.Request) { } // Check for a compiled in asset for the current theme. - bs, ok := s.assets[theme+"/"+file] + as, ok := s.assets[theme+"/"+file] if !ok { // Check for an overridden default asset. if s.assetDir != "" { @@ -134,18 +134,15 @@ func (s *staticsServer) serveAsset(w http.ResponseWriter, r *http.Request) { } // Check for a compiled in default asset. - bs, ok = s.assets[config.DefaultTheme+"/"+file] + as, ok = s.assets[config.DefaultTheme+"/"+file] if !ok { http.NotFound(w, r) return } } - assets.Serve(w, r, assets.Asset{ - ContentGz: bs, - Filename: file, - Modified: modificationTime, - }) + as.Modified = modificationTime + assets.Serve(w, r, as) } func (s *staticsServer) serveThemes(w http.ResponseWriter, r *http.Request) { diff --git a/lib/api/api_test.go b/lib/api/api_test.go index 4cc981bc3..6c6dc74a2 100644 --- a/lib/api/api_test.go +++ b/lib/api/api_test.go @@ -25,6 +25,7 @@ import ( "time" "github.com/d4l3k/messagediff" + "github.com/syncthing/syncthing/lib/assets" "github.com/syncthing/syncthing/lib/config" "github.com/syncthing/syncthing/lib/events" "github.com/syncthing/syncthing/lib/fs" @@ -152,19 +153,25 @@ func TestAssetsDir(t *testing.T) { gw := gzip.NewWriter(buf) gw.Write([]byte("default")) gw.Close() - def := buf.String() + def := assets.Asset{ + Content: buf.String(), + Gzipped: true, + } buf = new(bytes.Buffer) gw = gzip.NewWriter(buf) gw.Write([]byte("foo")) gw.Close() - foo := buf.String() + foo := assets.Asset{ + Content: buf.String(), + Gzipped: true, + } e := &staticsServer{ theme: "foo", mut: sync.NewRWMutex(), assetDir: "testdata", - assets: map[string]string{ + assets: map[string]assets.Asset{ "foo/a": foo, // overridden in foo/a "foo/b": foo, "default/a": def, // overridden in default/a (but foo/a takes precedence) diff --git a/lib/api/auto/auto_test.go b/lib/api/auto/auto_test.go index 73b7736c2..2e997d5e3 100644 --- a/lib/api/auto/auto_test.go +++ b/lib/api/auto/auto_test.go @@ -22,9 +22,12 @@ func TestAssets(t *testing.T) { if !ok { t.Fatal("No index.html in compiled in assets") } + if !idx.Gzipped { + t.Fatal("default/index.html should be compressed") + } var gr *gzip.Reader - gr, _ = gzip.NewReader(strings.NewReader(idx)) + gr, _ = gzip.NewReader(strings.NewReader(idx.Content)) html, _ := ioutil.ReadAll(gr) if !bytes.Contains(html, []byte("Hello, world!` - indexGz := compress(indexHTML) +func TestServe(t *testing.T) { testServe(t, false) } +func TestServeGzip(t *testing.T) { testServe(t, true) } + +func testServe(t *testing.T, gzip bool) { + const indexHTML = `Hello, world!` + content := indexHTML + if gzip { + content = compress(indexHTML) + } handler := func(w http.ResponseWriter, r *http.Request) { Serve(w, r, Asset{ - ContentGz: indexGz, - Filename: r.URL.Path[1:], - Modified: time.Unix(0, 0), + Content: content, + Gzipped: gzip, + Length: len(indexHTML), + Filename: r.URL.Path[1:], + Modified: time.Unix(0, 0), }) } @@ -73,7 +82,17 @@ func TestServe(t *testing.T) { } body, _ := ioutil.ReadAll(res.Body) - if acceptGzip { + + // Content-Length is the number of bytes in the encoded (compressed) body + // (https://stackoverflow.com/a/3819303). + n, err := strconv.Atoi(res.Header.Get("Content-Length")) + if err != nil { + t.Errorf("malformed Content-Length %q", res.Header.Get("Content-Length")) + } else if n != len(body) { + t.Errorf("wrong Content-Length %d, should be %d", n, len(body)) + } + + if gzip && acceptGzip { body = decompress(body) } if string(body) != indexHTML { diff --git a/script/genassets.go b/script/genassets.go index 373c9c5fc..f41c17187 100644 --- a/script/genassets.go +++ b/script/genassets.go @@ -15,6 +15,7 @@ import ( "fmt" "go/format" "io" + "io/ioutil" "os" "path/filepath" "strconv" @@ -27,20 +28,35 @@ var tpl = template.Must(template.New("assets").Parse(`// Code generated by genas package auto -const Generated int64 = {{.Generated}} +import ( + "time" + + "github.com/syncthing/syncthing/lib/assets" +) + +func Assets() map[string]assets.Asset { + var ret = make(map[string]assets.Asset, {{.Assets | len}}) + t := time.Unix({{.Generated}}, 0) -func Assets() map[string]string { - var assets = make(map[string]string, {{.Assets | len}}) {{range $asset := .Assets}} - assets["{{$asset.Name}}"] = {{$asset.Data}}{{end}} - return assets + ret["{{$asset.Name}}"] = assets.Asset{ + Content: {{$asset.Data}}, + Gzipped: {{$asset.Gzipped}}, + Length: {{$asset.Length}}, + Filename: {{$asset.Name | printf "%q"}}, + Modified: t, + } +{{end}} + return ret } `)) type asset struct { - Name string - Data string + Name string + Data string + Length int + Gzipped bool } var assets []asset @@ -57,22 +73,32 @@ func walkerFor(basePath string) filepath.WalkFunc { } if info.Mode().IsRegular() { - fd, err := os.Open(name) + data, err := ioutil.ReadFile(name) if err != nil { return err } + length := len(data) var buf bytes.Buffer - gw := gzip.NewWriter(&buf) - io.Copy(gw, fd) - fd.Close() - gw.Flush() + gw, _ := gzip.NewWriterLevel(&buf, gzip.BestCompression) + gw.Write(data) gw.Close() + // Only replace asset by gzipped version if it is smaller. + // In practice, this means HTML, CSS, SVG etc. get compressed, + // while PNG and WOFF files are left uncompressed. + // lib/assets detects gzip and sets headers/decompresses. + gzipped := buf.Len() < len(data) + if gzipped { + data = buf.Bytes() + } + name, _ = filepath.Rel(basePath, name) assets = append(assets, asset{ - Name: filepath.ToSlash(name), - Data: fmt.Sprintf("%q", buf.String()), + Name: filepath.ToSlash(name), + Data: fmt.Sprintf("%q", string(data)), + Length: length, + Gzipped: gzipped, }) }