fix: critical bugs from code review (data corruption, error contract, HTTP hardening) #5

Merged
frx merged 7 commits from refactor/code-review-fixes into master 2026-06-28 14:04:25 +00:00
7 changed files with 82 additions and 28 deletions
Showing only changes of commit 4f89f59232 - Show all commits

View File

@ -1,6 +1,7 @@
package main
import (
"fmt"
"log/slog"
"net/http"
"strings"
@ -54,6 +55,7 @@ func setupHTTP(cfg config.HTTP, srv xhttp.Server, log *slog.Logger) *http.Server
router := mux.NewRouter()
router.Use(
middlewareRecovery(log),
middlewareCustomWriterInjector(),
mux.CORSMethodMiddleware(router),
middlewareLogger(log),
@ -101,6 +103,29 @@ func setupHTTP(cfg config.HTTP, srv xhttp.Server, log *slog.Logger) *http.Server
}
}
func middlewareRecovery(log *slog.Logger) mux.MiddlewareFunc {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
defer func() {
rec := recover()
if rec == nil {
return
}
if rec == http.ErrAbortHandler {
panic(rec)
}
xcontext.LogWithError(
r.Context(), log, fmt.Errorf("%v", rec), "recovered from panic",
slog.String("method", r.Method),
slog.String("path", r.URL.Path),
)
http.Error(w, "internal server error", http.StatusInternalServerError)
}()
next.ServeHTTP(w, r)
})
}
}
func middlewareCustomWriterInjector() mux.MiddlewareFunc {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

View File

@ -213,6 +213,15 @@ func (c courseTemplServer) List(w http.ResponseWriter, r *http.Request) {
}
})
totalPages := 0
if pathParams.PerPage > 0 {
totalPages = listCoursesResult.Count / pathParams.PerPage
}
currentPage := pathParams.Page
if currentPage > 0 && totalPages > 0 && currentPage > totalPages {
currentPage = totalPages
}
params = bootstrap.ListCoursesParams{
FilterForm: bootstrap.FilterFormParams{
Render: true,
@ -233,8 +242,8 @@ func (c courseTemplServer) List(w http.ResponseWriter, r *http.Request) {
Courses: params.Courses,
Categories: params.Categories,
Pagination: bootstrap.Pagination{
Page: pathParams.Page,
TotalPages: listCoursesResult.Count / pathParams.PerPage,
Page: currentPage,
TotalPages: totalPages,
BaseURL: r.URL.Path,
},
}
@ -285,7 +294,10 @@ func (c courseTemplServer) Index(w http.ResponseWriter, r *http.Request) {
stats := bootstrap.MakeNewStats(1, 2, 3)
coursesResult, err := c.app.Queries.ListCourses.Handle(ctx, query.ListCourse{})
const indexCoursesLimit = 200
coursesResult, err := c.app.Queries.ListCourses.Handle(ctx, query.ListCourse{
Limit: indexCoursesLimit,
})
if handleError(ctx, err, w, c.log, "unable to list courses") {
return
}

View File

@ -88,14 +88,22 @@ func parsePaginationFromQuery(r *http.Request) (out pagination, err error) {
} else {
out.PerPage = 20
}
if out.PerPage < 1 {
out.PerPage = 1
} else if out.PerPage > 100 {
out.PerPage = 100
}
if query.Has("page") {
out.Page, err = strconv.Atoi(query.Get("page"))
if err != nil {
return out, errors.NewValidationError("page", "bad per_page value")
return out, errors.NewValidationError("page", "bad page value")
}
} else if !query.Has("next") {
out.Page = 1
}
if out.Page < 1 && !query.Has("next") {
out.Page = 1
}
return out, nil
}