serve webdav,http: fix crash when the server fails to start - fixes #9882

When the HTTP server failed to initialise, for example because the
listen address was already in use, rclone panicked with a nil pointer
dereference instead of reporting the error.

The deferred cleanup in the constructor read the provider from the
named return value, but `return nil, err` sets that to nil before the
deferred function runs. Use a local variable for the server instead.

cmd/serve/http/http.go

@@ -170,8 +170,8 @@ func (s *HTTP) auth(r *http.Request, user, pass string) (value any, err error) {
 	return VFS, err
 }
 
-func newServer(ctx context.Context, f fs.Fs, opt *Options, vfsOpt *vfscommon.Options, proxyOpt *proxy.Options) (s *HTTP, err error) {
-	s = &HTTP{
+func newServer(ctx context.Context, f fs.Fs, opt *Options, vfsOpt *vfscommon.Options, proxyOpt *proxy.Options) (_ *HTTP, err error) {
+	s := &HTTP{
 		f:        f,
 		ctx:      ctx,
 		opt:      *opt,

cmd/serve/http/http_test.go

@@ -542,3 +542,20 @@ func TestRc(t *testing.T) {
 		"vfs_cache_mode": "off",
 	})
 }
+
+// TestNewServerError checks that a server initialisation failure is
+// returned as an error rather than panicking in the cleanup.
+func TestNewServerError(t *testing.T) {
+	ctx := context.Background()
+	f, err := fs.NewFs(ctx, "testdata/files")
+	require.NoError(t, err)
+
+	opts := Options{
+		HTTP: libhttp.DefaultCfg(),
+	}
+	opts.HTTP.ListenAddr = []string{"localhost:-1"}
+
+	s, err := newServer(ctx, f, &opts, &vfscommon.Opt, &proxy.Opt)
+	require.Error(t, err)
+	assert.Nil(t, s)
+}

cmd/serve/webdav/webdav.go

@@ -258,8 +258,8 @@ func webDAVCompressMiddleware() func(http.Handler) http.Handler {
 var _ webdav.FileSystem = (*WebDAV)(nil)
 
 // Make a new WebDAV to serve the remote
-func newWebDAV(ctx context.Context, f fs.Fs, opt *Options, vfsOpt *vfscommon.Options, proxyOpt *proxy.Options) (w *WebDAV, err error) {
-	w = &WebDAV{
+func newWebDAV(ctx context.Context, f fs.Fs, opt *Options, vfsOpt *vfscommon.Options, proxyOpt *proxy.Options) (_ *WebDAV, err error) {
+	w := &WebDAV{
 		f:            f,
 		ctx:          ctx,
 		opt:          *opt,

cmd/serve/webdav/webdav_test.go

@@ -438,3 +438,17 @@ func TestMoveOverwriteFalseStillRejects(t *testing.T) {
 	assert.Equal(t, http.StatusPreconditionFailed, resp.StatusCode,
 		"MOVE with explicit Overwrite: F must still return 412 when destination exists")
 }
+
+// TestNewWebDAVError checks that a server initialisation failure is
+// returned as an error rather than panicking in the cleanup.
+func TestNewWebDAVError(t *testing.T) {
+	f, err := fs.NewFs(context.Background(), t.TempDir())
+	require.NoError(t, err)
+
+	opt := Opt
+	opt.HTTP.ListenAddr = []string{"localhost:-1"}
+
+	w, err := newWebDAV(context.Background(), f, &opt, &vfscommon.Opt, &proxy.Opt)
+	require.Error(t, err)
+	assert.Nil(t, w)
+}