Url Query Modification Lost

ID

go.url_query_modification_lost

Severity

high

Remediation Complexity

simple

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

Go

Tags

CWE:563, correctness, reliability

Description

Reports a mutation of the url.Values returned by (*url.URL).Query() that is immediately thrown away, such as u.Query().Add("k", "v"). Query() re-parses the raw query string and returns a fresh copy every time it is called, so mutating that copy never updates the URL.

Rationale

Because Query() returns a new url.Values on each call, chaining a mutator directly onto it (u.Query().Add(…​), .Set(…​), .Del(…​)) modifies a value that is discarded the moment the statement ends — the intended change to the URL silently never happens. The values must be captured, modified, and written back through u.RawQuery.

To stay free of false positives the rule fires only when the receiver of the mutator resolves to net/url.Values, the previous call in the chain is Query(), and the result is a discarded statement. Reading the copy (u.Query().Encode()) is fine, and a same-named method on an unrelated type is never reported.

u.Query().Add("k", "v") // FLAW — Query() returns a copy; the Add is lost

q := u.Query()          // OK — keep the copy, modify it, write it back
q.Add("k", "v")
u.RawQuery = q.Encode()

u.Query().Encode()      // OK — read-only access

Remediation

Capture the values in a variable, mutate that variable, then assign the encoded result back: q := u.Query(); q.Add("k", "v"); u.RawQuery = q.Encode().