From f2a390b2d4510afd87fd5b0053f5404b27e84e16 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Mon, 7 Sep 2026 16:55:51 +0100 Subject: [PATCH] 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 | 4 ++-- cmd/serve/http/http_test.go | 17 +++++++++++++++++ cmd/serve/webdav/webdav.go | 4 ++-- cmd/serve/webdav/webdav_test.go | 14 ++++++++++++++ 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/cmd/serve/http/http.go b/cmd/serve/http/http.go index e867928bf..9ec487c2a 100644 --- a/cmd/serve/http/http.go +++ b/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, diff --git a/cmd/serve/http/http_test.go b/cmd/serve/http/http_test.go index c9010c305..4567fa434 100644 --- a/cmd/serve/http/http_test.go +++ b/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) +} diff --git a/cmd/serve/webdav/webdav.go b/cmd/serve/webdav/webdav.go index d3420bb40..f3cf2aa82 100644 --- a/cmd/serve/webdav/webdav.go +++ b/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, diff --git a/cmd/serve/webdav/webdav_test.go b/cmd/serve/webdav/webdav_test.go index bd1482641..0254ea2bd 100644 --- a/cmd/serve/webdav/webdav_test.go +++ b/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) +}