fix: panic recovery, rate limiting, transaction error handling, pagination
Hardening pass over the API surface. No route's auth requirements change.
Resilience:
- Add recover middleware. There was none, so an unhandled panic in any
handler propagated out of the process instead of becoming a 500.
- Add a centralized ErrorHandler so errors and recovered panics return the
same {success,message} envelope as the utils helpers, not Fiber's default
plain-text body. 5xx responses are logged with method and path.
Rate limiting:
- Global 300/min per IP as an abuse backstop, exempting health/readiness
probes and websocket upgrades.
- 10/min shared across every credential endpoint (customer/miler/admin/hub
login, verify-pin, reset-pin, email OTP). PINs are 4 digits, so the whole
keyspace was previously walkable in seconds. One shared limiter instance
means rotating between endpoints doesn't reset the budget.
- Add TRUSTED_PROXIES config. Limits key on c.IP(), which behind a TLS
terminator is the proxy, collapsing every client into one bucket. When set,
X-Forwarded-For is honoured only from those proxies so the header can't be
spoofed to dodge the limit. Logs a warning when unset.
Transactions:
- Check the error on all 51 previously-unchecked tx.Save/Create/Delete/
Model(...).Update/Commit calls across 6 controllers. A failed write inside
a transaction was silently ignored and the request still reported success;
an unchecked Commit could fail with the caller told everything worked.
Each site now rolls back and returns a specific message.
Pagination:
- Add utils.ParsePage/Paginated, reusing the pageno/pagesize convention
GetAdminBookings already established. Default 500, hard cap 1000.
- Apply to the previously unbounded consignments, tripsheets, exceptions,
app-users and clients endpoints. Defaults are high so existing consoles
that don't paginate keep working; the cap only stops a growing table from
being loaded wholesale. total is now a real COUNT, not len(data).
- GetClients also loaded the entire auth table to join in memory; it now
fetches only the current page's rows.
Tests (first in the repo):
- Extract the hyperlocal pincode rule out of BookingPickupComplete into
isHyperlocal so it is testable, covering the short/empty pincode fallback.
- Cover calculateVolumetricWeight and the ParsePage clamping rules.
Repo hygiene:
- Tag scratch/*.go with //go:build ignore. Each declared its own main(), so
`go build ./...` failed on redeclaration; it now passes repo-wide.
- Untrack scratch/node_modules (216 files) and ignore node_modules, test
artifacts, and the `doormile` binary `go build .` emits.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
71
utils/pagination.go
Normal file
71
utils/pagination.go
Normal file
@@ -0,0 +1,71 @@
|
||||
package utils
|
||||
|
||||
import (
|
||||
"math"
|
||||
|
||||
"github.com/gofiber/fiber/v2"
|
||||
"gorm.io/gorm"
|
||||
)
|
||||
|
||||
const (
|
||||
// DefaultPageSize bounds a list request that doesn't ask for a specific
|
||||
// window. It is deliberately high: these endpoints previously returned
|
||||
// whole tables, so a small default would silently truncate results for
|
||||
// consoles that don't paginate yet. It exists to stop a growing table
|
||||
// from being loaded into memory wholesale, not to enforce a page size.
|
||||
DefaultPageSize = 500
|
||||
|
||||
// MaxPageSize is the ceiling a caller can request, so a client can't opt
|
||||
// back into an unbounded scan by sending pagesize=999999.
|
||||
MaxPageSize = 1000
|
||||
)
|
||||
|
||||
// Page is a validated pagination window. Build one with ParsePage rather than
|
||||
// constructing it directly.
|
||||
type Page struct {
|
||||
No int // 1-based page number
|
||||
Size int
|
||||
Offset int
|
||||
}
|
||||
|
||||
// ParsePage reads ?pageno and ?pagesize off the request, matching the naming
|
||||
// GetAdminBookings already established. Both are clamped into a safe range;
|
||||
// absent or malformed values fall back to page 1 at DefaultPageSize rather
|
||||
// than erroring, so a bad query string degrades instead of failing the call.
|
||||
func ParsePage(c *fiber.Ctx) Page {
|
||||
no := c.QueryInt("pageno", 1)
|
||||
if no < 1 {
|
||||
no = 1
|
||||
}
|
||||
|
||||
size := c.QueryInt("pagesize", DefaultPageSize)
|
||||
if size < 1 {
|
||||
size = DefaultPageSize
|
||||
}
|
||||
if size > MaxPageSize {
|
||||
size = MaxPageSize
|
||||
}
|
||||
|
||||
return Page{No: no, Size: size, Offset: (no - 1) * size}
|
||||
}
|
||||
|
||||
// Apply scopes a query to this page's window.
|
||||
func (p Page) Apply(q *gorm.DB) *gorm.DB {
|
||||
return q.Offset(p.Offset).Limit(p.Size)
|
||||
}
|
||||
|
||||
// Paginated writes one page of a larger result set, using the same response
|
||||
// keys GetAdminBookings already returns. It keeps the {success, data, total}
|
||||
// shape utils.List emits and only adds keys, so clients that ignore the new
|
||||
// fields are unaffected. total is the count of all matching rows, not the
|
||||
// length of data.
|
||||
func Paginated(c *fiber.Ctx, data interface{}, total int64, p Page) error {
|
||||
return c.JSON(fiber.Map{
|
||||
"success": true,
|
||||
"data": data,
|
||||
"total": total,
|
||||
"pageno": p.No,
|
||||
"pagesize": p.Size,
|
||||
"pages": int(math.Ceil(float64(total) / float64(p.Size))),
|
||||
})
|
||||
}
|
||||
113
utils/pagination_test.go
Normal file
113
utils/pagination_test.go
Normal file
@@ -0,0 +1,113 @@
|
||||
package utils
|
||||
|
||||
import (
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/gofiber/fiber/v2"
|
||||
)
|
||||
|
||||
// pageFor spins a request through a throwaway Fiber app so ParsePage sees a
|
||||
// real *fiber.Ctx rather than a hand-built stub.
|
||||
func pageFor(t *testing.T, query string) Page {
|
||||
t.Helper()
|
||||
|
||||
var got Page
|
||||
app := fiber.New()
|
||||
app.Get("/", func(c *fiber.Ctx) error {
|
||||
got = ParsePage(c)
|
||||
return nil
|
||||
})
|
||||
|
||||
resp, err := app.Test(httptest.NewRequest("GET", "/?"+query, nil))
|
||||
if err != nil {
|
||||
t.Fatalf("test request failed: %v", err)
|
||||
}
|
||||
_ = resp.Body.Close()
|
||||
|
||||
return got
|
||||
}
|
||||
|
||||
func TestParsePageDefaults(t *testing.T) {
|
||||
p := pageFor(t, "")
|
||||
|
||||
if p.No != 1 {
|
||||
t.Errorf("No = %d, want 1", p.No)
|
||||
}
|
||||
if p.Size != DefaultPageSize {
|
||||
t.Errorf("Size = %d, want %d", p.Size, DefaultPageSize)
|
||||
}
|
||||
if p.Offset != 0 {
|
||||
t.Errorf("Offset = %d, want 0", p.Offset)
|
||||
}
|
||||
}
|
||||
|
||||
func TestParsePageClamping(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
query string
|
||||
wantNo int
|
||||
wantSize int
|
||||
wantOffset int
|
||||
}{
|
||||
{
|
||||
name: "explicit page and size", query: "pageno=3&pagesize=20",
|
||||
wantNo: 3, wantSize: 20, wantOffset: 40,
|
||||
},
|
||||
{
|
||||
// A client must not be able to opt back into an unbounded scan.
|
||||
name: "oversized pagesize is capped", query: "pagesize=999999",
|
||||
wantNo: 1, wantSize: MaxPageSize, wantOffset: 0,
|
||||
},
|
||||
{
|
||||
name: "zero page falls back to first", query: "pageno=0",
|
||||
wantNo: 1, wantSize: DefaultPageSize, wantOffset: 0,
|
||||
},
|
||||
{
|
||||
name: "negative page falls back to first", query: "pageno=-5",
|
||||
wantNo: 1, wantSize: DefaultPageSize, wantOffset: 0,
|
||||
},
|
||||
{
|
||||
name: "zero pagesize falls back to default", query: "pagesize=0",
|
||||
wantNo: 1, wantSize: DefaultPageSize, wantOffset: 0,
|
||||
},
|
||||
{
|
||||
name: "negative pagesize falls back to default", query: "pagesize=-10",
|
||||
wantNo: 1, wantSize: DefaultPageSize, wantOffset: 0,
|
||||
},
|
||||
{
|
||||
// Garbage must degrade to defaults, not fail the request.
|
||||
name: "non-numeric values fall back to defaults", query: "pageno=abc&pagesize=xyz",
|
||||
wantNo: 1, wantSize: DefaultPageSize, wantOffset: 0,
|
||||
},
|
||||
{
|
||||
name: "offset is derived from page and size", query: "pageno=5&pagesize=100",
|
||||
wantNo: 5, wantSize: 100, wantOffset: 400,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
p := pageFor(t, tc.query)
|
||||
|
||||
if p.No != tc.wantNo {
|
||||
t.Errorf("No = %d, want %d", p.No, tc.wantNo)
|
||||
}
|
||||
if p.Size != tc.wantSize {
|
||||
t.Errorf("Size = %d, want %d", p.Size, tc.wantSize)
|
||||
}
|
||||
if p.Offset != tc.wantOffset {
|
||||
t.Errorf("Offset = %d, want %d", p.Offset, tc.wantOffset)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestParsePageNeverExceedsMax(t *testing.T) {
|
||||
// Guards the invariant the cap exists for, independent of the table above.
|
||||
for _, q := range []string{"pagesize=1001", "pagesize=5000", "pagesize=2147483647"} {
|
||||
if p := pageFor(t, q); p.Size > MaxPageSize {
|
||||
t.Errorf("%s produced Size = %d, which exceeds MaxPageSize %d", q, p.Size, MaxPageSize)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user