From 0df7466d2b0771380cc198e083e1ec0819f51036 Mon Sep 17 00:00:00 2001 From: Zach Kipp Date: Tue, 10 Jan 2023 21:05:44 -0800 Subject: [PATCH] cmd/rcd: Fix command docs to include command specific prefix (#6675) This change addresses two issues with commands that re-used flags from common packages: 1) cobra.Command definitions did not include the command specific prefix in doc strings. 2) Command specific flag prefixes were added after generating command doc strings. --- cmd/help.go | 2 -- cmd/rcd/rcd.go | 6 ++++- cmd/serve/http/http.go | 12 ++++++--- cmd/serve/restic/restic.go | 10 ++++--- cmd/serve/webdav/webdav.go | 10 ++++--- fs/rc/rcflags/rcflags.go | 8 +++--- lib/http/auth.go | 34 ++++++++++++++++++----- lib/http/auth_test.go | 15 +++++++++++ lib/http/server.go | 55 +++++++++++++++++++++++++------------- lib/http/server_test.go | 9 +++++++ lib/http/template.go | 27 ++++++++++++++++--- lib/http/template_test.go | 15 +++++++++++ 12 files changed, 159 insertions(+), 44 deletions(-) create mode 100644 lib/http/auth_test.go create mode 100644 lib/http/template_test.go diff --git a/cmd/help.go b/cmd/help.go index ecc34b63e..c0371089a 100644 --- a/cmd/help.go +++ b/cmd/help.go @@ -13,7 +13,6 @@ import ( "github.com/rclone/rclone/fs/config/configflags" "github.com/rclone/rclone/fs/filter/filterflags" "github.com/rclone/rclone/fs/log/logflags" - "github.com/rclone/rclone/fs/rc/rcflags" "github.com/rclone/rclone/lib/atexit" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -174,7 +173,6 @@ func setupRootCommand(rootCmd *cobra.Command) { // Add global flags configflags.AddFlags(ci, pflag.CommandLine) filterflags.AddFlags(pflag.CommandLine) - rcflags.AddFlags(pflag.CommandLine) logflags.AddFlags(pflag.CommandLine) Root.Run = runRoot diff --git a/cmd/rcd/rcd.go b/cmd/rcd/rcd.go index 818580f3a..542a287b0 100644 --- a/cmd/rcd/rcd.go +++ b/cmd/rcd/rcd.go @@ -15,7 +15,11 @@ import ( "github.com/spf13/cobra" ) +// flagPrefix is the prefix used to uniquely identify command line flags. +const flagPrefix = "rc-" + func init() { + rcflags.AddFlags(cmd.Root.Flags(), flagPrefix) cmd.Root.AddCommand(commandDefinition) } @@ -32,7 +36,7 @@ for GET requests on the URL passed in. It will also open the URL in the browser when rclone is run. See the [rc documentation](/rc/) for more info on the rc flags. -` + libhttp.Help + libhttp.TemplateHelp + libhttp.AuthHelp, +` + libhttp.Help(flagPrefix) + libhttp.TemplateHelp(flagPrefix) + libhttp.AuthHelp(flagPrefix), Annotations: map[string]string{ "versionIntroduced": "v1.45", }, diff --git a/cmd/serve/http/http.go b/cmd/serve/http/http.go index 0385b21c7..6dc728fd5 100644 --- a/cmd/serve/http/http.go +++ b/cmd/serve/http/http.go @@ -44,11 +44,15 @@ var DefaultOpt = Options{ // Opt is options set by command line flags var Opt = DefaultOpt +// flagPrefix is the prefix used to uniquely identify command line flags. +// It is intentionally empty for this package. +const flagPrefix = "" + func init() { flagSet := Command.Flags() - libhttp.AddAuthFlagsPrefix(flagSet, "", &Opt.Auth) - libhttp.AddHTTPFlagsPrefix(flagSet, "", &Opt.HTTP) - libhttp.AddTemplateFlagsPrefix(flagSet, "", &Opt.Template) + libhttp.AddAuthFlagsPrefix(flagSet, flagPrefix, &Opt.Auth) + libhttp.AddHTTPFlagsPrefix(flagSet, flagPrefix, &Opt.HTTP) + libhttp.AddTemplateFlagsPrefix(flagSet, flagPrefix, &Opt.Template) vfsflags.AddFlags(flagSet) proxyflags.AddFlags(flagSet) } @@ -68,7 +72,7 @@ The server will log errors. Use ` + "`-v`" + ` to see access logs. ` + "`--bwlimit`" + ` will be respected for file transfers. Use ` + "`--stats`" + ` to control the stats printing. -` + libhttp.Help + libhttp.TemplateHelp + libhttp.AuthHelp + vfs.Help + proxy.Help, +` + libhttp.Help(flagPrefix) + libhttp.TemplateHelp(flagPrefix) + libhttp.AuthHelp(flagPrefix) + vfs.Help + proxy.Help, Annotations: map[string]string{ "versionIntroduced": "v1.39", }, diff --git a/cmd/serve/restic/restic.go b/cmd/serve/restic/restic.go index fbdc947e1..db6427dad 100644 --- a/cmd/serve/restic/restic.go +++ b/cmd/serve/restic/restic.go @@ -47,10 +47,14 @@ var DefaultOpt = Options{ // Opt is options set by command line flags var Opt = DefaultOpt +// flagPrefix is the prefix used to uniquely identify command line flags. +// It is intentionally empty for this package. +const flagPrefix = "" + func init() { flagSet := Command.Flags() - libhttp.AddAuthFlagsPrefix(flagSet, "", &Opt.Auth) - libhttp.AddHTTPFlagsPrefix(flagSet, "", &Opt.HTTP) + libhttp.AddAuthFlagsPrefix(flagSet, flagPrefix, &Opt.Auth) + libhttp.AddHTTPFlagsPrefix(flagSet, flagPrefix, &Opt.HTTP) flags.BoolVarP(flagSet, &Opt.Stdio, "stdio", "", false, "Run an HTTP2 server on stdin/stdout") flags.BoolVarP(flagSet, &Opt.AppendOnly, "append-only", "", false, "Disallow deletion of repository data") flags.BoolVarP(flagSet, &Opt.PrivateRepos, "private-repos", "", false, "Users can only access their private repo") @@ -142,7 +146,7 @@ these **must** end with /. Eg The` + "`--private-repos`" + ` flag can be used to limit users to repositories starting with a path of ` + "`//`" + `. -` + libhttp.Help + libhttp.AuthHelp, +` + libhttp.Help(flagPrefix) + libhttp.AuthHelp(flagPrefix), Annotations: map[string]string{ "versionIntroduced": "v1.40", }, diff --git a/cmd/serve/webdav/webdav.go b/cmd/serve/webdav/webdav.go index daaa537f9..cf9ec713c 100644 --- a/cmd/serve/webdav/webdav.go +++ b/cmd/serve/webdav/webdav.go @@ -48,10 +48,14 @@ var DefaultOpt = Options{ // Opt is options set by command line flags var Opt = DefaultOpt +// flagPrefix is the prefix used to uniquely identify command line flags. +// It is intentionally empty for this package. +const flagPrefix = "" + func init() { flagSet := Command.Flags() - libhttp.AddAuthFlagsPrefix(flagSet, "", &Opt.Auth) - libhttp.AddHTTPFlagsPrefix(flagSet, "", &Opt.HTTP) + libhttp.AddAuthFlagsPrefix(flagSet, flagPrefix, &Opt.Auth) + libhttp.AddHTTPFlagsPrefix(flagSet, flagPrefix, &Opt.HTTP) libhttp.AddTemplateFlagsPrefix(flagSet, "", &Opt.Template) vfsflags.AddFlags(flagSet) proxyflags.AddFlags(flagSet) @@ -103,7 +107,7 @@ Create a new DWORD BasicAuthLevel with value 2. https://learn.microsoft.com/en-us/office/troubleshoot/powerpoint/office-opens-blank-from-sharepoint -` + libhttp.Help + libhttp.TemplateHelp + libhttp.AuthHelp + vfs.Help + proxy.Help, +` + libhttp.Help(flagPrefix) + libhttp.TemplateHelp(flagPrefix) + libhttp.AuthHelp(flagPrefix) + vfs.Help + proxy.Help, Annotations: map[string]string{ "versionIntroduced": "v1.39", }, diff --git a/fs/rc/rcflags/rcflags.go b/fs/rc/rcflags/rcflags.go index db2412e06..b51e7bf3f 100644 --- a/fs/rc/rcflags/rcflags.go +++ b/fs/rc/rcflags/rcflags.go @@ -13,7 +13,7 @@ var ( ) // AddFlags adds the remote control flags to the flagSet -func AddFlags(flagSet *pflag.FlagSet) { +func AddFlags(flagSet *pflag.FlagSet, commonFlagPrefix string) { rc.AddOption("rc", &Opt) flags.BoolVarP(flagSet, &Opt.Enabled, "rc", "", false, "Enable the remote control server") flags.StringVarP(flagSet, &Opt.Files, "rc-files", "", "", "Path to local files to serve on the HTTP server") @@ -28,7 +28,7 @@ func AddFlags(flagSet *pflag.FlagSet) { flags.BoolVarP(flagSet, &Opt.EnableMetrics, "rc-enable-metrics", "", false, "Enable prometheus metrics on /metrics") flags.DurationVarP(flagSet, &Opt.JobExpireDuration, "rc-job-expire-duration", "", Opt.JobExpireDuration, "Expire finished async jobs older than this value") flags.DurationVarP(flagSet, &Opt.JobExpireInterval, "rc-job-expire-interval", "", Opt.JobExpireInterval, "Interval to check for expired async jobs") - Opt.HTTP.AddFlagsPrefix(flagSet, "rc-") - Opt.Auth.AddFlagsPrefix(flagSet, "rc-") - Opt.Template.AddFlagsPrefix(flagSet, "rc-") + Opt.HTTP.AddFlagsPrefix(flagSet, commonFlagPrefix) + Opt.Auth.AddFlagsPrefix(flagSet, commonFlagPrefix) + Opt.Template.AddFlagsPrefix(flagSet, commonFlagPrefix) } diff --git a/lib/http/auth.go b/lib/http/auth.go index 50b024f70..0e7fd6d29 100644 --- a/lib/http/auth.go +++ b/lib/http/auth.go @@ -1,20 +1,25 @@ package http import ( + "bytes" + "html/template" + "log" + "github.com/rclone/rclone/fs/config/flags" "github.com/spf13/pflag" ) -// AuthHelp contains text describing the http authentication to add to the command help. -var AuthHelp = ` +// AuthHelp returns text describing the http authentication to add to the command help. +func AuthHelp(prefix string) string { + help := ` #### Authentication By default this will serve files without needing a login. You can either use an htpasswd file which can take lots of users, or -set a single username and password with the ` + "`--user` and `--pass`" + ` flags. +set a single username and password with the ` + "`--{{ .Prefix }}user` and `--{{ .Prefix }}pass`" + ` flags. -Use ` + "`--htpasswd /path/to/htpasswd`" + ` to provide an htpasswd file. This is +Use ` + "`--{{ .Prefix }}htpasswd /path/to/htpasswd`" + ` to provide an htpasswd file. This is in standard apache format and supports MD5, SHA1 and BCrypt for basic authentication. Bcrypt is recommended. @@ -26,10 +31,27 @@ To create an htpasswd file: The password file can be updated while rclone is running. -Use ` + "`--realm`" + ` to set the authentication realm. +Use ` + "`--{{ .Prefix }}realm`" + ` to set the authentication realm. -Use ` + "`--salt`" + ` to change the password hashing salt from the default. +Use ` + "`--{{ .Prefix }}salt`" + ` to change the password hashing salt from the default. ` + tmpl, err := template.New("auth help").Parse(help) + if err != nil { + log.Fatal("Fatal error parsing template", err) + } + + data := struct { + Prefix string + }{ + Prefix: prefix, + } + buf := &bytes.Buffer{} + err = tmpl.Execute(buf, data) + if err != nil { + log.Fatal("Fatal error executing template", err) + } + return buf.String() +} // CustomAuthFn if used will be used to authenticate user, pass. If an error // is returned then the user is not authenticated. diff --git a/lib/http/auth_test.go b/lib/http/auth_test.go new file mode 100644 index 000000000..dec115a12 --- /dev/null +++ b/lib/http/auth_test.go @@ -0,0 +1,15 @@ +package http + +import ( + "strings" + "testing" +) + +func TestHelpPrefixAuth(t *testing.T) { + // This test assumes template variables are placed correctly. + const testPrefix = "server-help-test" + helpMessage := AuthHelp(testPrefix) + if !strings.Contains(helpMessage, testPrefix) { + t.Fatal("flag prefix not found") + } +} diff --git a/lib/http/server.go b/lib/http/server.go index a7c1f64b2..89e6666c7 100644 --- a/lib/http/server.go +++ b/lib/http/server.go @@ -2,6 +2,7 @@ package http import ( + "bytes" "context" "crypto/tls" "crypto/x509" @@ -23,56 +24,74 @@ import ( "github.com/spf13/pflag" ) -// Help contains text describing the http server to add to the command +// Help returns text describing the http server to add to the command // help. -var Help = ` +func Help(prefix string) string { + help := ` ### Server options -Use ` + "`--addr`" + ` to specify which IP address and port the server should -listen on, eg ` + "`--addr 1.2.3.4:8000` or `--addr :8080`" + ` to listen to all +Use ` + "`--{{ .Prefix }}addr`" + ` to specify which IP address and port the server should +listen on, eg ` + "`--{{ .Prefix }}addr 1.2.3.4:8000` or `--{{ .Prefix }}addr :8080`" + ` to listen to all IPs. By default it only listens on localhost. You can use port :0 to let the OS choose an available port. -If you set ` + "`--addr`" + ` to listen on a public or LAN accessible IP address +If you set ` + "`--{{ .Prefix }}addr`" + ` to listen on a public or LAN accessible IP address then using Authentication is advised - see the next section for info. You can use a unix socket by setting the url to ` + "`unix:///path/to/socket`" + ` or just by using an absolute path name. Note that unix sockets bypass the authentication - this is expected to be done with file system permissions. -` + "`--addr`" + ` may be repeated to listen on multiple IPs/ports/sockets. +` + "`--{{ .Prefix }}addr`" + ` may be repeated to listen on multiple IPs/ports/sockets. -` + "`--server-read-timeout` and `--server-write-timeout`" + ` can be used to +` + "`--{{ .Prefix }}server-read-timeout` and `--{{ .Prefix }}server-write-timeout`" + ` can be used to control the timeouts on the server. Note that this is the total time for a transfer. -` + "`--max-header-bytes`" + ` controls the maximum number of bytes the server will +` + "`--{{ .Prefix }}max-header-bytes`" + ` controls the maximum number of bytes the server will accept in the HTTP header. -` + "`--baseurl`" + ` controls the URL prefix that rclone serves from. By default -rclone will serve from the root. If you used ` + "`--baseurl \"/rclone\"`" + ` then +` + "`--{{ .Prefix }}baseurl`" + ` controls the URL prefix that rclone serves from. By default +rclone will serve from the root. If you used ` + "`--{{ .Prefix }}baseurl \"/rclone\"`" + ` then rclone would serve from a URL starting with "/rclone/". This is useful if you wish to proxy rclone serve. Rclone automatically -inserts leading and trailing "/" on ` + "`--baseurl`" + `, so ` + "`--baseurl \"rclone\"`" + `, -` + "`--baseurl \"/rclone\"` and `--baseurl \"/rclone/\"`" + ` are all treated +inserts leading and trailing "/" on ` + "`--{{ .Prefix }}baseurl`" + `, so ` + "`--{{ .Prefix }}baseurl \"rclone\"`" + `, +` + "`--{{ .Prefix }}baseurl \"/rclone\"` and `--{{ .Prefix }}baseurl \"/rclone/\"`" + ` are all treated identically. #### TLS (SSL) By default this will serve over http. If you want you can serve over -https. You will need to supply the ` + "`--cert` and `--key`" + ` flags. +https. You will need to supply the ` + "`--{{ .Prefix }}cert` and `--{{ .Prefix }}key`" + ` flags. If you wish to do client side certificate validation then you will need to -supply ` + "`--client-ca`" + ` also. +supply ` + "`--{{ .Prefix }}client-ca`" + ` also. -` + "`--cert`" + ` should be a either a PEM encoded certificate or a concatenation -of that with the CA certificate. ` + "`--key`" + ` should be the PEM encoded -private key and ` + "`--client-ca`" + ` should be the PEM encoded client +` + "`--{{ .Prefix }}cert`" + ` should be a either a PEM encoded certificate or a concatenation +of that with the CA certificate. ` + "`--k{{ .Prefix }}ey`" + ` should be the PEM encoded +private key and ` + "`--{{ .Prefix }}client-ca`" + ` should be the PEM encoded client certificate authority certificate. ---min-tls-version is minimum TLS version that is acceptable. Valid +--{{ .Prefix }}min-tls-version is minimum TLS version that is acceptable. Valid values are "tls1.0", "tls1.1", "tls1.2" and "tls1.3" (default "tls1.0"). ` + tmpl, err := template.New("server help").Parse(help) + if err != nil { + log.Fatal("Fatal error parsing template", err) + } + + data := struct { + Prefix string + }{ + Prefix: prefix, + } + buf := &bytes.Buffer{} + err = tmpl.Execute(buf, data) + if err != nil { + log.Fatal("Fatal error executing template", err) + } + return buf.String() +} // Middleware function signature required by chi.Router.Use() type Middleware func(http.Handler) http.Handler diff --git a/lib/http/server_test.go b/lib/http/server_test.go index 506a78d51..4cb303685 100644 --- a/lib/http/server_test.go +++ b/lib/http/server_test.go @@ -442,3 +442,12 @@ func TestNewServerTLS(t *testing.T) { }) } } + +func TestHelpPrefixServer(t *testing.T) { + // This test assumes template variables are placed correctly. + const testPrefix = "server-help-test" + helpMessage := Help(testPrefix) + if !strings.Contains(helpMessage, testPrefix) { + t.Fatal("flag prefix not found") + } +} diff --git a/lib/http/template.go b/lib/http/template.go index bed80dc30..fbd514463 100644 --- a/lib/http/template.go +++ b/lib/http/template.go @@ -1,8 +1,10 @@ package http import ( + "bytes" "embed" "html/template" + "log" "os" "time" @@ -11,11 +13,12 @@ import ( "github.com/rclone/rclone/fs/config/flags" ) -// TemplateHelp describes how to use a custom template -var TemplateHelp = ` +// TemplateHelp returns a string that describes how to use a custom template +func TemplateHelp(prefix string) string { + help := ` #### Template -` + "`--template`" + ` allows a user to specify a custom markup template for HTTP +` + "`--{{ .Prefix }}template`" + ` allows a user to specify a custom markup template for HTTP and WebDAV serve functions. The server exports the following markup to be used within the template to server pages: @@ -39,6 +42,24 @@ to be used within the template to server pages: |-- .ModTime | The UTC timestamp of an entry. | ` + tmpl, err := template.New("template help").Parse(help) + if err != nil { + log.Fatal("Fatal error parsing template", err) + } + + data := struct { + Prefix string + }{ + Prefix: prefix, + } + buf := &bytes.Buffer{} + err = tmpl.Execute(buf, data) + if err != nil { + log.Fatal("Fatal error executing template", err) + } + return buf.String() +} + // TemplateConfig for the templating functionality type TemplateConfig struct { Path string diff --git a/lib/http/template_test.go b/lib/http/template_test.go new file mode 100644 index 000000000..c6d787330 --- /dev/null +++ b/lib/http/template_test.go @@ -0,0 +1,15 @@ +package http + +import ( + "strings" + "testing" +) + +func TestHelpPrefixTemplate(t *testing.T) { + // This test assumes template variables are placed correctly. + const testPrefix = "template-help-test" + helpMessage := TemplateHelp(testPrefix) + if !strings.Contains(helpMessage, testPrefix) { + t.Fatal("flag prefix not found") + } +}