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)
This commit is contained in:
parent
e9bf63d18b
commit
af9ffddf84
@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -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
|
||||
}
|
||||
|
||||
@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@ -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,
|
||||
|
||||
73
internal/handlers/render_template_test.go
Normal file
73
internal/handlers/render_template_test.go
Normal file
@ -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 </html> tag,
|
||||
// proving the full template was rendered before being sent.
|
||||
assert.Contains(t, body, "</html>")
|
||||
// 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, "</html>")
|
||||
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, "</html>")
|
||||
assert.NotContains(t, body, "Internal Server Error")
|
||||
}
|
||||
@ -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.
|
||||
|
||||
Loading…
Reference in New Issue
Block a user