Checkout and four other handlers take a field from the URL when the posted form leaves it out #205

New Issue

2026-10-09T16:02:12Z

cgalo5758 commented

2026-10-09 16:02:12 +00:00

What a contributor runs into

The rule for reading a request is that a handler never calls r.FormValue and reads r.PostForm instead, whether or not it uses the forms library (docs/operator-ux-conventions.md, "Server-side validation"). r.FormValue reads the posted body and the URL's query string together. When both carry a field, the body's value wins. When the body leaves the field out, the URL's value is used.

Five POST handlers still read their fields that way:

Checkout (HandleCheckout, internal/server/billing.go) reads price_id. A POST /billing/checkout?price_id=… with an empty body opens a Stripe Checkout for that price. The Products page and the add-ons list both post price_id in the body.
Keep plan and Cancel plan (PostKeep and PostCancel, internal/server/member_plan_moves.go) read ladder_id. Cancel also reads timing, so ?timing=immediate in the URL ends the subscription now instead of at the period end.
The product's Stripe sync (SyncProductToStripe, internal/server/operator_billing.go) reads price_id.
Revoke and transition (RevokeGrantAndTransition, internal/server/operator_enrollment.go) reads org_id.

The tests post every one of these fields in the body, so nothing covers the URL as a second source. The console refuses a cross-origin POST before any of these handlers run, so the URL opens no new way in.

Why it costs

Each of these handlers accepts a request its page never sends, and no test says whether it should. A new handler copied from one of them copies the read along with it. Nothing in make lint refuses r.FormValue.

Where

internal/server/billing.go (HandleCheckout), internal/server/member_plan_moves.go (PostKeep, PostCancel), internal/server/operator_billing.go (SyncProductToStripe), internal/server/operator_enrollment.go (RevokeGrantAndTransition).

Done when

The five handlers read these fields from r.PostForm. For each handler, a test that posts the field only in the URL gets the same answer as a request with the field missing.
.golangci.yml refuses r.FormValue. parseAddress (internal/integrations/fedwiki/web/requests.go) also calls it, for POST routes and for one DELETE route whose fields htmx sends in the URL, so it reads each field from where its method carries it.
docs/operator-ux-conventions.md describes r.FormValue accurately: the posted value wins, and the URL's value is used when the body has none.

## What a contributor runs into The rule for reading a request is that a handler never calls `r.FormValue` and reads `r.PostForm` instead, whether or not it uses the forms library (`docs/operator-ux-conventions.md`, "Server-side validation"). `r.FormValue` reads the posted body and the URL's query string together. When both carry a field, the body's value wins. When the body leaves the field out, the URL's value is used. Five POST handlers still read their fields that way: - **Checkout** (`HandleCheckout`, `internal/server/billing.go`) reads `price_id`. A `POST /billing/checkout?price_id=…` with an empty body opens a Stripe Checkout for that price. The Products page and the add-ons list both post `price_id` in the body. - **Keep plan and Cancel plan** (`PostKeep` and `PostCancel`, `internal/server/member_plan_moves.go`) read `ladder_id`. Cancel also reads `timing`, so `?timing=immediate` in the URL ends the subscription now instead of at the period end. - **The product's Stripe sync** (`SyncProductToStripe`, `internal/server/operator_billing.go`) reads `price_id`. - **Revoke and transition** (`RevokeGrantAndTransition`, `internal/server/operator_enrollment.go`) reads `org_id`. The tests post every one of these fields in the body, so nothing covers the URL as a second source. The console refuses a cross-origin POST before any of these handlers run, so the URL opens no new way in. ## Why it costs Each of these handlers accepts a request its page never sends, and no test says whether it should. A new handler copied from one of them copies the read along with it. Nothing in `make lint` refuses `r.FormValue`. ## Where `internal/server/billing.go` (`HandleCheckout`), `internal/server/member_plan_moves.go` (`PostKeep`, `PostCancel`), `internal/server/operator_billing.go` (`SyncProductToStripe`), `internal/server/operator_enrollment.go` (`RevokeGrantAndTransition`). ## Done when - The five handlers read these fields from `r.PostForm`. For each handler, a test that posts the field only in the URL gets the same answer as a request with the field missing. - `.golangci.yml` refuses `r.FormValue`. `parseAddress` (`internal/integrations/fedwiki/web/requests.go`) also calls it, for POST routes and for one DELETE route whose fields htmx sends in the URL, so it reads each field from where its method carries it. - `docs/operator-ux-conventions.md` describes `r.FormValue` accurately: the posted value wins, and the URL's value is used when the body has none.

cgalo5758 added the

kind

debt

area/billing area/operator-ui area/member-ui labels 2026-10-09 16:02:12 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: wiki-cafe/member-console#205