- Initial commit of the updated static plugin

Signed-off-by: Valery Piashchynski <[email protected]>

5 files changed, 104 insertions, 111 deletions

diff --git a/plugins/static/config.go b/plugins/static/config.go
index 90efea76..2519c04f 100644
--- a/plugins/static/config.go
+++ b/plugins/static/config.go
@@ -2,8 +2,6 @@ package static
 
 import (
 	"os"
-	"path"
-	"strings"
 
 	"github.com/spiral/errors"
 )
@@ -14,10 +12,14 @@ type Config struct {
 		// Dir contains name of directory to control access to.
 		Dir string
 
-		// Forbid specifies list of file extensions which are forbidden for access.
-		// Example: .php, .exe, .bat, .htaccess and etc.
+		// forbid specifies list of file extensions which are forbidden for access.
+		// example: .php, .exe, .bat, .htaccess and etc.
 		Forbid []string
 
+		// Allow specifies list of file extensions which are allowed for access.
+		// example: .php, .exe, .bat, .htaccess and etc.
+		Allow []string
+
 		// Always specifies list of extensions which must always be served by static
 		// service, even if file not found.
 		Always []string
@@ -48,29 +50,3 @@ func (c *Config) Valid() error {
 
 	return nil
 }
-
-// AlwaysForbid must return true if file extension is not allowed for the upload.
-func (c *Config) AlwaysForbid(filename string) bool {
-	ext := strings.ToLower(path.Ext(filename))
-
-	for _, v := range c.Static.Forbid {
-		if ext == v {
-			return true
-		}
-	}
-
-	return false
-}
-
-// AlwaysServe must indicate that file is expected to be served by static service.
-func (c *Config) AlwaysServe(filename string) bool {
-	ext := strings.ToLower(path.Ext(filename))
-
-	for _, v := range c.Static.Always {
-		if ext == v {
-			return true
-		}
-	}
-
-	return false
-}
diff --git a/plugins/static/plugin.go b/plugins/static/plugin.go
index 76cb9e68..b6c25f3d 100644
--- a/plugins/static/plugin.go
+++ b/plugins/static/plugin.go
@@ -1,8 +1,10 @@
 package static
 
 import (
+	"io/fs"
 	"net/http"
 	"path"
+	"strings"
 
 	"github.com/spiral/errors"
 	"github.com/spiral/roadrunner/v2/plugins/config"
@@ -23,6 +25,14 @@ type Plugin struct {
 
 	// root is initiated http directory
 	root http.Dir
+
+	// file extensions which are allowed to be served
+	allowedExtensions map[string]struct{}
+
+	// file extensions which are forbidden to be served
+	forbiddenExtensions map[string]struct{}
+
+	alwaysServe map[string]struct{}
 }
 
 // Init must return configure service and return true if service hasStatus enabled. Must return error in case of
@@ -50,6 +60,33 @@ func (s *Plugin) Init(cfg config.Configurer, log logger.Logger) error {
 		return errors.E(op, err)
 	}
 
+	// create 2 hashmaps with the allowed and forbidden file extensions
+	s.allowedExtensions = make(map[string]struct{}, len(s.cfg.Static.Allow))
+	s.forbiddenExtensions = make(map[string]struct{}, len(s.cfg.Static.Forbid))
+	s.alwaysServe = make(map[string]struct{}, len(s.cfg.Static.Always))
+
+	for i := 0; i < len(s.cfg.Static.Forbid); i++ {
+		s.forbiddenExtensions[s.cfg.Static.Forbid[i]] = struct{}{}
+	}
+
+	for i := 0; i < len(s.cfg.Static.Allow); i++ {
+		s.forbiddenExtensions[s.cfg.Static.Allow[i]] = struct{}{}
+	}
+
+	// check if any forbidden items presented in the allowed
+	// if presented, delete such items from allowed
+	for k := range s.forbiddenExtensions {
+		if _, ok := s.allowedExtensions[k]; ok {
+			delete(s.allowedExtensions, k)
+		}
+	}
+
+	for i := 0; i < len(s.cfg.Static.Always); i++ {
+		s.alwaysServe[s.cfg.Static.Always[i]] = struct{}{}
+	}
+
+	// at this point we have distinct allowed and forbidden hashmaps, also with alwaysServed
+
 	return nil
 }
 
@@ -73,45 +110,77 @@ func (s *Plugin) Middleware(next http.Handler) http.HandlerFunc {
 			}
 		}
 
-		if !s.handleStatic(w, r) {
-			next.ServeHTTP(w, r)
+		fPath := path.Clean(r.URL.Path)
+		ext := strings.ToLower(path.Ext(fPath))
+
+		// check that file is in forbidden list
+		if _, ok := s.forbiddenExtensions[ext]; ok {
+			http.Error(w, "file is forbidden", 404)
+			return
+		}
+
+		f, err := s.root.Open(fPath)
+		if err != nil {
+			// if we should always serve files with some extensions
+			// show error to the user and invoke next middleware
+			if _, ok := s.alwaysServe[ext]; ok {
+				//http.Error(w, err.Error(), 404)
+				w.WriteHeader(404)
+				next.ServeHTTP(w, r)
+				return
+			}
+			// else, return with error
+			http.Error(w, err.Error(), 404)
+			return
 		}
-	}
-}
 
-func (s *Plugin) handleStatic(w http.ResponseWriter, r *http.Request) bool {
-	fPath := path.Clean(r.URL.Path)
+		defer func() {
+			err = f.Close()
+			if err != nil {
+				s.log.Error("file close error", "error", err)
+			}
+		}()
 
-	if s.cfg.AlwaysForbid(fPath) {
-		return false
-	}
+		// here we know, that file extension is not in the AlwaysServe and file exists
+		// (or by some reason, there is no error from the http.Open method)
 
-	f, err := s.root.Open(fPath)
-	if err != nil {
-		if s.cfg.AlwaysServe(fPath) {
-			w.WriteHeader(404)
-			return true
+		// if we have some allowed extensions, we should check them
+		if len(s.allowedExtensions) > 0 {
+			if _, ok := s.allowedExtensions[ext]; ok {
+				d, err := s.check(f)
+				if err != nil {
+					http.Error(w, err.Error(), 404)
+					return
+				}
+
+				http.ServeContent(w, r, d.Name(), d.ModTime(), f)
+			}
+			// otherwise we guess, that all file extensions are allowed
+		} else {
+			d, err := s.check(f)
+			if err != nil {
+				http.Error(w, err.Error(), 404)
+				return
+			}
+
+			http.ServeContent(w, r, d.Name(), d.ModTime(), f)
 		}
 
-		return false
+		next.ServeHTTP(w, r)
 	}
-	defer func() {
-		err = f.Close()
-		if err != nil {
-			s.log.Error("file closing error", "error", err)
-		}
-	}()
+}
 
+func (s *Plugin) check(f http.File) (fs.FileInfo, error) {
+	const op = errors.Op("http_file_check")
 	d, err := f.Stat()
 	if err != nil {
-		return false
+		return nil, err
 	}
 
 	// do not serve directories
 	if d.IsDir() {
-		return false
+		return nil, errors.E(op, errors.Str("directory path provided, should be path to the file"))
 	}
 
-	http.ServeContent(w, r, d.Name(), d.ModTime(), f)
-	return true
+	return d, nil
 }
diff --git a/tests/plugins/static/config_test.go b/tests/plugins/static/config_test.go
deleted file mode 100644
index d73fd845..00000000
--- a/tests/plugins/static/config_test.go
+++ /dev/null
@@ -1,49 +0,0 @@
-package static
-
-import (
-	"testing"
-
-	"github.com/spiral/roadrunner/v2/plugins/static"
-	"github.com/stretchr/testify/assert"
-)
-
-func TestConfig_Forbids(t *testing.T) {
-	cfg := static.Config{Static: &struct {
-		Dir      string
-		Forbid   []string
-		Always   []string
-		Request  map[string]string
-		Response map[string]string
-	}{Dir: "", Forbid: []string{".php"}, Always: nil, Request: nil, Response: nil}}
-
-	assert.True(t, cfg.AlwaysForbid("index.php"))
-	assert.True(t, cfg.AlwaysForbid("index.PHP"))
-	assert.True(t, cfg.AlwaysForbid("phpadmin/index.bak.php"))
-	assert.False(t, cfg.AlwaysForbid("index.html"))
-}
-
-func TestConfig_Valid(t *testing.T) {
-	assert.NoError(t, (&static.Config{Static: &struct {
-		Dir      string
-		Forbid   []string
-		Always   []string
-		Request  map[string]string
-		Response map[string]string
-	}{Dir: "./"}}).Valid())
-
-	assert.Error(t, (&static.Config{Static: &struct {
-		Dir      string
-		Forbid   []string
-		Always   []string
-		Request  map[string]string
-		Response map[string]string
-	}{Dir: "./http.go"}}).Valid())
-
-	assert.Error(t, (&static.Config{Static: &struct {
-		Dir      string
-		Forbid   []string
-		Always   []string
-		Request  map[string]string
-		Response map[string]string
-	}{Dir: "./dir/"}}).Valid())
-}
diff --git a/tests/plugins/static/configs/.rr-http-static-files.yaml b/tests/plugins/static/configs/.rr-http-static-files.yaml
index d6b3032e..0e003dae 100644
--- a/tests/plugins/static/configs/.rr-http-static-files.yaml
+++ b/tests/plugins/static/configs/.rr-http-static-files.yaml
@@ -18,11 +18,7 @@ http:
     dir: "../../../tests"
     forbid: [ ".php", ".htaccess" ]
     always: [ ".ico" ]
-    request:
-      "Example-Request-Header": "Value"
-    # Automatically add headers to every response.
-    response:
-      "X-Powered-By": "RoadRunner"
+
   pool:
     num_workers: 2
     max_jobs: 0
@@ -30,4 +26,4 @@ http:
     destroy_timeout: 60s
 logs:
   mode: development
-  level: error
\ No newline at end of file
+  level: error
diff --git a/tests/plugins/static/static_plugin_test.go b/tests/plugins/static/static_plugin_test.go
index 38562537..b58f1f6b 100644
--- a/tests/plugins/static/static_plugin_test.go
+++ b/tests/plugins/static/static_plugin_test.go
@@ -259,7 +259,8 @@ func TestStaticFilesForbid(t *testing.T) {
 
 	err = cont.RegisterAll(
 		cfg,
-		mockLogger,
+		//mockLogger,
+		&logger.ZapLogger{},
 		&server.Plugin{},
 		&httpPlugin.Plugin{},
 		&gzip.Plugin{},