rcd: refactor rclone rc server to use lib/http

cmd/rcd/rcd.go

@@ -11,6 +11,7 @@ import (
 	"github.com/rclone/rclone/fs/rc/rcflags"
 	"github.com/rclone/rclone/fs/rc/rcserver"
 	"github.com/rclone/rclone/lib/atexit"
+	libhttp "github.com/rclone/rclone/lib/http"
 	"github.com/spf13/cobra"
 )
 
@@ -31,7 +32,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,
 	Annotations: map[string]string{
 		"versionIntroduced": "v1.45",
 	},

fs/rc/config_test.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/rclone/rclone/cmd/serve/httplib"
 	"github.com/rclone/rclone/fs"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -79,7 +78,6 @@ func TestOptionsGetMarshal(t *testing.T) {
 	ci := fs.GetConfig(ctx)
 
 	// Add some real options
-	AddOption("http", &httplib.DefaultOpt)
 	AddOption("main", ci)
 	AddOption("rc", &DefaultOpt)
 

fs/rc/rc.go

@@ -13,12 +13,14 @@ import (
 	_ "net/http/pprof" // install the pprof http handlers
 	"time"
 
-	"github.com/rclone/rclone/cmd/serve/httplib"
+	libhttp "github.com/rclone/rclone/lib/http"
 )
 
 // Options contains options for the remote control server
 type Options struct {
-	HTTPOptions              httplib.Options
+	HTTP                     libhttp.Config
+	Auth                     libhttp.AuthConfig
+	Template                 libhttp.TemplateConfig
 	Enabled                  bool   // set to enable the server
 	Serve                    bool   // set to serve files from remotes
 	Files                    string // set to enable serving files locally
@@ -36,14 +38,16 @@ type Options struct {
 
 // DefaultOpt is the default values used for Options
 var DefaultOpt = Options{
-	HTTPOptions:       httplib.DefaultOpt,
+	HTTP:              libhttp.DefaultCfg(),
+	Auth:              libhttp.DefaultAuthCfg(),
+	Template:          libhttp.DefaultTemplateCfg(),
 	Enabled:           false,
 	JobExpireDuration: 60 * time.Second,
 	JobExpireInterval: 10 * time.Second,
 }
 
 func init() {
-	DefaultOpt.HTTPOptions.ListenAddr = "localhost:5572"
+	DefaultOpt.HTTP.ListenAddr = []string{"localhost:5572"}
 }
 
 // WriteJSON writes JSON in out to w

fs/rc/rcflags/rcflags.go

@@ -2,7 +2,6 @@
 package rcflags
 
 import (
-	"github.com/rclone/rclone/cmd/serve/httplib/httpflags"
 	"github.com/rclone/rclone/fs/config/flags"
 	"github.com/rclone/rclone/fs/rc"
 	"github.com/spf13/pflag"
@@ -29,5 +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")
-	httpflags.AddFlagsPrefix(flagSet, "rc-", &Opt.HTTPOptions)
+	Opt.HTTP.AddFlagsPrefix(flagSet, "rc-")
+	Opt.Auth.AddFlagsPrefix(flagSet, "rc-")
+	Opt.Template.AddFlagsPrefix(flagSet, "rc-")
 }

fs/rc/rcserver/rcserver.go

@@ -18,9 +18,9 @@ import (
 	"sync"
 	"time"
 
+	"github.com/go-chi/chi/v5/middleware"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
-	"github.com/rclone/rclone/cmd/serve/httplib"
 	"github.com/rclone/rclone/fs"
 	"github.com/rclone/rclone/fs/accounting"
 	"github.com/rclone/rclone/fs/cache"
@@ -31,6 +31,7 @@ import (
 	"github.com/rclone/rclone/fs/rc/jobs"
 	"github.com/rclone/rclone/fs/rc/rcflags"
 	"github.com/rclone/rclone/fs/rc/webgui"
+	libhttp "github.com/rclone/rclone/lib/http"
 	"github.com/rclone/rclone/lib/http/serve"
 	"github.com/rclone/rclone/lib/random"
 	"github.com/skratchdot/open-golang/open"
@@ -59,7 +60,10 @@ func Start(ctx context.Context, opt *rc.Options) (*Server, error) {
 	jobs.SetOpt(opt) // set the defaults for jobs
 	if opt.Enabled {
 		// Serve on the DefaultServeMux so can have global registrations appear
-		s := newServer(ctx, opt, http.DefaultServeMux)
+		s, err := newServer(ctx, opt, http.DefaultServeMux)
+		if err != nil {
+			return nil, err
+		}
 		return s, s.Serve()
 	}
 	return nil, nil
@@ -67,14 +71,14 @@ func Start(ctx context.Context, opt *rc.Options) (*Server, error) {
 
 // Server contains everything to run the rc server
 type Server struct {
-	*httplib.Server
 	ctx            context.Context // for global config
+	server         *libhttp.Server
 	files          http.Handler
 	pluginsHandler http.Handler
 	opt            *rc.Options
 }
 
-func newServer(ctx context.Context, opt *rc.Options, mux *http.ServeMux) *Server {
+func newServer(ctx context.Context, opt *rc.Options, mux *http.ServeMux) (*Server, error) {
 	fileHandler := http.Handler(nil)
 	pluginsHandler := http.Handler(nil)
 	// Add some more mime types which are often missing
@@ -97,16 +101,16 @@ func newServer(ctx context.Context, opt *rc.Options, mux *http.ServeMux) *Server
 		if opt.NoAuth {
 			fs.Logf(nil, "It is recommended to use web gui with auth.")
 		} else {
-			if opt.HTTPOptions.BasicUser == "" && opt.HTTPOptions.HtPasswd == "" {
+			if opt.Auth.BasicUser == "" && opt.Auth.HtPasswd == "" {
-				opt.HTTPOptions.BasicUser = "gui"
+				opt.Auth.BasicUser = "gui"
-				fs.Infof(nil, "No username specified. Using default username: %s \n", rcflags.Opt.HTTPOptions.BasicUser)
+				fs.Infof(nil, "No username specified. Using default username: %s \n", rcflags.Opt.Auth.BasicUser)
 			}
-			if opt.HTTPOptions.BasicPass == "" && opt.HTTPOptions.HtPasswd == "" {
+			if opt.Auth.BasicPass == "" && opt.Auth.HtPasswd == "" {
 				randomPass, err := random.Password(128)
 				if err != nil {
 					log.Fatalf("Failed to make password: %v", err)
 				}
-				opt.HTTPOptions.BasicPass = randomPass
+				opt.Auth.BasicPass = randomPass
 				fs.Infof(nil, "No password specified. Using random password: %s \n", randomPass)
 			}
 		}
@@ -119,53 +123,76 @@ func newServer(ctx context.Context, opt *rc.Options, mux *http.ServeMux) *Server
 	}
 
 	s := &Server{
-		Server:         httplib.NewServer(mux, &opt.HTTPOptions),
 		ctx:            ctx,
 		opt:            opt,
 		files:          fileHandler,
 		pluginsHandler: pluginsHandler,
 	}
-	mux.HandleFunc("/", s.handler)
 
-	return s
+	var err error
+	s.server, err = libhttp.NewServer(ctx,
+		libhttp.WithConfig(opt.HTTP),
+		libhttp.WithAuth(opt.Auth),
+		libhttp.WithTemplate(opt.Template),
+	)
+	if err != nil {
+		return nil, fmt.Errorf("failed to init server: %w", err)
+	}
+
+	router := s.server.Router()
+	router.Use(
+		middleware.SetHeader("Accept-Ranges", "bytes"),
+		middleware.SetHeader("Server", "rclone/"+fs.Version),
+	)
+
+	// Add the debug handler which is installed in the default mux
+	router.Handle("/debug/*", mux)
+
+	// FIXME split these up into individual functions
+	router.Get("/*", s.handler)
+	router.Head("/*", s.handler)
+	router.Post("/*", s.handler)
+	router.Options("/*", s.handler)
+
+	return s, nil
 }
 
 // Serve runs the http server in the background.
 //
 // Use s.Close() and s.Wait() to shutdown server
 func (s *Server) Serve() error {
-	err := s.Server.Serve()
+	s.server.Serve()
-	if err != nil {
-		return err
-	}
-	fs.Logf(nil, "Serving remote control on %s", s.URL())
-	// Open the files in the browser if set
-	if s.files != nil {
-		openURL, err := url.Parse(s.URL())
-		if err != nil {
-			return fmt.Errorf("invalid serving URL: %w", err)
-		}
-		// Add username, password into the URL if they are set
-		user, pass := s.opt.HTTPOptions.BasicUser, s.opt.HTTPOptions.BasicPass
-		if user != "" && pass != "" {
-			openURL.User = url.UserPassword(user, pass)
 
-			// Base64 encode username and password to be sent through url
+	for _, URL := range s.server.URLs() {
-			loginToken := user + ":" + pass
+		fs.Logf(nil, "Serving remote control on %s", URL)
-			parameters := url.Values{}
+		// Open the files in the browser if set
-			encodedToken := base64.URLEncoding.EncodeToString([]byte(loginToken))
+		if s.files != nil {
-			fs.Debugf(nil, "login_token %q", encodedToken)
+			openURL, err := url.Parse(URL)
-			parameters.Add("login_token", encodedToken)
+			if err != nil {
-			openURL.RawQuery = parameters.Encode()
+				return fmt.Errorf("invalid serving URL: %w", err)
-			openURL.RawPath = "/#/login"
+			}
-		}
+			// Add username, password into the URL if they are set
-		// Don't open browser if serving in testing environment or required not to do so.
+			user, pass := s.opt.Auth.BasicUser, s.opt.Auth.BasicPass
-		if flag.Lookup("test.v") == nil && !s.opt.WebGUINoOpenBrowser {
+			if user != "" && pass != "" {
-			if err := open.Start(openURL.String()); err != nil {
+				openURL.User = url.UserPassword(user, pass)
-				fs.Errorf(nil, "Failed to open Web GUI in browser: %v. Manually access it at: %s", err, openURL.String())
+
+				// Base64 encode username and password to be sent through url
+				loginToken := user + ":" + pass
+				parameters := url.Values{}
+				encodedToken := base64.URLEncoding.EncodeToString([]byte(loginToken))
+				fs.Debugf(nil, "login_token %q", encodedToken)
+				parameters.Add("login_token", encodedToken)
+				openURL.RawQuery = parameters.Encode()
+				openURL.RawPath = "/#/login"
+			}
+			// Don't open browser if serving in testing environment or required not to do so.
+			if flag.Lookup("test.v") == nil && !s.opt.WebGUINoOpenBrowser {
+				if err := open.Start(openURL.String()); err != nil {
+					fs.Errorf(nil, "Failed to open Web GUI in browser: %v. Manually access it at: %s", err, openURL.String())
+				}
+			} else {
+				fs.Logf(nil, "Web GUI is not automatically opening browser. Navigate to %s to use.", openURL.String())
 			}
-		} else {
-			fs.Logf(nil, "Web GUI is not automatically opening browser. Navigate to %s to use.", openURL.String())
 		}
 	}
 	return nil
@@ -185,11 +212,7 @@ func writeError(path string, in rc.Params, w http.ResponseWriter, err error, sta
 
 // handler reads incoming requests and dispatches them
 func (s *Server) handler(w http.ResponseWriter, r *http.Request) {
-	urlPath, ok := s.Path(w, r)
+	path := strings.TrimLeft(r.URL.Path, "/")
-	if !ok {
-		return
-	}
-	path := strings.TrimLeft(urlPath, "/")
 
 	allowOrigin := rcflags.Opt.AccessControlAllowOrigin
 	if allowOrigin != "" {
@@ -200,7 +223,12 @@ func (s *Server) handler(w http.ResponseWriter, r *http.Request) {
 		})
 		w.Header().Add("Access-Control-Allow-Origin", allowOrigin)
 	} else {
-		w.Header().Add("Access-Control-Allow-Origin", s.URL())
+		urls := s.server.URLs()
+		if len(urls) == 1 {
+			w.Header().Add("Access-Control-Allow-Origin", urls[0])
+		} else {
+			fs.Errorf(nil, "Warning, need exactly 1 URL for Access-Control-Allow-Origin, got %d %q", len(urls), urls)
+		}
 	}
 
 	// echo back access control headers client needs
@@ -260,7 +288,7 @@ func (s *Server) handlePost(w http.ResponseWriter, r *http.Request, path string)
 	}
 
 	// Check to see if it requires authorisation
-	if !s.opt.NoAuth && call.AuthRequired && !s.UsingAuth() {
+	if !s.opt.NoAuth && call.AuthRequired && !s.server.UsingAuth() {
 		writeError(path, in, w, fmt.Errorf("authentication must be set up on the rc server to use %q or the --rc-no-auth flag must be in use", path), http.StatusForbidden)
 		return
 	}
@@ -305,7 +333,7 @@ func (s *Server) handleOptions(w http.ResponseWriter, r *http.Request, path stri
 func (s *Server) serveRoot(w http.ResponseWriter, r *http.Request) {
 	remotes := config.FileSections()
 	sort.Strings(remotes)
-	directory := serve.NewDirectory("", s.HTMLTemplate)
+	directory := serve.NewDirectory("", s.server.HTMLTemplate())
 	directory.Name = "List of all rclone remotes."
 	q := url.Values{}
 	for _, remote := range remotes {
@@ -333,7 +361,7 @@ func (s *Server) serveRemote(w http.ResponseWriter, r *http.Request, path string
 			return
 		}
 		// Make the entries for display
-		directory := serve.NewDirectory(path, s.HTMLTemplate)
+		directory := serve.NewDirectory(path, s.server.HTMLTemplate())
 		for _, entry := range entries {
 			_, isDir := entry.(fs.Directory)
 			//directory.AddHTMLEntry(entry.Remote(), isDir, entry.Size(), entry.ModTime(r.Context()))
@@ -401,3 +429,13 @@ func (s *Server) handleGet(w http.ResponseWriter, r *http.Request, path string)
 	}
 	http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound)
 }
+
+// Wait blocks while the server is serving requests
+func (s *Server) Wait() {
+	s.server.Wait()
+}
+
+// Shutdown gracefully shuts down the server
+func (s *Server) Shutdown() error {
+	return s.server.Shutdown()
+}

fs/rc/rcserver/rcserver_test.go

@@ -13,14 +13,13 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-
 	_ "github.com/rclone/rclone/backend/local"
 	"github.com/rclone/rclone/fs"
 	"github.com/rclone/rclone/fs/accounting"
 	"github.com/rclone/rclone/fs/config/configfile"
 	"github.com/rclone/rclone/fs/rc"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -49,24 +48,24 @@ func TestMain(m *testing.M) {
 // We'll do the majority of the testing with the httptest framework
 func TestRcServer(t *testing.T) {
 	opt := rc.DefaultOpt
-	opt.HTTPOptions.ListenAddr = testBindAddress
+	opt.HTTP.ListenAddr = []string{testBindAddress}
-	opt.HTTPOptions.Template = testTemplate
+	opt.Template.Path = testTemplate
 	opt.Enabled = true
 	opt.Serve = true
 	opt.Files = testFs
 	mux := http.NewServeMux()
-	rcServer := newServer(context.Background(), &opt, mux)
+	rcServer, err := newServer(context.Background(), &opt, mux)
+	require.NoError(t, err)
 	assert.NoError(t, rcServer.Serve())
 	defer func() {
-		rcServer.Close()
+		assert.NoError(t, rcServer.Shutdown())
 		rcServer.Wait()
 	}()
-	testURL := rcServer.Server.URL()
+	testURL := rcServer.server.URLs()[0]
 
 	// Do the simplest possible test to check the server is alive
 	// Do it a few times to wait for the server to start
 	var resp *http.Response
-	var err error
 	for i := 0; i < 10; i++ {
 		resp, err = http.Get(testURL + "file.txt")
 		if err == nil {
@@ -89,6 +88,8 @@ func TestRcServer(t *testing.T) {
 type testRun struct {
 	Name        string
 	URL         string
+	User        string
+	Pass        string
 	Status      int
 	Method      string
 	Range       string
@@ -103,9 +104,11 @@ type testRun struct {
 func testServer(t *testing.T, tests []testRun, opt *rc.Options) {
 	ctx := context.Background()
 	configfile.Install()
-	mux := http.NewServeMux()
+	opt.Template.Path = testTemplate
-	opt.HTTPOptions.Template = testTemplate
+	rcServer, err := newServer(ctx, opt, http.DefaultServeMux)
-	rcServer := newServer(ctx, opt, mux)
+	require.NoError(t, err)
+	testURL := rcServer.server.URLs()[0]
+	mux := rcServer.server.Router()
 	for _, test := range tests {
 		t.Run(test.Name, func(t *testing.T) {
 			method := test.Method
@@ -125,9 +128,12 @@ func testServer(t *testing.T, tests []testRun, opt *rc.Options) {
 			if test.ContentType != "" {
 				req.Header.Add("Content-Type", test.ContentType)
 			}
+			if test.User != "" && test.Pass != "" {
+				req.SetBasicAuth(test.User, test.Pass)
+			}
 
 			w := httptest.NewRecorder()
-			rcServer.handler(w, req)
+			mux.ServeHTTP(w, req)
 			resp := w.Result()
 
 			assert.Equal(t, test.Status, resp.StatusCode)
@@ -141,6 +147,9 @@ func testServer(t *testing.T, tests []testRun, opt *rc.Options) {
 			}
 
 			for k, v := range test.Headers {
+				if v == "testURL" {
+					v = testURL
+				}
 				assert.Equal(t, v, resp.Header.Get(k), k)
 			}
 		})
@@ -151,6 +160,7 @@ func testServer(t *testing.T, tests []testRun, opt *rc.Options) {
 func newTestOpt() rc.Options {
 	opt := rc.DefaultOpt
 	opt.Enabled = true
+	opt.HTTP.ListenAddr = []string{testBindAddress}
 	return opt
 }
 
@@ -550,7 +560,7 @@ func TestMethods(t *testing.T) {
 		Status:   http.StatusOK,
 		Expected: "",
 		Headers: map[string]string{
-			"Access-Control-Allow-Origin":   "http://localhost:5572/",
+			"Access-Control-Allow-Origin":   "testURL",
 			"Access-Control-Request-Method": "POST, OPTIONS, GET, HEAD",
 			"Access-Control-Allow-Headers":  "authorization, Content-Type",
 		},
@@ -559,12 +569,7 @@ func TestMethods(t *testing.T) {
 		URL:    "",
 		Method: "POTATO",
 		Status: http.StatusMethodNotAllowed,
-		Expected: `{
+		Expected: `Method Not Allowed
-	"error": "method \"POTATO\" not allowed",
-	"input": null,
-	"path": "",
-	"status": 405
-}
 `,
 	}}
 	opt := newTestOpt()
@@ -732,20 +737,40 @@ func TestNoAuth(t *testing.T) {
 
 func TestWithUserPass(t *testing.T) {
 	tests := []testRun{{
-		Name:        "auth",
+		Name:        "authMissing",
+		URL:         "rc/noopauth",
+		Method:      "POST",
+		Body:        `{}`,
+		ContentType: "application/javascript",
+		Status:      http.StatusUnauthorized,
+		Expected:    "401 Unauthorized\n",
+	}, {
+		Name:        "authWrong",
+		URL:         "rc/noopauth",
+		Method:      "POST",
+		Body:        `{}`,
+		ContentType: "application/javascript",
+		Status:      http.StatusUnauthorized,
+		Expected:    "401 Unauthorized\n",
+		User:        "user1",
+		Pass:        "pass2",
+	}, {
+		Name:        "authOK",
 		URL:         "rc/noopauth",
 		Method:      "POST",
 		Body:        `{}`,
 		ContentType: "application/javascript",
 		Status:      http.StatusOK,
 		Expected:    "{}\n",
+		User:        "user",
+		Pass:        "pass",
 	}}
 	opt := newTestOpt()
 	opt.Serve = false
 	opt.Files = ""
 	opt.NoAuth = false
-	opt.HTTPOptions.BasicUser = "user"
+	opt.Auth.BasicUser = "user"
-	opt.HTTPOptions.BasicPass = "pass"
+	opt.Auth.BasicPass = "pass"
 	testServer(t, tests, &opt)
 }
 
@@ -780,3 +805,26 @@ func TestRCAsync(t *testing.T) {
 	opt.Files = ""
 	testServer(t, tests, &opt)
 }
+
+// Check the debug handlers are attached
+func TestRCDebug(t *testing.T) {
+	tests := []testRun{{
+		Name:        "index",
+		URL:         "debug/pprof/",
+		Method:      "GET",
+		ContentType: "text/html",
+		Status:      http.StatusOK,
+		Contains:    regexp.MustCompile(`Types of profiles available`),
+	}, {
+		Name:        "goroutines",
+		URL:         "debug/pprof/goroutine?debug=1",
+		Method:      "GET",
+		ContentType: "text/html",
+		Status:      http.StatusOK,
+		Contains:    regexp.MustCompile(`goroutine profile`),
+	}}
+	opt := newTestOpt()
+	opt.Serve = true
+	opt.Files = ""
+	testServer(t, tests, &opt)
+}