Fix QA nits from PR #5: DomainID type, io.ReadAll limit, variable naming #6

New issue

Closed

opened 2026-06-14 16:41:48 +00:00 by ldraney · 0 comments

ldraney commented

2026-06-14 16:41:48 +00:00

Owner

Type

Bug

Lineage

Follow-up from PR #5 QA review. Non-blocking nits that should be cleaned up.

Repo

ldraney/godaddy-tofu

What Broke

Three code quality issues identified during QA re-review of PR #5:

Domain.DomainID is float64 but should be int64 — swagger spec says integer. float64 can lose precision on large IDs.
io.ReadAll in error path has no size limit — client.go:67 reads the entire error response body with no cap. A malformed response could exhaust memory.
ListDomains local variable url shadows net/url import — domains.go:38 still uses url as local var name, inconsistent with the reqURL naming applied in the encoding fix (commit 4a97205).

Repro Steps

Read pkg/godaddy/domains.go — DomainID is float64, ListDomains uses url not reqURL
Read pkg/godaddy/client.go — io.ReadAll at line 67 has no size limit

Expected Behavior

DomainID should be int64
Error body read should be capped (e.g., io.LimitReader(resp.Body, 1<<20))
All local URL variables should use reqURL consistently

Environment

pkg/godaddy/ client code on main branch after PR #5 merge

File Targets

pkg/godaddy/domains.go — fix DomainID type, rename url to reqURL in ListDomains
pkg/godaddy/client.go — wrap io.ReadAll with io.LimitReader

Acceptance Criteria

Domain.DomainID is int64
Error response body read is capped
All local URL variables use reqURL consistently
go build ./... and go vet ./... pass

Constraints

Small, focused cleanup — no functional changes beyond the three nits

Follow-up from PR #5
Can be bundled with #4 (integration tests) or done as a standalone PR

### Type Bug ### Lineage Follow-up from PR #5 QA review. Non-blocking nits that should be cleaned up. ### Repo `ldraney/godaddy-tofu` ### What Broke Three code quality issues identified during QA re-review of PR #5: 1. **`Domain.DomainID` is `float64` but should be `int64`** — swagger spec says integer. `float64` can lose precision on large IDs. 2. **`io.ReadAll` in error path has no size limit** — `client.go:67` reads the entire error response body with no cap. A malformed response could exhaust memory. 3. **`ListDomains` local variable `url` shadows `net/url` import** — `domains.go:38` still uses `url` as local var name, inconsistent with the `reqURL` naming applied in the encoding fix (commit 4a97205). ### Repro Steps 1. Read `pkg/godaddy/domains.go` — `DomainID` is `float64`, `ListDomains` uses `url` not `reqURL` 2. Read `pkg/godaddy/client.go` — `io.ReadAll` at line 67 has no size limit ### Expected Behavior - `DomainID` should be `int64` - Error body read should be capped (e.g., `io.LimitReader(resp.Body, 1<<20)`) - All local URL variables should use `reqURL` consistently ### Environment `pkg/godaddy/` client code on main branch after PR #5 merge ### File Targets - `pkg/godaddy/domains.go` — fix `DomainID` type, rename `url` to `reqURL` in `ListDomains` - `pkg/godaddy/client.go` — wrap `io.ReadAll` with `io.LimitReader` ### Acceptance Criteria - [ ] `Domain.DomainID` is `int64` - [ ] Error response body read is capped - [ ] All local URL variables use `reqURL` consistently - [ ] `go build ./...` and `go vet ./...` pass ### Constraints - Small, focused cleanup — no functional changes beyond the three nits ### Related - Follow-up from PR #5 - Can be bundled with #4 (integration tests) or done as a standalone PR