From 700f32718e6a52b308ab832f695007b6f39b0fd2 Mon Sep 17 00:00:00 2001 From: Oleg Lobanov Date: Mon, 1 Jun 2020 01:12:36 +0200 Subject: [PATCH] refactor: add more go linters (#970) --- .circleci/config.yml | 4 +- .golangci.yml | 132 ++++++++++++++++++++++++++++++ auth/json.go | 7 +- auth/storage.go | 4 +- cmd/cmds_add.go | 2 +- cmd/cmds_rm.go | 4 +- cmd/config.go | 11 ++- cmd/config_import.go | 5 +- cmd/config_init.go | 3 +- cmd/docs.go | 12 +-- cmd/hash.go | 3 +- cmd/root.go | 28 ++++--- cmd/rule_rm.go | 7 +- cmd/rules.go | 23 +++--- cmd/rules_add.go | 3 +- cmd/upgrade.go | 5 +- cmd/users.go | 44 +++++----- cmd/users_add.go | 11 +-- cmd/users_find.go | 3 +- cmd/users_import.go | 12 +-- cmd/users_update.go | 3 +- cmd/utils.go | 13 +-- cmd/version.go | 3 +- files/file.go | 29 ++++--- files/listing.go | 2 + files/utils.go | 38 +++++---- fileutils/dir.go | 2 +- fileutils/file.go | 2 +- http/auth.go | 13 ++- http/commands.go | 28 ++++--- http/data.go | 6 +- http/http.go | 9 +- http/public.go | 2 +- http/raw.go | 19 +++-- http/resource.go | 4 +- http/share.go | 4 +- http/static.go | 25 +++--- http/users.go | 5 +- http/utils.go | 2 +- runner/parser.go | 7 +- runner/runner.go | 4 +- search/search.go | 3 +- settings/dir.go | 8 +- share/storage.go | 8 +- storage/bolt/auth.go | 1 + storage/bolt/bolt.go | 17 ++-- storage/bolt/config.go | 9 +- storage/bolt/importer/conf.go | 10 +-- storage/bolt/importer/importer.go | 17 ++-- storage/bolt/importer/users.go | 9 +- storage/bolt/share.go | 7 +- storage/bolt/users.go | 9 +- storage/bolt/utils.go | 1 + storage/storage.go | 2 +- users/storage.go | 8 +- users/users.go | 5 +- 56 files changed, 436 insertions(+), 221 deletions(-) create mode 100644 .golangci.yml diff --git a/.circleci/config.yml b/.circleci/config.yml index 0ab73806..32ecb8af 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -2,10 +2,10 @@ version: 2 jobs: lint: docker: - - image: golangci/golangci-lint:v1.16 + - image: golangci/golangci-lint:v1.27.0 steps: - checkout - - run: golangci-lint run -v -D errcheck + - run: golangci-lint run -v build-node: docker: - image: circleci/node diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..d94a3448 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,132 @@ +linters-settings: + dupl: + threshold: 100 + exhaustive: + default-signifies-exhaustive: false + funlen: + lines: 100 + statements: 50 + goconst: + min-len: 2 + min-occurrences: 2 + gocritic: + enabled-tags: + - diagnostic + - experimental + - opinionated + - performance + - style + disabled-checks: + - dupImport # https://github.com/go-critic/go-critic/issues/845 + - ifElseChain + - octalLiteral + - whyNoLint + - wrapperFunc + gocyclo: + min-complexity: 15 + goimports: + local-prefixes: github.com/filebrowser/filebrowser + golint: + min-confidence: 0 + gomnd: + settings: + mnd: + # don't include the "operation" and "assign" + checks: argument,case,condition,return + govet: + check-shadowing: true + lll: + line-length: 140 + maligned: + suggest-new: true + misspell: + locale: US + nolintlint: + allow-leading-space: true # don't require machine-readable nolint directives (i.e. with no leading space) + allow-unused: false # report any unused nolint directives + require-explanation: false # don't require an explanation for nolint directives + require-specific: false # don't require nolint directives to be specific about which linter is being skipped + +linters: + # please, do not use `enable-all`: it's deprecated and will be removed soon. + # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint + disable-all: true + enable: + - bodyclose + - deadcode + - depguard + - dogsled + - dupl + - errcheck + - funlen + - gochecknoinits + - goconst + - gocritic + - gocyclo + - gofmt + - goimports + - golint + - gomnd + - goprintffuncname + - gosec + - gosimple + - govet + - ineffassign + - interfacer + - lll + - misspell + - nakedret + - nolintlint + - rowserrcheck + - scopelint + - staticcheck + - structcheck + - stylecheck + - typecheck + - unconvert + - unparam + - unused + - varcheck + - whitespace + - prealloc + + # don't enable: + # - asciicheck + # - exhaustive (TODO: enable after next release; current release at time of writing is v1.27) + # - gochecknoglobals + # - gocognit + # - godot + # - godox + # - goerr113 + # - maligned + # - nestif + # - testpackage + # - wsl + +issues: + exclude-rules: + - path: cmd/.*.go + linters: + - gochecknoinits + - path: .*_test.go + linters: + - lll + - gochecknoinits + - gocyclo + - funlen + - dupl + - scopelint + - text: "Auther" + linters: + - misspell + +run: + skip-dirs: + - frontend/ + skip-files: + - http/rice-box.go + +# golangci.com configuration +# https://github.com/golangci/golangci/wiki/Configuration +service: + golangci-lint-version: 1.27.x # use the fixed version to not introduce new linters unexpectedly \ No newline at end of file diff --git a/auth/json.go b/auth/json.go index 3f551486..641b73cd 100644 --- a/auth/json.go +++ b/auth/json.go @@ -20,7 +20,7 @@ type jsonCred struct { ReCaptcha string `json:"recaptcha"` } -// JSONAuth is a json implementaion of an Auther. +// JSONAuth is a json implementation of an Auther. type JSONAuth struct { ReCaptcha *ReCaptcha `json:"recaptcha" yaml:"recaptcha"` } @@ -40,7 +40,7 @@ func (a JSONAuth) Auth(r *http.Request, sto *users.Storage, root string) (*users // If ReCaptcha is enabled, check the code. if a.ReCaptcha != nil && len(a.ReCaptcha.Secret) > 0 { - ok, err := a.ReCaptcha.Ok(cred.ReCaptcha) + ok, err := a.ReCaptcha.Ok(cred.ReCaptcha) //nolint:shadow if err != nil { return nil, err @@ -66,7 +66,7 @@ func (a JSONAuth) LoginPage() bool { const reCaptchaAPI = "/recaptcha/api/siteverify" -// ReCaptcha identifies a recaptcha conenction. +// ReCaptcha identifies a recaptcha connection. type ReCaptcha struct { Host string `json:"host"` Key string `json:"key"` @@ -89,6 +89,7 @@ func (r *ReCaptcha) Ok(response string) (bool, error) { if err != nil { return false, err } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { return false, nil diff --git a/auth/storage.go b/auth/storage.go index 2cf63e05..c0723ba5 100644 --- a/auth/storage.go +++ b/auth/storage.go @@ -18,8 +18,8 @@ type Storage struct { } // NewStorage creates a auth storage from a backend. -func NewStorage(back StorageBackend, users *users.Storage) *Storage { - return &Storage{back: back, users: users} +func NewStorage(back StorageBackend, userStore *users.Storage) *Storage { + return &Storage{back: back, users: userStore} } // Get wraps a StorageBackend.Get. diff --git a/cmd/cmds_add.go b/cmd/cmds_add.go index 3fcd7141..5706817b 100644 --- a/cmd/cmds_add.go +++ b/cmd/cmds_add.go @@ -14,7 +14,7 @@ var cmdsAddCmd = &cobra.Command{ Use: "add ", Short: "Add a command to run on a specific event", Long: `Add a command to run on a specific event.`, - Args: cobra.MinimumNArgs(2), + Args: cobra.MinimumNArgs(2), //nolint:mnd Run: python(func(cmd *cobra.Command, args []string, d pythonData) { s, err := d.store.Settings.Get() checkErr(err) diff --git a/cmd/cmds_rm.go b/cmd/cmds_rm.go index 2a863213..dc0c06f5 100644 --- a/cmd/cmds_rm.go +++ b/cmd/cmds_rm.go @@ -23,7 +23,7 @@ You can also specify an optional parameter (index_end) so you can remove all commands from 'index' to 'index_end', including 'index_end'.`, Args: func(cmd *cobra.Command, args []string) error { - if err := cobra.RangeArgs(2, 3)(cmd, args); err != nil { + if err := cobra.RangeArgs(2, 3)(cmd, args); err != nil { //nolint:mnd return err } @@ -43,7 +43,7 @@ including 'index_end'.`, i, err := strconv.Atoi(args[1]) checkErr(err) f := i - if len(args) == 3 { + if len(args) == 3 { //nolint:mnd f, err = strconv.Atoi(args[2]) checkErr(err) } diff --git a/cmd/config.go b/cmd/config.go index 278521ed..8b69e7b7 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -8,11 +8,12 @@ import ( "strings" "text/tabwriter" + "github.com/spf13/cobra" + "github.com/spf13/pflag" + "github.com/filebrowser/filebrowser/v2/auth" "github.com/filebrowser/filebrowser/v2/errors" "github.com/filebrowser/filebrowser/v2/settings" - "github.com/spf13/cobra" - "github.com/spf13/pflag" ) func init() { @@ -44,6 +45,7 @@ func addConfigFlags(flags *pflag.FlagSet) { flags.Bool("branding.disableExternal", false, "disable external links such as GitHub links") } +//nolint:gocyclo func getAuthentication(flags *pflag.FlagSet, defaults ...interface{}) (settings.AuthMethod, auth.Auther) { method := settings.AuthMethod(mustGetString(flags, "auth.method")) @@ -53,11 +55,12 @@ func getAuthentication(flags *pflag.FlagSet, defaults ...interface{}) (settings. for _, arg := range defaults { switch def := arg.(type) { case *settings.Settings: - method = settings.AuthMethod(def.AuthMethod) + method = def.AuthMethod case auth.Auther: ms, err := json.Marshal(def) checkErr(err) - json.Unmarshal(ms, &defaultAuther) + err = json.Unmarshal(ms, &defaultAuther) + checkErr(err) } } } diff --git a/cmd/config_import.go b/cmd/config_import.go index 9fef477e..7871a9f8 100644 --- a/cmd/config_import.go +++ b/cmd/config_import.go @@ -6,9 +6,10 @@ import ( "path/filepath" "reflect" + "github.com/spf13/cobra" + "github.com/filebrowser/filebrowser/v2/auth" "github.com/filebrowser/filebrowser/v2/settings" - "github.com/spf13/cobra" ) func init() { @@ -55,7 +56,7 @@ The path must be for a json or yaml file.`, checkErr(err) var rawAuther interface{} - if filepath.Ext(args[0]) != ".json" { + if filepath.Ext(args[0]) != ".json" { //nolint:goconst rawAuther = cleanUpInterfaceMap(file.Auther.(map[interface{}]interface{})) } else { rawAuther = file.Auther diff --git a/cmd/config_init.go b/cmd/config_init.go index 48ec3873..86a69914 100644 --- a/cmd/config_init.go +++ b/cmd/config_init.go @@ -4,8 +4,9 @@ import ( "fmt" "strings" - "github.com/filebrowser/filebrowser/v2/settings" "github.com/spf13/cobra" + + "github.com/filebrowser/filebrowser/v2/settings" ) func init() { diff --git a/cmd/docs.go b/cmd/docs.go index 34aee2b3..9ebb9573 100644 --- a/cmd/docs.go +++ b/cmd/docs.go @@ -88,7 +88,7 @@ func generateMarkdown(cmd *cobra.Command, w io.Writer) { short := cmd.Short long := cmd.Long - if len(long) == 0 { + if long == "" { long = short } @@ -106,21 +106,21 @@ func generateMarkdown(cmd *cobra.Command, w io.Writer) { buf.WriteString(fmt.Sprintf("```\n%s\n```\n\n", cmd.Example)) } - printOptions(buf, cmd, name) + printOptions(buf, cmd) _, err := buf.WriteTo(w) checkErr(err) } func generateFlagsTable(fs *pflag.FlagSet, buf io.StringWriter) { - buf.WriteString("| Name | Shorthand | Usage |\n") - buf.WriteString("|------|-----------|-------|\n") + _, _ = buf.WriteString("| Name | Shorthand | Usage |\n") + _, _ = buf.WriteString("|------|-----------|-------|\n") fs.VisitAll(func(f *pflag.Flag) { - buf.WriteString("|" + f.Name + "|" + f.Shorthand + "|" + f.Usage + "|\n") + _, _ = buf.WriteString("|" + f.Name + "|" + f.Shorthand + "|" + f.Usage + "|\n") }) } -func printOptions(buf *bytes.Buffer, cmd *cobra.Command, name string) { +func printOptions(buf *bytes.Buffer, cmd *cobra.Command) { flags := cmd.NonInheritedFlags() flags.SetOutput(buf) if flags.HasAvailableFlags() { diff --git a/cmd/hash.go b/cmd/hash.go index e92203e4..1d472a3c 100644 --- a/cmd/hash.go +++ b/cmd/hash.go @@ -3,8 +3,9 @@ package cmd import ( "fmt" - "github.com/filebrowser/filebrowser/v2/users" "github.com/spf13/cobra" + + "github.com/filebrowser/filebrowser/v2/users" ) func init() { diff --git a/cmd/root.go b/cmd/root.go index a3fa4cec..8f23e95c 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -13,16 +13,17 @@ import ( "strings" "syscall" - "github.com/filebrowser/filebrowser/v2/auth" - fbhttp "github.com/filebrowser/filebrowser/v2/http" - "github.com/filebrowser/filebrowser/v2/settings" - "github.com/filebrowser/filebrowser/v2/storage" - "github.com/filebrowser/filebrowser/v2/users" homedir "github.com/mitchellh/go-homedir" "github.com/spf13/cobra" "github.com/spf13/pflag" v "github.com/spf13/viper" lumberjack "gopkg.in/natefinch/lumberjack.v2" + + "github.com/filebrowser/filebrowser/v2/auth" + fbhttp "github.com/filebrowser/filebrowser/v2/http" + "github.com/filebrowser/filebrowser/v2/settings" + "github.com/filebrowser/filebrowser/v2/storage" + "github.com/filebrowser/filebrowser/v2/users" ) var ( @@ -113,16 +114,17 @@ user created with the credentials from options "username" and "password".`, var listener net.Listener - if server.Socket != "" { + switch { + case server.Socket != "": listener, err = net.Listen("unix", server.Socket) checkErr(err) - } else if server.TLSKey != "" && server.TLSCert != "" { - cer, err := tls.LoadX509KeyPair(server.TLSCert, server.TLSKey) + case server.TLSKey != "" && server.TLSCert != "": + cer, err := tls.LoadX509KeyPair(server.TLSCert, server.TLSKey) //nolint:shadow checkErr(err) - listener, err = tls.Listen("tcp", adr, &tls.Config{Certificates: []tls.Certificate{cer}}) + listener, err = tls.Listen("tcp", adr, &tls.Config{Certificates: []tls.Certificate{cer}}) //nolint:shadow checkErr(err) - } else { - listener, err = net.Listen("tcp", adr) + default: + listener, err = net.Listen("tcp", adr) //nolint:shadow checkErr(err) } @@ -142,13 +144,14 @@ user created with the credentials from options "username" and "password".`, }, pythonConfig{allowNoDB: true}), } -func cleanupHandler(listener net.Listener, c chan os.Signal) { +func cleanupHandler(listener net.Listener, c chan os.Signal) { //nolint:interfacer sig := <-c log.Printf("Caught signal %s: shutting down.", sig) listener.Close() os.Exit(0) } +//nolint:gocyclo func getRunParams(flags *pflag.FlagSet, st *storage.Storage) *settings.Server { server, err := st.Settings.GetServer() checkErr(err) @@ -348,5 +351,4 @@ func initConfig() { } else { cfgFile = "Using config file: " + v.ConfigFileUsed() } - } diff --git a/cmd/rule_rm.go b/cmd/rule_rm.go index 862eff51..60b98c93 100644 --- a/cmd/rule_rm.go +++ b/cmd/rule_rm.go @@ -3,15 +3,16 @@ package cmd import ( "strconv" + "github.com/spf13/cobra" + "github.com/filebrowser/filebrowser/v2/settings" "github.com/filebrowser/filebrowser/v2/users" - "github.com/spf13/cobra" ) func init() { rulesCmd.AddCommand(rulesRmCommand) rulesRmCommand.Flags().Uint("index", 0, "index of rule to remove") - rulesRmCommand.MarkFlagRequired("index") + _ = rulesRmCommand.MarkFlagRequired("index") } var rulesRmCommand = &cobra.Command{ @@ -43,7 +44,7 @@ including 'index_end'.`, i, err := strconv.Atoi(args[0]) checkErr(err) f := i - if len(args) == 2 { + if len(args) == 2 { //nolint:mnd f, err = strconv.Atoi(args[1]) checkErr(err) } diff --git a/cmd/rules.go b/cmd/rules.go index b7239563..3bf91dd1 100644 --- a/cmd/rules.go +++ b/cmd/rules.go @@ -3,12 +3,13 @@ package cmd import ( "fmt" + "github.com/spf13/cobra" + "github.com/spf13/pflag" + "github.com/filebrowser/filebrowser/v2/rules" "github.com/filebrowser/filebrowser/v2/settings" "github.com/filebrowser/filebrowser/v2/storage" "github.com/filebrowser/filebrowser/v2/users" - "github.com/spf13/cobra" - "github.com/spf13/pflag" ) func init() { @@ -18,8 +19,8 @@ func init() { } var rulesCmd = &cobra.Command{ - Use: "rules", - Short: "Rules management utility", + Use: "rules", + Short: "Rules management utility", Long: `On each subcommand you'll have available at least two flags: "username" and "id". You must either set only one of them or none. If you set one of them, the command will apply to @@ -28,14 +29,14 @@ rules.`, Args: cobra.NoArgs, } -func runRules(st *storage.Storage, cmd *cobra.Command, users func(*users.User), global func(*settings.Settings)) { +func runRules(st *storage.Storage, cmd *cobra.Command, usersFn func(*users.User), globalFn func(*settings.Settings)) { id := getUserIdentifier(cmd.Flags()) if id != nil { user, err := st.Users.Get("", id) checkErr(err) - if users != nil { - users(user) + if usersFn != nil { + usersFn(user) } printRules(user.Rules, id) @@ -45,8 +46,8 @@ func runRules(st *storage.Storage, cmd *cobra.Command, users func(*users.User), s, err := st.Settings.Get() checkErr(err) - if global != nil { - global(s) + if globalFn != nil { + globalFn(s) } printRules(s.Rules, id) @@ -65,14 +66,14 @@ func getUserIdentifier(flags *pflag.FlagSet) interface{} { return nil } -func printRules(rules []rules.Rule, id interface{}) { +func printRules(rulez []rules.Rule, id interface{}) { if id == nil { fmt.Printf("Global Rules:\n\n") } else { fmt.Printf("Rules for user %v:\n\n", id) } - for id, rule := range rules { + for id, rule := range rulez { fmt.Printf("(%d) ", id) if rule.Regex { if rule.Allow { diff --git a/cmd/rules_add.go b/cmd/rules_add.go index bfed8f15..fcdc7fb4 100644 --- a/cmd/rules_add.go +++ b/cmd/rules_add.go @@ -3,10 +3,11 @@ package cmd import ( "regexp" + "github.com/spf13/cobra" + "github.com/filebrowser/filebrowser/v2/rules" "github.com/filebrowser/filebrowser/v2/settings" "github.com/filebrowser/filebrowser/v2/users" - "github.com/spf13/cobra" ) func init() { diff --git a/cmd/upgrade.go b/cmd/upgrade.go index 18f39ca1..9f747c9c 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -1,8 +1,9 @@ package cmd import ( - "github.com/filebrowser/filebrowser/v2/storage/bolt/importer" "github.com/spf13/cobra" + + "github.com/filebrowser/filebrowser/v2/storage/bolt/importer" ) func init() { @@ -10,7 +11,7 @@ func init() { upgradeCmd.Flags().String("old.database", "", "") upgradeCmd.Flags().String("old.config", "", "") - upgradeCmd.MarkFlagRequired("old.database") + _ = upgradeCmd.MarkFlagRequired("old.database") } var upgradeCmd = &cobra.Command{ diff --git a/cmd/users.go b/cmd/users.go index 74c1c185..9ef0da18 100644 --- a/cmd/users.go +++ b/cmd/users.go @@ -7,10 +7,11 @@ import ( "strconv" "text/tabwriter" - "github.com/filebrowser/filebrowser/v2/settings" - "github.com/filebrowser/filebrowser/v2/users" "github.com/spf13/cobra" "github.com/spf13/pflag" + + "github.com/filebrowser/filebrowser/v2/settings" + "github.com/filebrowser/filebrowser/v2/users" ) func init() { @@ -24,38 +25,38 @@ var usersCmd = &cobra.Command{ Args: cobra.NoArgs, } -func printUsers(users []*users.User) { +func printUsers(usrs []*users.User) { w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) fmt.Fprintln(w, "ID\tUsername\tScope\tLocale\tV. Mode\tAdmin\tExecute\tCreate\tRename\tModify\tDelete\tShare\tDownload\tPwd Lock") - for _, user := range users { + for _, u := range usrs { fmt.Fprintf(w, "%d\t%s\t%s\t%s\t%s\t%t\t%t\t%t\t%t\t%t\t%t\t%t\t%t\t%t\t\n", - user.ID, - user.Username, - user.Scope, - user.Locale, - user.ViewMode, - user.Perm.Admin, - user.Perm.Execute, - user.Perm.Create, - user.Perm.Rename, - user.Perm.Modify, - user.Perm.Delete, - user.Perm.Share, - user.Perm.Download, - user.LockPassword, + u.ID, + u.Username, + u.Scope, + u.Locale, + u.ViewMode, + u.Perm.Admin, + u.Perm.Execute, + u.Perm.Create, + u.Perm.Rename, + u.Perm.Modify, + u.Perm.Delete, + u.Perm.Share, + u.Perm.Download, + u.LockPassword, ) } w.Flush() } -func parseUsernameOrID(arg string) (string, uint) { - id, err := strconv.ParseUint(arg, 10, 0) +func parseUsernameOrID(arg string) (username string, id uint) { + id64, err := strconv.ParseUint(arg, 10, 0) if err != nil { return arg, 0 } - return "", uint(id) + return "", uint(id64) } func addUserFlags(flags *pflag.FlagSet) { @@ -84,6 +85,7 @@ func getViewMode(flags *pflag.FlagSet) users.ViewMode { return viewMode } +//nolint:gocyclo func getUserDefaults(flags *pflag.FlagSet, defaults *settings.UserDefaults, all bool) { visit := func(flag *pflag.Flag) { switch flag.Name { diff --git a/cmd/users_add.go b/cmd/users_add.go index b5ccd252..d3b8f825 100644 --- a/cmd/users_add.go +++ b/cmd/users_add.go @@ -1,8 +1,9 @@ package cmd import ( - "github.com/filebrowser/filebrowser/v2/users" "github.com/spf13/cobra" + + "github.com/filebrowser/filebrowser/v2/users" ) func init() { @@ -14,7 +15,7 @@ var usersAddCmd = &cobra.Command{ Use: "add ", Short: "Create a new user", Long: `Create a new user and add it to the database.`, - Args: cobra.ExactArgs(2), + Args: cobra.ExactArgs(2), //nolint:mnd Run: python(func(cmd *cobra.Command, args []string, d pythonData) { s, err := d.store.Settings.Get() checkErr(err) @@ -33,9 +34,9 @@ var usersAddCmd = &cobra.Command{ servSettings, err := d.store.Settings.GetServer() checkErr(err) - //since getUserDefaults() polluted s.Defaults.Scope - //which makes the Scope not the one saved in the db - //we need the right s.Defaults.Scope here + // since getUserDefaults() polluted s.Defaults.Scope + // which makes the Scope not the one saved in the db + // we need the right s.Defaults.Scope here s2, err := d.store.Settings.Get() checkErr(err) diff --git a/cmd/users_find.go b/cmd/users_find.go index 2199aba9..b1b03bc1 100644 --- a/cmd/users_find.go +++ b/cmd/users_find.go @@ -1,8 +1,9 @@ package cmd import ( - "github.com/filebrowser/filebrowser/v2/users" "github.com/spf13/cobra" + + "github.com/filebrowser/filebrowser/v2/users" ) func init() { diff --git a/cmd/users_import.go b/cmd/users_import.go index ea84beab..236e9cea 100644 --- a/cmd/users_import.go +++ b/cmd/users_import.go @@ -2,11 +2,13 @@ package cmd import ( "errors" + "fmt" "os" "strconv" - "github.com/filebrowser/filebrowser/v2/users" "github.com/spf13/cobra" + + "github.com/filebrowser/filebrowser/v2/users" ) func init() { @@ -65,8 +67,7 @@ list or set it to 0.`, // with the new username. If there is, print an error and cancel the // operation if user.Username != onDB.Username { - conflictuous, err := d.store.Users.Get("", user.Username) - if err == nil { + if conflictuous, err := d.store.Users.Get("", user.Username); err == nil { //nolint:shadow checkErr(usernameConflictError(user.Username, conflictuous.ID, user.ID)) } } @@ -82,6 +83,7 @@ list or set it to 0.`, }, pythonConfig{}), } -func usernameConflictError(username string, original, new uint) error { - return errors.New("can't import user with ID " + strconv.Itoa(int(new)) + " and username \"" + username + "\" because the username is already registred with the user " + strconv.Itoa(int(original))) +func usernameConflictError(username string, originalID, newID uint) error { + return fmt.Errorf(`can't import user with ID %d and username "%s" because the username is already registred with the user %d`, + newID, username, originalID) } diff --git a/cmd/users_update.go b/cmd/users_update.go index cf7cd2ac..040dabd5 100644 --- a/cmd/users_update.go +++ b/cmd/users_update.go @@ -1,9 +1,10 @@ package cmd import ( + "github.com/spf13/cobra" + "github.com/filebrowser/filebrowser/v2/settings" "github.com/filebrowser/filebrowser/v2/users" - "github.com/spf13/cobra" ) func init() { diff --git a/cmd/utils.go b/cmd/utils.go index 72f0177c..bd5ec2f3 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -9,12 +9,13 @@ import ( "path/filepath" "github.com/asdine/storm" - "github.com/filebrowser/filebrowser/v2/settings" - "github.com/filebrowser/filebrowser/v2/storage" - "github.com/filebrowser/filebrowser/v2/storage/bolt" "github.com/spf13/cobra" "github.com/spf13/pflag" yaml "gopkg.in/yaml.v2" + + "github.com/filebrowser/filebrowser/v2/settings" + "github.com/filebrowser/filebrowser/v2/storage" + "github.com/filebrowser/filebrowser/v2/storage/bolt" ) func checkErr(err error) { @@ -70,7 +71,9 @@ func dbExists(path string) (bool, error) { d := filepath.Dir(path) _, err = os.Stat(d) if os.IsNotExist(err) { - os.MkdirAll(d, 0700) + if err := os.MkdirAll(d, 0700); err != nil { //nolint:shadow + return false, err + } return false, nil } } @@ -113,7 +116,7 @@ func marshal(filename string, data interface{}) error { encoder := json.NewEncoder(fd) encoder.SetIndent("", " ") return encoder.Encode(data) - case ".yml", ".yaml": + case ".yml", ".yaml": //nolint:goconst encoder := yaml.NewEncoder(fd) return encoder.Encode(data) default: diff --git a/cmd/version.go b/cmd/version.go index cec5a415..5877505e 100644 --- a/cmd/version.go +++ b/cmd/version.go @@ -3,8 +3,9 @@ package cmd import ( "fmt" - "github.com/filebrowser/filebrowser/v2/version" "github.com/spf13/cobra" + + "github.com/filebrowser/filebrowser/v2/version" ) func init() { diff --git a/files/file.go b/files/file.go index 9e6671e9..5abf0792 100644 --- a/files/file.go +++ b/files/file.go @@ -1,8 +1,8 @@ package files import ( - "crypto/md5" - "crypto/sha1" + "crypto/md5" //nolint:gosec + "crypto/sha1" //nolint:gosec "crypto/sha256" "crypto/sha512" "encoding/hex" @@ -17,9 +17,10 @@ import ( "strings" "time" + "github.com/spf13/afero" + "github.com/filebrowser/filebrowser/v2/errors" "github.com/filebrowser/filebrowser/v2/rules" - "github.com/spf13/afero" ) // FileInfo describes a file. @@ -74,7 +75,10 @@ func NewFileInfo(opts FileOptions) (*FileInfo, error) { if opts.Expand { if file.IsDir { - return file, file.readListing(opts.Checker) + if err := file.readListing(opts.Checker); err != nil { //nolint:shadow + return nil, err + } + return file, nil } err = file.detectType(opts.Modify, true) @@ -105,6 +109,7 @@ func (i *FileInfo) Checksum(algo string) error { var h hash.Hash + //nolint:gosec switch algo { case "md5": h = md5.New() @@ -127,6 +132,8 @@ func (i *FileInfo) Checksum(algo string) error { return nil } +//nolint:goconst +//TODO: use constants func (i *FileInfo) detectType(modify, saveContent bool) error { // failing to detect the type should not return error. // imagine the situation where a file in a dir with thousands @@ -198,9 +205,9 @@ func (i *FileInfo) detectSubtitles() { // TODO: detect multiple languages. Base.Lang.vtt - path := strings.TrimSuffix(i.Path, ext) + ".vtt" - if _, err := i.Fs.Stat(path); err == nil { - i.Subtitles = append(i.Subtitles, path) + fPath := strings.TrimSuffix(i.Path, ext) + ".vtt" + if _, err := i.Fs.Stat(fPath); err == nil { + i.Subtitles = append(i.Subtitles, fPath) } } @@ -219,16 +226,16 @@ func (i *FileInfo) readListing(checker rules.Checker) error { for _, f := range dir { name := f.Name() - path := path.Join(i.Path, name) + fPath := path.Join(i.Path, name) - if !checker.Check(path) { + if !checker.Check(fPath) { continue } if strings.HasPrefix(f.Mode().String(), "L") { // It's a symbolic link. We try to follow it. If it doesn't work, // we stay with the link information instead if the target's. - info, err := i.Fs.Stat(path) + info, err := i.Fs.Stat(fPath) if err == nil { f = info } @@ -242,7 +249,7 @@ func (i *FileInfo) readListing(checker rules.Checker) error { Mode: f.Mode(), IsDir: f.IsDir(), Extension: filepath.Ext(name), - Path: path, + Path: fPath, } if file.IsDir { diff --git a/files/listing.go b/files/listing.go index ad60e51e..448d7e9c 100644 --- a/files/listing.go +++ b/files/listing.go @@ -16,8 +16,10 @@ type Listing struct { } // ApplySort applies the sort order using .Order and .Sort +//nolint:goconst func (l Listing) ApplySort() { // Check '.Order' to know how to sort + // TODO: use enum if !l.Sorting.Asc { switch l.Sorting.By { case "name": diff --git a/files/utils.go b/files/utils.go index 98a62500..519a5326 100644 --- a/files/utils.go +++ b/files/utils.go @@ -4,41 +4,45 @@ import ( "unicode/utf8" ) -func isBinary(content []byte, n int) bool { +func isBinary(content []byte, _ int) bool { maybeStr := string(content) runeCnt := utf8.RuneCount(content) runeIndex := 0 gotRuneErrCnt := 0 firstRuneErrIndex := -1 - for _, b := range maybeStr { + const ( // 8 and below are control chars (e.g. backspace, null, eof, etc) - if b <= 8 { + maxControlCharsCode = 8 + // 0xFFFD(65533) is the "error" Rune or "Unicode replacement character" + // see https://golang.org/pkg/unicode/utf8/#pkg-constants + unicodeReplacementChar = 0xFFFD + ) + + for _, b := range maybeStr { + if b <= maxControlCharsCode { return true } - // 0xFFFD(65533) is the "error" Rune or "Unicode replacement character" - // see https://golang.org/pkg/unicode/utf8/#pkg-constants - if b == 0xFFFD { - //if it is not the last (utf8.UTFMax - x) rune + if b == unicodeReplacementChar { + // if it is not the last (utf8.UTFMax - x) rune if runeCnt > utf8.UTFMax && runeIndex < runeCnt-utf8.UTFMax { return true - } else { - //else it is the last (utf8.UTFMax - x) rune - //there maybe Vxxx, VVxx, VVVx, thus, we may got max 3 0xFFFD rune (asume V is the byte we got) - //for Chinese, it can only be Vxx, VVx, we may got max 2 0xFFFD rune - gotRuneErrCnt++ + } + // else it is the last (utf8.UTFMax - x) rune + // there maybe Vxxx, VVxx, VVVx, thus, we may got max 3 0xFFFD rune (assume V is the byte we got) + // for Chinese, it can only be Vxx, VVx, we may got max 2 0xFFFD rune + gotRuneErrCnt++ - //mark the first time - if firstRuneErrIndex == -1 { - firstRuneErrIndex = runeIndex - } + // mark the first time + if firstRuneErrIndex == -1 { + firstRuneErrIndex = runeIndex } } runeIndex++ } - //if last (utf8.UTFMax - x ) rune has the "error" Rune, but not all + // if last (utf8.UTFMax - x ) rune has the "error" Rune, but not all if firstRuneErrIndex != -1 && gotRuneErrCnt != runeCnt-firstRuneErrIndex { return true } diff --git a/fileutils/dir.go b/fileutils/dir.go index 56d99645..07a3528e 100644 --- a/fileutils/dir.go +++ b/fileutils/dir.go @@ -9,7 +9,7 @@ import ( // CopyDir copies a directory from source to dest and all // of its sub-directories. It doesn't stop if it finds an error // during the copy. Returns an error if any. -func CopyDir(fs afero.Fs, source string, dest string) error { +func CopyDir(fs afero.Fs, source, dest string) error { // Get properties of source. srcinfo, err := fs.Stat(source) if err != nil { diff --git a/fileutils/file.go b/fileutils/file.go index 64919bbc..1b1e6403 100644 --- a/fileutils/file.go +++ b/fileutils/file.go @@ -9,7 +9,7 @@ import ( // CopyFile copies a file from source to dest and returns // an error if any. -func CopyFile(fs afero.Fs, source string, dest string) error { +func CopyFile(fs afero.Fs, source, dest string) error { // Open the source file. src, err := fs.Open(source) if err != nil { diff --git a/http/auth.go b/http/auth.go index 2a56d3b1..41f0551e 100644 --- a/http/auth.go +++ b/http/auth.go @@ -10,10 +10,15 @@ import ( jwt "github.com/dgrijalva/jwt-go" "github.com/dgrijalva/jwt-go/request" + "github.com/filebrowser/filebrowser/v2/errors" "github.com/filebrowser/filebrowser/v2/users" ) +const ( + TokenExpirationTime = time.Hour * 2 +) + type userInfo struct { ID uint `json:"id"` Locale string `json:"locale"` @@ -161,7 +166,7 @@ var renewHandler = withUser(func(w http.ResponseWriter, r *http.Request, d *data return printToken(w, r, d, d.user) }) -func printToken(w http.ResponseWriter, r *http.Request, d *data, user *users.User) (int, error) { +func printToken(w http.ResponseWriter, _ *http.Request, d *data, user *users.User) (int, error) { claims := &authToken{ User: userInfo{ ID: user.ID, @@ -173,7 +178,7 @@ func printToken(w http.ResponseWriter, r *http.Request, d *data, user *users.Use }, StandardClaims: jwt.StandardClaims{ IssuedAt: time.Now().Unix(), - ExpiresAt: time.Now().Add(time.Hour * 2).Unix(), + ExpiresAt: time.Now().Add(TokenExpirationTime).Unix(), Issuer: "File Browser", }, } @@ -185,6 +190,8 @@ func printToken(w http.ResponseWriter, r *http.Request, d *data, user *users.Use } w.Header().Set("Content-Type", "cty") - w.Write([]byte(signed)) + if _, err := w.Write([]byte(signed)); err != nil { + return http.StatusInternalServerError, err + } return 0, nil } diff --git a/http/commands.go b/http/commands.go index f6371b7d..cdb5ea1e 100644 --- a/http/commands.go +++ b/http/commands.go @@ -9,8 +9,13 @@ import ( "strings" "time" - "github.com/filebrowser/filebrowser/v2/runner" "github.com/gorilla/websocket" + + "github.com/filebrowser/filebrowser/v2/runner" +) + +const ( + WSWriteDeadline = 10 * time.Second ) var upgrader = websocket.Upgrader{ @@ -22,12 +27,14 @@ var ( cmdNotAllowed = []byte("Command not allowed.") ) -func wsErr(ws *websocket.Conn, r *http.Request, status int, err error) { +func wsErr(ws *websocket.Conn, r *http.Request, status int, err error) { //nolint:unparam txt := http.StatusText(status) if err != nil || status >= 400 { log.Printf("%s: %v %s %v", r.URL.Path, status, r.RemoteAddr, err) } - ws.WriteControl(websocket.CloseInternalServerErr, []byte(txt), time.Now().Add(10*time.Second)) + if err := ws.WriteControl(websocket.CloseInternalServerErr, []byte(txt), time.Now().Add(WSWriteDeadline)); err != nil { //nolint:shadow + log.Print(err) + } } var commandsHandler = withUser(func(w http.ResponseWriter, r *http.Request, d *data) (int, error) { @@ -40,7 +47,7 @@ var commandsHandler = withUser(func(w http.ResponseWriter, r *http.Request, d *d var raw string for { - _, msg, err := conn.ReadMessage() + _, msg, err := conn.ReadMessage() //nolint:shadow if err != nil { wsErr(conn, r, http.StatusInternalServerError, err) return 0, nil @@ -53,8 +60,7 @@ var commandsHandler = withUser(func(w http.ResponseWriter, r *http.Request, d *d } if !d.user.CanExecute(strings.Split(raw, " ")[0]) { - err := conn.WriteMessage(websocket.TextMessage, cmdNotAllowed) - if err != nil { + if err := conn.WriteMessage(websocket.TextMessage, cmdNotAllowed); err != nil { //nolint:shadow wsErr(conn, r, http.StatusInternalServerError, err) } @@ -63,15 +69,13 @@ var commandsHandler = withUser(func(w http.ResponseWriter, r *http.Request, d *d command, err := runner.ParseCommand(d.settings, raw) if err != nil { - err := conn.WriteMessage(websocket.TextMessage, []byte(err.Error())) - if err != nil { + if err := conn.WriteMessage(websocket.TextMessage, []byte(err.Error())); err != nil { //nolint:shadow wsErr(conn, r, http.StatusInternalServerError, err) } - return 0, nil } - cmd := exec.Command(command[0], command[1:]...) + cmd := exec.Command(command[0], command[1:]...) //nolint:gosec cmd.Dir = d.user.FullPath(r.URL.Path) stdout, err := cmd.StdoutPipe() @@ -93,7 +97,9 @@ var commandsHandler = withUser(func(w http.ResponseWriter, r *http.Request, d *d s := bufio.NewScanner(io.MultiReader(stdout, stderr)) for s.Scan() { - conn.WriteMessage(websocket.TextMessage, s.Bytes()) + if err := conn.WriteMessage(websocket.TextMessage, s.Bytes()); err != nil { + log.Print(err) + } } if err := cmd.Wait(); err != nil { diff --git a/http/data.go b/http/data.go index 4be2a94b..ad0b5b1b 100644 --- a/http/data.go +++ b/http/data.go @@ -41,9 +41,9 @@ func (d *data) Check(path string) bool { return true } -func handle(fn handleFunc, prefix string, storage *storage.Storage, server *settings.Server) http.Handler { +func handle(fn handleFunc, prefix string, store *storage.Storage, server *settings.Server) http.Handler { handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - settings, err := storage.Settings.Get() + settings, err := store.Settings.Get() if err != nil { log.Fatalln("ERROR: couldn't get settings") return @@ -51,7 +51,7 @@ func handle(fn handleFunc, prefix string, storage *storage.Storage, server *sett status, err := fn(w, r, &data{ Runner: &runner.Runner{Settings: settings}, - store: storage, + store: store, settings: settings, server: server, }) diff --git a/http/http.go b/http/http.go index 3cf67035..20828f44 100644 --- a/http/http.go +++ b/http/http.go @@ -3,9 +3,10 @@ package http import ( "net/http" + "github.com/gorilla/mux" + "github.com/filebrowser/filebrowser/v2/settings" "github.com/filebrowser/filebrowser/v2/storage" - "github.com/gorilla/mux" ) type modifyRequest struct { @@ -13,11 +14,11 @@ type modifyRequest struct { Which []string `json:"which"` // Answer to: which fields? } -func NewHandler(storage *storage.Storage, server *settings.Server) (http.Handler, error) { +func NewHandler(store *storage.Storage, server *settings.Server) (http.Handler, error) { server.Clean() r := mux.NewRouter() - index, static := getStaticHandlers(storage, server) + index, static := getStaticHandlers(store, server) // NOTE: This fixes the issue where it would redirect if people did not put a // trailing slash in the end. I hate this decision since this allows some awful @@ -25,7 +26,7 @@ func NewHandler(storage *storage.Storage, server *settings.Server) (http.Handler r = r.SkipClean(true) monkey := func(fn handleFunc, prefix string) http.Handler { - return handle(fn, prefix, storage, server) + return handle(fn, prefix, store, server) } r.PathPrefix("/static").Handler(static) diff --git a/http/public.go b/http/public.go index ba2692b3..4467613d 100644 --- a/http/public.go +++ b/http/public.go @@ -46,7 +46,7 @@ func ifPathWithName(r *http.Request) string { pathElements := strings.Split(r.URL.Path, "/") // prevent maliciously constructed parameters like `/api/public/dl/XZzCDnK2_not_exists_hash_name` // len(pathElements) will be 1, and golang will panic `runtime error: index out of range` - if len(pathElements) < 2 { + if len(pathElements) < 2 { //nolint: mnd return r.URL.Path } id := pathElements[len(pathElements)-2] diff --git a/http/raw.go b/http/raw.go index 2d39bed7..e9c393c6 100644 --- a/http/raw.go +++ b/http/raw.go @@ -7,34 +7,37 @@ import ( "path/filepath" "strings" - "github.com/filebrowser/filebrowser/v2/files" - "github.com/filebrowser/filebrowser/v2/users" "github.com/hacdias/fileutils" "github.com/mholt/archiver" + + "github.com/filebrowser/filebrowser/v2/files" + "github.com/filebrowser/filebrowser/v2/users" ) -func parseQueryFiles(r *http.Request, f *files.FileInfo, u *users.User) ([]string, error) { - files := []string{} +func parseQueryFiles(r *http.Request, f *files.FileInfo, _ *users.User) ([]string, error) { + var fileSlice []string names := strings.Split(r.URL.Query().Get("files"), ",") if len(names) == 0 { - files = append(files, f.Path) + fileSlice = append(fileSlice, f.Path) } else { for _, name := range names { - name, err := url.QueryUnescape(strings.Replace(name, "+", "%2B", -1)) + name, err := url.QueryUnescape(strings.Replace(name, "+", "%2B", -1)) //nolint:shadow if err != nil { return nil, err } name = fileutils.SlashClean(name) - files = append(files, filepath.Join(f.Path, name)) + fileSlice = append(fileSlice, filepath.Join(f.Path, name)) } } - return files, nil + return fileSlice, nil } +//nolint: goconst func parseQueryAlgorithm(r *http.Request) (string, archiver.Writer, error) { + // TODO: use enum switch r.URL.Query().Get("algo") { case "zip", "true", "": return ".zip", archiver.NewZip(), nil diff --git a/http/resource.go b/http/resource.go index 6defa007..1b2a9d43 100644 --- a/http/resource.go +++ b/http/resource.go @@ -74,7 +74,7 @@ var resourcePostPutHandler = withUser(func(w http.ResponseWriter, r *http.Reques } defer func() { - io.Copy(ioutil.Discard, r.Body) + _, _ = io.Copy(ioutil.Discard, r.Body) }() // For directories, only allow POST for creation. @@ -119,6 +119,7 @@ var resourcePostPutHandler = withUser(func(w http.ResponseWriter, r *http.Reques return errToStatus(err), err }) +//nolint: goconst var resourcePatchHandler = withUser(func(w http.ResponseWriter, r *http.Request, d *data) (int, error) { src := r.URL.Path dst := r.URL.Query().Get("destination") @@ -134,6 +135,7 @@ var resourcePatchHandler = withUser(func(w http.ResponseWriter, r *http.Request, } switch action { + // TODO: use enum case "copy": if !d.user.Perm.Create { return http.StatusForbidden, nil diff --git a/http/share.go b/http/share.go index c2786535..2c01ca26 100644 --- a/http/share.go +++ b/http/share.go @@ -57,7 +57,9 @@ var sharePostHandler = withPermShare(func(w http.ResponseWriter, r *http.Request var err error s, err = d.store.Share.GetPermanent(r.URL.Path, d.user.ID) if err == nil { - w.Write([]byte(path.Join(d.server.BaseURL, "/share/", s.Hash))) + if _, err := w.Write([]byte(path.Join(d.server.BaseURL, "/share/", s.Hash))); err != nil { + return http.StatusInternalServerError, err + } return 0, nil } } diff --git a/http/static.go b/http/static.go index 0fb4b6e7..d97559b6 100644 --- a/http/static.go +++ b/http/static.go @@ -11,13 +11,14 @@ import ( "text/template" rice "github.com/GeertJohan/go.rice" + "github.com/filebrowser/filebrowser/v2/auth" "github.com/filebrowser/filebrowser/v2/settings" "github.com/filebrowser/filebrowser/v2/storage" "github.com/filebrowser/filebrowser/v2/version" ) -func handleWithStaticData(w http.ResponseWriter, r *http.Request, d *data, box *rice.Box, file, contentType string) (int, error) { +func handleWithStaticData(w http.ResponseWriter, _ *http.Request, d *data, box *rice.Box, file, contentType string) (int, error) { w.Header().Set("Content-Type", contentType) auther, err := d.store.Auth.Get(d.settings.AuthMethod) @@ -41,8 +42,8 @@ func handleWithStaticData(w http.ResponseWriter, r *http.Request, d *data, box * } if d.settings.Branding.Files != "" { - path := filepath.Join(d.settings.Branding.Files, "custom.css") - _, err := os.Stat(path) + fPath := filepath.Join(d.settings.Branding.Files, "custom.css") + _, err := os.Stat(fPath) //nolint:shadow if err != nil && !os.IsNotExist(err) { log.Printf("couldn't load custom styles: %v", err) @@ -54,7 +55,7 @@ func handleWithStaticData(w http.ResponseWriter, r *http.Request, d *data, box * } if d.settings.AuthMethod == auth.MethodJSONAuth { - raw, err := d.store.Auth.Get(d.settings.AuthMethod) + raw, err := d.store.Auth.Get(d.settings.AuthMethod) //nolint:shadow if err != nil { return http.StatusInternalServerError, err } @@ -84,29 +85,29 @@ func handleWithStaticData(w http.ResponseWriter, r *http.Request, d *data, box * return 0, nil } -func getStaticHandlers(storage *storage.Storage, server *settings.Server) (http.Handler, http.Handler) { +func getStaticHandlers(store *storage.Storage, server *settings.Server) (index, static http.Handler) { box := rice.MustFindBox("../frontend/dist") handler := http.FileServer(box.HTTPBox()) - index := handle(func(w http.ResponseWriter, r *http.Request, d *data) (int, error) { + index = handle(func(w http.ResponseWriter, r *http.Request, d *data) (int, error) { if r.Method != http.MethodGet { return http.StatusNotFound, nil } w.Header().Set("x-xss-protection", "1; mode=block") return handleWithStaticData(w, r, d, box, "index.html", "text/html; charset=utf-8") - }, "", storage, server) + }, "", store, server) - static := handle(func(w http.ResponseWriter, r *http.Request, d *data) (int, error) { + static = handle(func(w http.ResponseWriter, r *http.Request, d *data) (int, error) { if r.Method != http.MethodGet { return http.StatusNotFound, nil } if d.settings.Branding.Files != "" { if strings.HasPrefix(r.URL.Path, "img/") { - path := filepath.Join(d.settings.Branding.Files, r.URL.Path) - if _, err := os.Stat(path); err == nil { - http.ServeFile(w, r, path) + fPath := filepath.Join(d.settings.Branding.Files, r.URL.Path) + if _, err := os.Stat(fPath); err == nil { + http.ServeFile(w, r, fPath) return 0, nil } } else if r.URL.Path == "custom.css" && d.settings.Branding.Files != "" { @@ -121,7 +122,7 @@ func getStaticHandlers(storage *storage.Storage, server *settings.Server) (http. } return handleWithStaticData(w, r, d, box, r.URL.Path, "application/javascript; charset=utf-8") - }, "/static/", storage, server) + }, "/static/", store, server) return index, static } diff --git a/http/users.go b/http/users.go index b6a92dc3..ee05ef1c 100644 --- a/http/users.go +++ b/http/users.go @@ -8,9 +8,10 @@ import ( "strconv" "strings" + "github.com/gorilla/mux" + "github.com/filebrowser/filebrowser/v2/errors" "github.com/filebrowser/filebrowser/v2/users" - "github.com/gorilla/mux" ) type modifyUserRequest struct { @@ -27,7 +28,7 @@ func getUserID(r *http.Request) (uint, error) { return uint(i), err } -func getUser(w http.ResponseWriter, r *http.Request) (*modifyUserRequest, error) { +func getUser(_ http.ResponseWriter, r *http.Request) (*modifyUserRequest, error) { if r.Body == nil { return nil, errors.ErrEmptyRequest } diff --git a/http/utils.go b/http/utils.go index 110b327a..ec76fd1e 100644 --- a/http/utils.go +++ b/http/utils.go @@ -10,7 +10,7 @@ import ( "github.com/filebrowser/filebrowser/v2/errors" ) -func renderJSON(w http.ResponseWriter, r *http.Request, data interface{}) (int, error) { +func renderJSON(w http.ResponseWriter, _ *http.Request, data interface{}) (int, error) { marsh, err := json.Marshal(data) if err != nil { diff --git a/runner/parser.go b/runner/parser.go index c9e013b0..249c6d09 100644 --- a/runner/parser.go +++ b/runner/parser.go @@ -3,15 +3,16 @@ package runner import ( "os/exec" - "github.com/filebrowser/filebrowser/v2/settings" "github.com/caddyserver/caddy" + + "github.com/filebrowser/filebrowser/v2/settings" ) // ParseCommand parses the command taking in account if the current // instance uses a shell to run the commands or just calls the binary // directyly. func ParseCommand(s *settings.Settings, raw string) ([]string, error) { - command := []string{} + var command []string if len(s.Shell) == 0 { cmd, args, err := caddy.SplitCommandAndArgs(raw) @@ -27,7 +28,7 @@ func ParseCommand(s *settings.Settings, raw string) ([]string, error) { command = append(command, cmd) command = append(command, args...) } else { - command = append(s.Shell, raw) + command = append(s.Shell, raw) //nolint:gocritic } return command, nil diff --git a/runner/runner.go b/runner/runner.go index d484b0cf..b281ec28 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -60,9 +60,9 @@ func (r *Runner) exec(raw, evt, path, dst string, user *users.User) error { return err } - cmd := exec.Command(command[0], command[1:]...) + cmd := exec.Command(command[0], command[1:]...) //nolint:gosec cmd.Env = append(os.Environ(), fmt.Sprintf("FILE=%s", path)) - cmd.Env = append(cmd.Env, fmt.Sprintf("SCOPE=%s", user.Scope)) + cmd.Env = append(cmd.Env, fmt.Sprintf("SCOPE=%s", user.Scope)) //nolint:gocritic cmd.Env = append(cmd.Env, fmt.Sprintf("TRIGGER=%s", evt)) cmd.Env = append(cmd.Env, fmt.Sprintf("USERNAME=%s", user.Username)) cmd.Env = append(cmd.Env, fmt.Sprintf("DESTINATION=%s", dst)) diff --git a/search/search.go b/search/search.go index 375d7414..4d91a00b 100644 --- a/search/search.go +++ b/search/search.go @@ -4,8 +4,9 @@ import ( "os" "strings" - "github.com/filebrowser/filebrowser/v2/rules" "github.com/spf13/afero" + + "github.com/filebrowser/filebrowser/v2/rules" ) type searchOptions struct { diff --git a/settings/dir.go b/settings/dir.go index 49389724..5781e6d7 100644 --- a/settings/dir.go +++ b/settings/dir.go @@ -17,21 +17,21 @@ var ( ) // MakeUserDir makes the user directory according to settings. -func (settings *Settings) MakeUserDir(username, userScope, serverRoot string) (string, error) { +func (s *Settings) MakeUserDir(username, userScope, serverRoot string) (string, error) { var err error userScope = strings.TrimSpace(userScope) if userScope == "" || userScope == "./" { userScope = "." } - if !settings.CreateUserDir { + if !s.CreateUserDir { return userScope, nil } fs := afero.NewBasePathFs(afero.NewOsFs(), serverRoot) // Use the default auto create logic only if specific scope is not the default scope - if userScope != settings.Defaults.Scope { + if userScope != s.Defaults.Scope { // Try create the dir, for example: settings.Defaults.Scope == "." and userScope == "./foo" if userScope != "." { err = fs.MkdirAll(userScope, os.ModePerm) @@ -50,7 +50,7 @@ func (settings *Settings) MakeUserDir(username, userScope, serverRoot string) (s } // Create default user dir - userHomeBase := settings.Defaults.Scope + string(os.PathSeparator) + "users" + userHomeBase := s.Defaults.Scope + string(os.PathSeparator) + "users" userHome := userHomeBase + string(os.PathSeparator) + username err = fs.MkdirAll(userHome, os.ModePerm) if err != nil { diff --git a/share/storage.go b/share/storage.go index 6f317001..6073cbcc 100644 --- a/share/storage.go +++ b/share/storage.go @@ -33,7 +33,9 @@ func (s *Storage) GetByHash(hash string) (*Link, error) { } if link.Expire != 0 && link.Expire <= time.Now().Unix() { - s.Delete(link.Hash) + if err := s.Delete(link.Hash); err != nil { + return nil, err + } return nil, errors.ErrNotExist } @@ -55,7 +57,9 @@ func (s *Storage) Gets(path string, id uint) ([]*Link, error) { for i, link := range links { if link.Expire != 0 && link.Expire <= time.Now().Unix() { - s.Delete(link.Hash) + if err := s.Delete(link.Hash); err != nil { + return nil, err + } links = append(links[:i], links[i+1:]...) } } diff --git a/storage/bolt/auth.go b/storage/bolt/auth.go index 3bf7ae00..6078f63c 100644 --- a/storage/bolt/auth.go +++ b/storage/bolt/auth.go @@ -2,6 +2,7 @@ package bolt import ( "github.com/asdine/storm" + "github.com/filebrowser/filebrowser/v2/auth" "github.com/filebrowser/filebrowser/v2/errors" "github.com/filebrowser/filebrowser/v2/settings" diff --git a/storage/bolt/bolt.go b/storage/bolt/bolt.go index 2b2dac92..ce67adc0 100644 --- a/storage/bolt/bolt.go +++ b/storage/bolt/bolt.go @@ -2,6 +2,7 @@ package bolt import ( "github.com/asdine/storm" + "github.com/filebrowser/filebrowser/v2/auth" "github.com/filebrowser/filebrowser/v2/settings" "github.com/filebrowser/filebrowser/v2/share" @@ -11,10 +12,10 @@ import ( // NewStorage creates a storage.Storage based on Bolt DB. func NewStorage(db *storm.DB) (*storage.Storage, error) { - users := users.NewStorage(usersBackend{db: db}) - share := share.NewStorage(shareBackend{db: db}) - settings := settings.NewStorage(settingsBackend{db: db}) - auth := auth.NewStorage(authBackend{db: db}, users) + userStore := users.NewStorage(usersBackend{db: db}) + shareStore := share.NewStorage(shareBackend{db: db}) + settingsStore := settings.NewStorage(settingsBackend{db: db}) + authStore := auth.NewStorage(authBackend{db: db}, userStore) err := save(db, "version", 2) if err != nil { @@ -22,9 +23,9 @@ func NewStorage(db *storm.DB) (*storage.Storage, error) { } return &storage.Storage{ - Auth: auth, - Users: users, - Share: share, - Settings: settings, + Auth: authStore, + Users: userStore, + Share: shareStore, + Settings: settingsStore, }, nil } diff --git a/storage/bolt/config.go b/storage/bolt/config.go index 7fad6010..b7142782 100644 --- a/storage/bolt/config.go +++ b/storage/bolt/config.go @@ -2,6 +2,7 @@ package bolt import ( "github.com/asdine/storm" + "github.com/filebrowser/filebrowser/v2/settings" ) @@ -10,12 +11,12 @@ type settingsBackend struct { } func (s settingsBackend) Get() (*settings.Settings, error) { - settings := &settings.Settings{} - return settings, get(s.db, "settings", settings) + set := &settings.Settings{} + return set, get(s.db, "settings", set) } -func (s settingsBackend) Save(settings *settings.Settings) error { - return save(s.db, "settings", settings) +func (s settingsBackend) Save(set *settings.Settings) error { + return save(s.db, "settings", set) } func (s settingsBackend) GetServer() (*settings.Server, error) { diff --git a/storage/bolt/importer/conf.go b/storage/bolt/importer/conf.go index b967c5ec..8100d8c4 100644 --- a/storage/bolt/importer/conf.go +++ b/storage/bolt/importer/conf.go @@ -7,14 +7,14 @@ import ( "os" "path/filepath" - "github.com/filebrowser/filebrowser/v2/auth" - "github.com/filebrowser/filebrowser/v2/users" - "github.com/asdine/storm" - "github.com/filebrowser/filebrowser/v2/settings" - "github.com/filebrowser/filebrowser/v2/storage" toml "github.com/pelletier/go-toml" yaml "gopkg.in/yaml.v2" + + "github.com/filebrowser/filebrowser/v2/auth" + "github.com/filebrowser/filebrowser/v2/settings" + "github.com/filebrowser/filebrowser/v2/storage" + "github.com/filebrowser/filebrowser/v2/users" ) type oldDefs struct { diff --git a/storage/bolt/importer/importer.go b/storage/bolt/importer/importer.go index 06ae2c1b..0356152a 100644 --- a/storage/bolt/importer/importer.go +++ b/storage/bolt/importer/importer.go @@ -2,34 +2,35 @@ package importer import ( "github.com/asdine/storm" + "github.com/filebrowser/filebrowser/v2/storage/bolt" ) // Import imports an old configuration to a newer database. -func Import(oldDB, oldConf, newDB string) error { - old, err := storm.Open(oldDB) +func Import(oldDBPath, oldConf, newDBPath string) error { + oldDB, err := storm.Open(oldDBPath) if err != nil { return err } - defer old.Close() + defer oldDB.Close() - new, err := storm.Open(newDB) + newDB, err := storm.Open(newDBPath) if err != nil { return err } - defer new.Close() + defer newDB.Close() - sto, err := bolt.NewStorage(new) + sto, err := bolt.NewStorage(newDB) if err != nil { return err } - err = importUsers(old, sto) + err = importUsers(oldDB, sto) if err != nil { return err } - err = importConf(old, oldConf, sto) + err = importConf(oldDB, oldConf, sto) if err != nil { return err } diff --git a/storage/bolt/importer/users.go b/storage/bolt/importer/users.go index fd797614..f5c1c4cd 100644 --- a/storage/bolt/importer/users.go +++ b/storage/bolt/importer/users.go @@ -5,10 +5,11 @@ import ( "fmt" "github.com/asdine/storm" + bolt "go.etcd.io/bbolt" + "github.com/filebrowser/filebrowser/v2/rules" "github.com/filebrowser/filebrowser/v2/storage" "github.com/filebrowser/filebrowser/v2/users" - bolt "go.etcd.io/bbolt" ) type oldUser struct { @@ -29,7 +30,7 @@ type oldUser struct { } func readOldUsers(db *storm.DB) ([]*oldUser, error) { - users := []*oldUser{} + var oldUsers []*oldUser err := db.Bolt.View(func(tx *bolt.Tx) error { return tx.Bucket([]byte("User")).ForEach(func(k []byte, v []byte) error { if len(v) > 0 && string(v)[0] == '{' { @@ -40,14 +41,14 @@ func readOldUsers(db *storm.DB) ([]*oldUser, error) { return err } - users = append(users, user) + oldUsers = append(oldUsers, user) } return nil }) }) - return users, err + return oldUsers, err } func convertUsersToNew(old []*oldUser) ([]*users.User, error) { diff --git a/storage/bolt/share.go b/storage/bolt/share.go index 99d53381..f74247ec 100644 --- a/storage/bolt/share.go +++ b/storage/bolt/share.go @@ -3,6 +3,7 @@ package bolt import ( "github.com/asdine/storm" "github.com/asdine/storm/q" + "github.com/filebrowser/filebrowser/v2/errors" "github.com/filebrowser/filebrowser/v2/share" ) @@ -46,5 +47,9 @@ func (s shareBackend) Save(l *share.Link) error { } func (s shareBackend) Delete(hash string) error { - return s.db.DeleteStruct(&share.Link{Hash: hash}) + err := s.db.DeleteStruct(&share.Link{Hash: hash}) + if err == storm.ErrNotFound { + return nil + } + return err } diff --git a/storage/bolt/users.go b/storage/bolt/users.go index cfd12f74..0db1c0d4 100644 --- a/storage/bolt/users.go +++ b/storage/bolt/users.go @@ -4,6 +4,7 @@ import ( "reflect" "github.com/asdine/storm" + "github.com/filebrowser/filebrowser/v2/errors" "github.com/filebrowser/filebrowser/v2/users" ) @@ -38,17 +39,17 @@ func (st usersBackend) GetBy(i interface{}) (user *users.User, err error) { } func (st usersBackend) Gets() ([]*users.User, error) { - users := []*users.User{} - err := st.db.All(&users) + var allUsers []*users.User + err := st.db.All(&allUsers) if err == storm.ErrNotFound { return nil, errors.ErrNotExist } if err != nil { - return users, err + return allUsers, err } - return users, err + return allUsers, err } func (st usersBackend) Update(user *users.User, fields ...string) error { diff --git a/storage/bolt/utils.go b/storage/bolt/utils.go index f34237f9..f45b4851 100644 --- a/storage/bolt/utils.go +++ b/storage/bolt/utils.go @@ -2,6 +2,7 @@ package bolt import ( "github.com/asdine/storm" + "github.com/filebrowser/filebrowser/v2/errors" ) diff --git a/storage/storage.go b/storage/storage.go index 3db87a23..79f7bcdd 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -7,7 +7,7 @@ import ( "github.com/filebrowser/filebrowser/v2/users" ) -// Storage is a storage powered by a Backend whih makes the neccessary +// Storage is a storage powered by a Backend which makes the necessary // verifications when fetching and saving data to ensure consistency. type Storage struct { Users *users.Storage diff --git a/users/storage.go b/users/storage.go index 3de6d247..f9b3aa80 100644 --- a/users/storage.go +++ b/users/storage.go @@ -40,7 +40,9 @@ func (s *Storage) Get(baseScope string, id interface{}) (user *User, err error) if err != nil { return } - user.Clean(baseScope) + if err := user.Clean(baseScope); err != nil { + return nil, err + } return } @@ -52,7 +54,9 @@ func (s *Storage) Gets(baseScope string) ([]*User, error) { } for _, user := range users { - user.Clean(baseScope) + if err := user.Clean(baseScope); err != nil { //nolint:shadow + return nil, err + } } return users, err diff --git a/users/users.go b/users/users.go index 4fdbef6c..1df0a89b 100644 --- a/users/users.go +++ b/users/users.go @@ -4,11 +4,11 @@ import ( "path/filepath" "regexp" - "github.com/filebrowser/filebrowser/v2/errors" + "github.com/spf13/afero" + "github.com/filebrowser/filebrowser/v2/errors" "github.com/filebrowser/filebrowser/v2/files" "github.com/filebrowser/filebrowser/v2/rules" - "github.com/spf13/afero" ) // ViewMode describes a view mode. @@ -52,6 +52,7 @@ var checkableFields = []string{ // Clean cleans up a user and verifies if all its fields // are alright to be saved. +//nolint:gocyclo func (u *User) Clean(baseScope string, fields ...string) error { if len(fields) == 0 { fields = checkableFields