Skip to content

httpnoctx precision: isHTTPClientReceiver only matches *http.Client pointers — value & embedded http.Client receivers escape (la [Content truncated due to length] #39017

Description

@github-actions

Summary

isHTTPClientReceiver (pkg/linters/httpnoctx/httpnoctx.go:91-106) only recognises a receiver whose static type is exactly *types.Pointer wrapping *types.Named(net/http.Client):

ptr, ok := t.(*types.Pointer)        // line 96 — requires a pointer
if !ok { return false }
named, ok := ptr.Elem().(*types.Named)

But the stdlib Get/Head/Post/PostForm methods have pointer receivers, so they are also in the method set of any addressable http.Client value. The following compile and are equally context-free, yet pass.TypesInfo.TypeOf(...) returns http.Client (a *types.Named, not a *types.Pointer), so the call is silently skipped:

var c http.Client; c.Get(url)              // local value (addressable)
type S struct{ client http.Client }; s.client.Get(url)   // value field
type W struct{ http.Client }; w.Get(url)   // embedded client

This is a false negative: the analyzer's own Doc says it "reports http.Client and package-level HTTP calls that do not accept a context.Context" — value and embedded http.Client receivers fall squarely within that contract, but the pointer-only type check misses them.

Evidence

  • Gate code: pkg/linters/httpnoctx/httpnoctx.go:96 (t.(*types.Pointer) is mandatory before the net/http.Client identity check at line 105).
  • Testdata covers only *http.Client parameters — pkg/linters/httpnoctx/testdata/src/httpnoctx/httpnoctx.go:12-28. No value-receiver, struct-field, or embedded-client case exists, so the gap is untested.
  • Contrast: the package-level path (isHTTPPackage, lines 109-123) already uses proper object identity (ObjectOf*types.PkgNameImported().Path()), so this is an internal inconsistency, not a design choice.

Current production impact

Latent. Every current prod HTTP client is constructed as &http.Client{...} (pointer), so there are zero active misses today. Filing to harden the linter before a value/embedded-client pattern is introduced and slips past enforcement.

Recommendation

Unwrap an optional pointer, then do the net/http.Client identity check on the resulting named type:

func isHTTPClientReceiver(pass *analysis.Pass, expr ast.Expr) bool {
    t := pass.TypesInfo.TypeOf(expr)
    if t == nil { return false }
    if ptr, ok := t.(*types.Pointer); ok { t = ptr.Elem() } // accept value OR pointer
    named, ok := t.(*types.Named)
    if !ok { return false }
    obj := named.Obj()
    return obj.Name() == "Client" && obj.Pkg() != nil && obj.Pkg().Path() == "net/http"
}

Add testdata covering: (a) var c http.Client; c.Get(...), (b) a struct field typed http.Client, (c) an embedded http.Client. Embedded receivers may need the selector's resolved receiver type rather than sel.X's type — confirm with the added testdata.

Validation checklist

  • isHTTPClientReceiver accepts both value and pointer net/http.Client.
  • New testdata // want lines added for value, struct-field, and embedded receivers.
  • Existing pointer-receiver want expectations unchanged (no new false positives on non-http.Client Get/Post methods).
  • go test ./pkg/linters/httpnoctx/... passes.

Effort: S (single file + testdata).


Found by Sergo R35. Orthogonal to the enforce/triage issue (#39016), which fixes 3 real *http.Client.Get prod sites.

Generated by 🤖 Sergo - Serena Go Expert · 247.3 AIC · ⌖ 10.6 AIC · ⊞ 6.3K ·

  • expires on Jun 19, 2026, 9:20 PM UTC-08:00

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