From b961e07c571871d74493602d0b2a630a9ca85222 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Thu, 1 Nov 2018 17:20:04 +0000 Subject: [PATCH] rc: ensure rclone fails to start up if the --rc port is in use already --- cmd/cmd.go | 5 +++- cmd/rcd/rcd.go | 11 ++++++-- fs/rc/rcserver/rcserver.go | 49 ++++++++++++++++++--------------- fs/rc/rcserver/rcserver_test.go | 7 +++-- 4 files changed, 44 insertions(+), 28 deletions(-) diff --git a/cmd/cmd.go b/cmd/cmd.go index 8e9cebd3d..6c85c7131 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -353,7 +353,10 @@ func initConfig() { fs.Debugf("rclone", "Version %q starting with parameters %q", fs.Version, os.Args) // Start the remote control server if configured - rcserver.Start(&rcflags.Opt) + _, err = rcserver.Start(&rcflags.Opt) + if err != nil { + log.Fatalf("Failed to start remote control: %v", err) + } // Setup CPU profiling if desired if *cpuProfile != "" { diff --git a/cmd/rcd/rcd.go b/cmd/rcd/rcd.go index ca77f2638..4a6fe2a8d 100644 --- a/cmd/rcd/rcd.go +++ b/cmd/rcd/rcd.go @@ -37,8 +37,13 @@ See the [rc documentation](/rc/) for more info on the rc flags. if len(args) > 0 { rcflags.Opt.Files = args[0] } - rcserver.Start(&rcflags.Opt) - // Run the rc forever - select {} + s, err := rcserver.Start(&rcflags.Opt) + if err != nil { + log.Fatalf("Failed to start remote control: %v", err) + } + if s == nil { + log.Fatal("rc server not configured") + } + s.Wait() }, } diff --git a/fs/rc/rcserver/rcserver.go b/fs/rc/rcserver/rcserver.go index b58105848..2dc3cda9c 100644 --- a/fs/rc/rcserver/rcserver.go +++ b/fs/rc/rcserver/rcserver.go @@ -21,25 +21,28 @@ import ( ) // Start the remote control server if configured -func Start(opt *rc.Options) { +// +// If the server wasn't configured the *Server returned may be nil +func Start(opt *rc.Options) (*Server, error) { if opt.Enabled { // Serve on the DefaultServeMux so can have global registrations appear s := newServer(opt, http.DefaultServeMux) - go s.serve() + return s, s.Serve() } + return nil, nil } -// server contains everything to run the server -type server struct { - srv *httplib.Server +// Server contains everything to run the rc server +type Server struct { + *httplib.Server files http.Handler opt *rc.Options } -func newServer(opt *rc.Options, mux *http.ServeMux) *server { - s := &server{ - srv: httplib.NewServer(mux, &opt.HTTPOptions), - opt: opt, +func newServer(opt *rc.Options, mux *http.ServeMux) *Server { + s := &Server{ + Server: httplib.NewServer(mux, &opt.HTTPOptions), + opt: opt, } mux.HandleFunc("/", s.handler) @@ -55,18 +58,20 @@ func newServer(opt *rc.Options, mux *http.ServeMux) *server { return s } -// serve runs the http server - doesn't return -func (s *server) serve() { - err := s.srv.Serve() +// 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() if err != nil { - fs.Errorf(nil, "Opening listener: %v", err) + return err } - fs.Logf(nil, "Serving remote control on %s", s.srv.URL()) + fs.Logf(nil, "Serving remote control on %s", s.URL()) // Open the files in the browser if set if s.files != nil { - _ = open.Start(s.srv.URL()) + _ = open.Start(s.URL()) } - s.srv.Wait() + return nil } // writeError writes a formatted error to the output @@ -94,7 +99,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) { +func (s *Server) handler(w http.ResponseWriter, r *http.Request) { path := strings.TrimLeft(r.URL.Path, "/") w.Header().Add("Access-Control-Allow-Origin", "*") @@ -116,7 +121,7 @@ func (s *server) handler(w http.ResponseWriter, r *http.Request) { } } -func (s *server) handlePost(w http.ResponseWriter, r *http.Request, path string) { +func (s *Server) handlePost(w http.ResponseWriter, r *http.Request, path string) { contentType := r.Header.Get("Content-Type") values := r.URL.Query() @@ -184,11 +189,11 @@ func (s *server) handlePost(w http.ResponseWriter, r *http.Request, path string) } } -func (s *server) handleOptions(w http.ResponseWriter, r *http.Request, path string) { +func (s *Server) handleOptions(w http.ResponseWriter, r *http.Request, path string) { w.WriteHeader(http.StatusOK) } -func (s *server) serveRoot(w http.ResponseWriter, r *http.Request) { +func (s *Server) serveRoot(w http.ResponseWriter, r *http.Request) { remotes := config.FileSections() sort.Strings(remotes) directory := serve.NewDirectory("") @@ -201,7 +206,7 @@ func (s *server) serveRoot(w http.ResponseWriter, r *http.Request) { directory.Serve(w, r) } -func (s *server) serveRemote(w http.ResponseWriter, r *http.Request, path string, fsName string) { +func (s *Server) serveRemote(w http.ResponseWriter, r *http.Request, path string, fsName string) { f, err := rc.GetCachedFs(fsName) if err != nil { writeError(path, nil, w, errors.Wrap(err, "failed to make Fs"), http.StatusInternalServerError) @@ -234,7 +239,7 @@ func (s *server) serveRemote(w http.ResponseWriter, r *http.Request, path string // Match URLS of the form [fs]/remote var fsMatch = regexp.MustCompile(`^\[(.*?)\](.*)$`) -func (s *server) handleGet(w http.ResponseWriter, r *http.Request, path string) { +func (s *Server) handleGet(w http.ResponseWriter, r *http.Request, path string) { // Look to see if this has an fs in the path match := fsMatch.FindStringSubmatch(path) switch { diff --git a/fs/rc/rcserver/rcserver_test.go b/fs/rc/rcserver/rcserver_test.go index 69864f2c4..816c014af 100644 --- a/fs/rc/rcserver/rcserver_test.go +++ b/fs/rc/rcserver/rcserver_test.go @@ -36,8 +36,11 @@ func TestRcServer(t *testing.T) { opt.Files = testFs mux := http.NewServeMux() rcServer := newServer(&opt, mux) - go rcServer.serve() - defer rcServer.srv.Close() + assert.NoError(t, rcServer.Serve()) + defer func() { + rcServer.Close() + rcServer.Wait() + }() // Do the simplest possible test to check the server is alive // Do it a few times to wait for the server to start