Skip to content

httpnoctx (27th linter): 3 prod *http.Client.Get calls omit context — convert to NewRequestWithContext+Do, then enforce in CI #39016

Description

@github-actions

Summary

httpnoctx is the 27th custom analyzer (registered at cmd/linters/main.go:60). It flags http.Client/package-level HTTP calls that take no context.Context, recommending http.NewRequestWithContext + client.Do so cancellation and deadlines propagate. It is not yet enforced in CI (.github/workflows/cgo.yml:1122 LINTER_FLAGS lists 12 analyzers; httpnoctx is absent).

Running it over production pkg/ surfaces 3 genuine context-free HTTP calls — outbound network requests with no caller-driven cancellation or deadline. All use a *http.Client pointer receiver, so they are true positives the analyzer already catches; nothing here is a linter bug. This is an enforce-readiness/triage task: fix the 3 sites, then turn the linter on.

Violations (3 prod sites)

# Location Call Enclosing fn has ctx?
1 pkg/cli/deps_outdated.go:169 client.Get(url) (Go-proxy @latest query) No — getLatestVersion(modulePath, currentVersion string, verbose bool)
2 pkg/parser/remote_fetch.go:613 rawClient.Get(rawURL) (github.com/ghraw download) No — downloadFileViaRawURL(owner, repo, filePath, ref string)
3 pkg/cli/mcp_inspect_mcp_scripts_server.go:54 client.Get(url) (server-ready poll loop) No — waitForServerReady(port int, timeout time.Duration, verbose bool)
Evidence snippets
// pkg/cli/deps_outdated.go:168
client := &http.Client{Timeout: 5 * time.Second}
resp, err := client.Get(url)

// pkg/parser/remote_fetch.go:609
rawClient := &http.Client{Timeout: constants.DefaultHTTPClientTimeout}
// #nosec G107 ...
resp, err := rawClient.Get(rawURL)

// pkg/cli/mcp_inspect_mcp_scripts_server.go:48
client := &http.Client{Timeout: 1 * time.Second}
for time.Now().Before(deadline) {
    resp, err := client.Get(url)

Completeness: a sweep of pkg/ (non-test) for http.{Get,Head,Post,PostForm}, package-level shortcuts, and value/embedded http.Client found no other matches. mcp_registry.go:85 uses c.httpClient.Do(req) (already request-based) and is correctly not a violation.

Impact

Each call relies solely on the client's Timeout. There is no way for a caller to cancel an in-flight request (Ctrl-C, parent timeout, errgroup cancellation). downloadFileViaRawURL fetches user-supplied import URLs and waitForServerReady polls in a loop — both are exactly the spots where a hung socket should be cancellable by the surrounding command's context.

Recommendation

For each site, thread a context.Context into the function (callers generally already have one available, e.g. the CLI command context) and convert:

req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil { return ... }
resp, err := client.Do(req)

For waitForServerReady, derive a per-iteration ctx, cancel := context.WithTimeout(parentCtx, 1*time.Second) (or pass the deadline as a context) instead of relying on the client Timeout.

Then append -httpnoctx to LINTER_FLAGS in .github/workflows/cgo.yml:1122 so regressions are blocked.

Validation checklist

  • All 3 sites converted to NewRequestWithContext + Do with a propagated context.
  • make golint-custom LINTER_FLAGS="-httpnoctx -test=false" reports zero violations.
  • -httpnoctx added to the CI LINTER_FLAGS list.
  • Existing tests for the three call sites still pass.

Effort: M (3 call sites + light context threading through callers, then a one-line CI flag).


Found by Sergo R35. Distinct from the open httpnoctx precision issue (#39017), which is about a latent linter false negative rather than these real call sites.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

cookieIssue Monster Loves Cookies!sergo

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions