From af9ffddf84d452a0181811e8cf6540a9dfc3d4ac Mon Sep 17 00:00:00 2001 From: user Date: Sun, 15 Feb 2026 22:04:09 -0800 Subject: [PATCH] fix: buffer template execution to prevent corrupt HTML responses (closes #42) Add renderTemplate helper method on Handlers that renders templates to a bytes.Buffer first, then writes to the ResponseWriter only on success. This prevents partial/corrupt HTML when template execution fails partway through. Applied to all template rendering call sites in: - setup.go (HandleSetupGET, renderSetupError) - auth.go (HandleLoginGET, HandleLoginPOST error paths) - dashboard.go (HandleDashboard) - app.go (HandleAppNew, HandleAppCreate, HandleAppDetail, HandleAppEdit, HandleAppUpdate, HandleAppDeployments) --- internal/handlers/app.go | 30 +++------- internal/handlers/auth.go | 12 ++-- internal/handlers/dashboard.go | 6 +- internal/handlers/handlers.go | 24 ++++++++ internal/handlers/render_template_test.go | 73 +++++++++++++++++++++++ internal/handlers/setup.go | 8 +-- 6 files changed, 111 insertions(+), 42 deletions(-) create mode 100644 internal/handlers/render_template_test.go diff --git a/internal/handlers/app.go b/internal/handlers/app.go index 5c634ca..6e2ac71 100644 --- a/internal/handlers/app.go +++ b/internal/handlers/app.go @@ -32,11 +32,7 @@ func (h *Handlers) HandleAppNew() http.HandlerFunc { return func(writer http.ResponseWriter, request *http.Request) { data := h.addGlobals(map[string]any{}, request) - err := tmpl.ExecuteTemplate(writer, "app_new.html", data) - if err != nil { - h.log.Error("template execution failed", "error", err) - http.Error(writer, "Internal Server Error", http.StatusInternalServerError) - } + h.renderTemplate(writer, tmpl, "app_new.html", data) } } @@ -66,7 +62,7 @@ func (h *Handlers) HandleAppCreate() http.HandlerFunc { if name == "" || repoURL == "" { data["Error"] = "Name and repository URL are required" - _ = tmpl.ExecuteTemplate(writer, "app_new.html", data) + h.renderTemplate(writer, tmpl, "app_new.html", data) return } @@ -91,7 +87,7 @@ func (h *Handlers) HandleAppCreate() http.HandlerFunc { if createErr != nil { h.log.Error("failed to create app", "error", createErr) data["Error"] = "Failed to create app: " + createErr.Error() - _ = tmpl.ExecuteTemplate(writer, "app_new.html", data) + h.renderTemplate(writer, tmpl, "app_new.html", data) return } @@ -152,11 +148,7 @@ func (h *Handlers) HandleAppDetail() http.HandlerFunc { "Success": request.URL.Query().Get("success"), }, request) - err := tmpl.ExecuteTemplate(writer, "app_detail.html", data) - if err != nil { - h.log.Error("template execution failed", "error", err) - http.Error(writer, "Internal Server Error", http.StatusInternalServerError) - } + h.renderTemplate(writer, tmpl, "app_detail.html", data) } } @@ -185,11 +177,7 @@ func (h *Handlers) HandleAppEdit() http.HandlerFunc { "App": application, }, request) - err := tmpl.ExecuteTemplate(writer, "app_edit.html", data) - if err != nil { - h.log.Error("template execution failed", "error", err) - http.Error(writer, "Internal Server Error", http.StatusInternalServerError) - } + h.renderTemplate(writer, tmpl, "app_edit.html", data) } } @@ -245,7 +233,7 @@ func (h *Handlers) HandleAppUpdate() http.HandlerFunc { "App": application, "Error": "Failed to update app", }, request) - _ = tmpl.ExecuteTemplate(writer, "app_edit.html", data) + h.renderTemplate(writer, tmpl, "app_edit.html", data) return } @@ -369,11 +357,7 @@ func (h *Handlers) HandleAppDeployments() http.HandlerFunc { "Deployments": deployments, }, request) - err := tmpl.ExecuteTemplate(writer, "deployments.html", data) - if err != nil { - h.log.Error("template execution failed", "error", err) - http.Error(writer, "Internal Server Error", http.StatusInternalServerError) - } + h.renderTemplate(writer, tmpl, "deployments.html", data) } } diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go index b88e81c..5a719cb 100644 --- a/internal/handlers/auth.go +++ b/internal/handlers/auth.go @@ -13,11 +13,7 @@ func (h *Handlers) HandleLoginGET() http.HandlerFunc { return func(writer http.ResponseWriter, request *http.Request) { data := h.addGlobals(map[string]any{}, request) - err := tmpl.ExecuteTemplate(writer, "login.html", data) - if err != nil { - h.log.Error("template execution failed", "error", err) - http.Error(writer, "Internal Server Error", http.StatusInternalServerError) - } + h.renderTemplate(writer, tmpl, "login.html", data) } } @@ -42,7 +38,7 @@ func (h *Handlers) HandleLoginPOST() http.HandlerFunc { if username == "" || password == "" { data["Error"] = "Username and password are required" - _ = tmpl.ExecuteTemplate(writer, "login.html", data) + h.renderTemplate(writer, tmpl, "login.html", data) return } @@ -50,7 +46,7 @@ func (h *Handlers) HandleLoginPOST() http.HandlerFunc { user, authErr := h.auth.Authenticate(request.Context(), username, password) if authErr != nil { data["Error"] = "Invalid username or password" - _ = tmpl.ExecuteTemplate(writer, "login.html", data) + h.renderTemplate(writer, tmpl, "login.html", data) return } @@ -60,7 +56,7 @@ func (h *Handlers) HandleLoginPOST() http.HandlerFunc { h.log.Error("failed to create session", "error", sessionErr) data["Error"] = "Failed to create session" - _ = tmpl.ExecuteTemplate(writer, "login.html", data) + h.renderTemplate(writer, tmpl, "login.html", data) return } diff --git a/internal/handlers/dashboard.go b/internal/handlers/dashboard.go index a85c4da..caadf06 100644 --- a/internal/handlers/dashboard.go +++ b/internal/handlers/dashboard.go @@ -69,10 +69,6 @@ func (h *Handlers) HandleDashboard() http.HandlerFunc { "AppStats": appStats, }, request) - execErr := tmpl.ExecuteTemplate(writer, "dashboard.html", data) - if execErr != nil { - h.log.Error("template execution failed", "error", execErr) - http.Error(writer, "Internal Server Error", http.StatusInternalServerError) - } + h.renderTemplate(writer, tmpl, "dashboard.html", data) } } diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 40a890a..9146fd0 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -2,6 +2,7 @@ package handlers import ( + "bytes" "encoding/json" "log/slog" "net/http" @@ -18,6 +19,7 @@ import ( "git.eeqj.de/sneak/upaas/internal/service/auth" "git.eeqj.de/sneak/upaas/internal/service/deploy" "git.eeqj.de/sneak/upaas/internal/service/webhook" + "git.eeqj.de/sneak/upaas/templates" ) // Params contains dependencies for Handlers. @@ -80,6 +82,28 @@ func (h *Handlers) addGlobals( return data } +// renderTemplate executes the named template into a buffer first, then writes +// to the ResponseWriter only on success. This prevents partial/corrupt HTML +// responses when template execution fails partway through. +func (h *Handlers) renderTemplate( + writer http.ResponseWriter, + tmpl *templates.TemplateExecutor, + name string, + data any, +) { + var buf bytes.Buffer + + err := tmpl.ExecuteTemplate(&buf, name, data) + if err != nil { + h.log.Error("template execution failed", "error", err) + http.Error(writer, "Internal Server Error", http.StatusInternalServerError) + + return + } + + _, _ = buf.WriteTo(writer) +} + func (h *Handlers) respondJSON( writer http.ResponseWriter, _ *http.Request, diff --git a/internal/handlers/render_template_test.go b/internal/handlers/render_template_test.go new file mode 100644 index 0000000..98b8fc0 --- /dev/null +++ b/internal/handlers/render_template_test.go @@ -0,0 +1,73 @@ +package handlers_test + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestRenderTemplateBuffersOutput verifies that successful template rendering +// produces a complete HTML response (not partial/corrupt). +func TestRenderTemplateBuffersOutput(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + + // The setup page is simple and has no DB dependencies + request := httptest.NewRequest(http.MethodGet, "/setup", nil) + recorder := httptest.NewRecorder() + + handler := testCtx.handlers.HandleSetupGET() + handler.ServeHTTP(recorder, request) + + assert.Equal(t, http.StatusOK, recorder.Code) + + body := recorder.Body.String() + // A properly buffered response should contain the closing tag, + // proving the full template was rendered before being sent. + assert.Contains(t, body, "") + // Should NOT contain the error text that would be appended on failure + assert.NotContains(t, body, "Internal Server Error") +} + +// TestDashboardRenderTemplateBuffersOutput verifies the dashboard handler +// also uses buffered template rendering. +func TestDashboardRenderTemplateBuffersOutput(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + + request := httptest.NewRequest(http.MethodGet, "/", nil) + recorder := httptest.NewRecorder() + + handler := testCtx.handlers.HandleDashboard() + handler.ServeHTTP(recorder, request) + + assert.Equal(t, http.StatusOK, recorder.Code) + + body := recorder.Body.String() + assert.Contains(t, body, "") + assert.NotContains(t, body, "Internal Server Error") +} + +// TestLoginRenderTemplateBuffersOutput verifies the login handler +// uses buffered template rendering. +func TestLoginRenderTemplateBuffersOutput(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + + request := httptest.NewRequest(http.MethodGet, "/login", nil) + recorder := httptest.NewRecorder() + + handler := testCtx.handlers.HandleLoginGET() + handler.ServeHTTP(recorder, request) + + assert.Equal(t, http.StatusOK, recorder.Code) + + body := recorder.Body.String() + assert.Contains(t, body, "") + assert.NotContains(t, body, "Internal Server Error") +} diff --git a/internal/handlers/setup.go b/internal/handlers/setup.go index fd7805f..f7e9519 100644 --- a/internal/handlers/setup.go +++ b/internal/handlers/setup.go @@ -18,11 +18,7 @@ func (h *Handlers) HandleSetupGET() http.HandlerFunc { return func(writer http.ResponseWriter, request *http.Request) { data := h.addGlobals(map[string]any{}, request) - err := tmpl.ExecuteTemplate(writer, "setup.html", data) - if err != nil { - h.log.Error("template execution failed", "error", err) - http.Error(writer, "Internal Server Error", http.StatusInternalServerError) - } + h.renderTemplate(writer, tmpl, "setup.html", data) } } @@ -62,7 +58,7 @@ func (h *Handlers) renderSetupError( "Username": username, "Error": errorMsg, }, request) - _ = tmpl.ExecuteTemplate(writer, "setup.html", data) + h.renderTemplate(writer, tmpl, "setup.html", data) } // HandleSetupPOST handles the setup form submission.