Skip to content

Resources are not closed on several error paths #7833

Description

@vvdvortsova

Issue Details

Hello, I found these warnings using the Svace static analyzer.

1. reverseproxy/httptransport.go: leaked net.Conn in dialContext

In the custom dialContext closure (around httptransport.go:275-365), conn is obtained from dialer.DialContext(ctx, network, address) at line 292. When h.ProxyProtocol is set, there are several places afterward where the function returns an error without closing conn even though the connection was already established:

  • line 303: return nil, fmt.Errorf(proxyProtocolInfo missing from context)
  • line 312: return nil, fmt.Errorf(unrecognized proxy protocol version)
  • line 329: return nil, fmt.Errorf(unexpected remote addr type in proxy protocol info)
  • line 351: return nil, DialError{err}

2. cmd/packagesfuncs.go: leaked resp.Body in downloadBuild on HTTP error responses

In downloadBuild (packagesfuncs.go:280-305), resp, err := http.Get(...) succeeds at line 286. If resp.StatusCode >= 400 (line 290), the function decodes resp.Body into a details struct and then returns in both outcomes:

  • decode failed return nil, fmt.Errorf(...) (line 300)
  • decode succeeded return nil, fmt.Errorf(...) (line 302)

resp.Body is never closed in either branch, so the underlying TCP connection can't be reused/released back to the connection pool.

3. modules/logging/filewriter.go : leaked *os.File when roll: false

In the file-opening logic (filewriter.go:222-244): file, err := os.OpenFile(...) succeeds at line 223, then info, err := file.Stat() is called at line 227. If roll is true (the default), file.Close() is called unconditionally right after, so that path is fine. But if roll is explicitly set to false, file is not closed there. Then, if a custom mode is configured (configuredMode != 0) and the earlier Stat() call returned an error, the function does return nil, fmt.Errorf(...) at line 235 without ever closing file.

This is a narrow edge case (it requires Stat() to fail right after a successful OpenFile, e.g. the file got removed/renamed concurrently), but it's a real leak when it happens: roll: false is a supported, documented configuration.

4. caddytest/caddytest.go : leaked *os.File from os.CreateTemp in validateTestPrerequisites

In validateTestPrerequisites (caddytest.go:277-298), f, err := os.CreateTemp("", "") opens a temp file at line 277. The registered t.Cleanup callback only does os.Remove(f.Name()) it removes the file from disk but never calls f.Close(). The file handle itself is never closed anywhere else in the function either (including the early return err on line 285 if fmt.Fprintf fails). The descriptor stays open until the GC finalizer kicks in or the test process exits.

This is test-helper code, so it doesn't affect production traffic, but caddytest is used across the whole integration test suite, and leaked descriptors can accumulate within a single go test run.

I'm happy to send a PR for any/all of these if that's preferred over just reporting them, let me know.

Assistance Disclosure

AI used

If AI was used, describe the extent to which it was used.

I'm not a native English speaker, so I used an AI to help translate this report from my native language. The technical analysis and code reading were done by me.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions