From 841f53fac85d8f9385fa274c0af3ea32f6995a9d Mon Sep 17 00:00:00 2001 From: juliaweber Date: Wed, 17 Jun 2026 10:28:51 +0200 Subject: [PATCH] Fix code review findings: image read, whitespace validation, offset reset, cleanup error - Replace src.Read(data) with io.ReadAll(src) to prevent silent truncation on large uploads - Reject zero-byte file uploads with 400 rather than storing empty BYTEA - Add strings.TrimSpace to validateFruit so whitespace-only name/osdb_number returns 422 - Reset offset to 0 in fruitStore.create() so navigating back to list after create shows page 1 - Log t.Cleanup DELETE error in integration test to surface constraint failures clearly Co-Authored-By: Claude Sonnet 4.6 --- backend/internal/handler/fruit_handler.go | 14 ++++++++++---- .../repository/fruit_repo_integration_test.go | 4 +++- frontend/src/stores/fruitStore.ts | 1 + 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/backend/internal/handler/fruit_handler.go b/backend/internal/handler/fruit_handler.go index 0b6c37b..0240fdb 100644 --- a/backend/internal/handler/fruit_handler.go +++ b/backend/internal/handler/fruit_handler.go @@ -3,8 +3,10 @@ package handler import ( "context" "errors" + "io" "net/http" "strconv" + "strings" "github.com/labstack/echo/v4" @@ -60,10 +62,10 @@ func NewFruitHandler(repo FruitRepository) *FruitHandler { func validateFruit(dto domain.FruitWriteDTO) []string { var errs []string - if dto.Name == "" { + if strings.TrimSpace(dto.Name) == "" { errs = append(errs, "name is required") } - if dto.OSDBNumber == "" { + if strings.TrimSpace(dto.OSDBNumber) == "" { errs = append(errs, "osdb_number is required") } if _, ok := validFruitTypes[dto.FruitType]; !ok { @@ -220,8 +222,12 @@ func (h *FruitHandler) UploadImage(c echo.Context) error { } defer src.Close() - data := make([]byte, file.Size) - if _, err := src.Read(data); err != nil { + if file.Size == 0 { + return c.JSON(http.StatusBadRequest, map[string]string{"error": "image file is empty"}) + } + + data, err := io.ReadAll(src) + if err != nil { return c.JSON(http.StatusInternalServerError, map[string]string{"error": "internal server error"}) } diff --git a/backend/internal/repository/fruit_repo_integration_test.go b/backend/internal/repository/fruit_repo_integration_test.go index 3d25a9e..858e7ca 100644 --- a/backend/internal/repository/fruit_repo_integration_test.go +++ b/backend/internal/repository/fruit_repo_integration_test.go @@ -41,7 +41,9 @@ func TestFruitRepoIntegration(t *testing.T) { // cleanup after test t.Cleanup(func() { - pool.Exec(ctx, `DELETE FROM fruits WHERE osdb_number LIKE 'TEST-%'`) + if _, err := pool.Exec(ctx, `DELETE FROM fruits WHERE osdb_number LIKE 'TEST-%'`); err != nil { + t.Logf("cleanup: %v", err) + } }) // Create diff --git a/frontend/src/stores/fruitStore.ts b/frontend/src/stores/fruitStore.ts index de3a562..f23acde 100644 --- a/frontend/src/stores/fruitStore.ts +++ b/frontend/src/stores/fruitStore.ts @@ -52,6 +52,7 @@ export const useFruitStore = defineStore('fruit', () => { const fruit = await createFruit(dto) fruits.value = [fruit, ...fruits.value] total.value += 1 + offset.value = 0 return fruit }