From 734c7fbd89241133499a88674d5cf62de2ca1469 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Mon, 27 Apr 2026 08:16:22 +0300 Subject: fix(generator): write to temp file, check close error, rename on success Fixes data loss risk in writePage: - Write to a temp file instead of the target path. - Explicitly close the temp file and check the error (was ignored). - Rename to the final path only after successful close. - Remove the temp file on any error so truncated output is never left on disk. Added TestWritePage (happy path + template error) and TestWritePage_tempFileCleanedOnError to verify no corruption of existing file. --- internal/generator/generator.go | 25 +++++-- internal/generator/generator_test.go | 129 +++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 4 deletions(-) (limited to 'internal') diff --git a/internal/generator/generator.go b/internal/generator/generator.go index 079cc38..0b51edd 100644 --- a/internal/generator/generator.go +++ b/internal/generator/generator.go @@ -199,16 +199,33 @@ func writePage(tmpl *template.Template, posts []*post.Post, pageIndex, totalPage filename := pageFilename(pageIndex) path := filepath.Join(cfg.OutputDir, filename) - f, err := os.Create(path) + tmpFile, err := os.CreateTemp(cfg.OutputDir, filename+".*.tmp") if err != nil { - return fmt.Errorf("create %s: %w", filename, err) + return fmt.Errorf("create temp for %s: %w", filename, err) } - defer f.Close() + tmpPath := tmpFile.Name() - if err := tmpl.Execute(f, data); err != nil { + ok := false + defer func() { + _ = tmpFile.Close() + if !ok { + _ = os.Remove(tmpPath) + } + }() + + if err := tmpl.Execute(tmpFile, data); err != nil { return fmt.Errorf("render %s: %w", filename, err) } + if err := tmpFile.Close(); err != nil { + return fmt.Errorf("close temp %s: %w", filename, err) + } + + if err := os.Rename(tmpPath, path); err != nil { + return fmt.Errorf("rename %s: %w", filename, err) + } + + ok = true return nil } diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go index 47b4cd3..0e3708b 100644 --- a/internal/generator/generator_test.go +++ b/internal/generator/generator_test.go @@ -448,3 +448,132 @@ func TestRun_writesPagesAndAtom(t *testing.T) { t.Fatalf("index.html missing favicon link: %s", string(indexHTML)) } } + +func TestWritePage(t *testing.T) { + t.Parallel() + + meta, err := loadThemeMeta("neon") + if err != nil { + t.Fatalf("loadThemeMeta: %v", err) + } + all, err := allThemesJSON() + if err != nil { + t.Fatalf("allThemesJSON: %v", err) + } + + tests := []struct { + name string + posts []*post.Post + pageIndex int + totalPages int + baseURL string + wantErr bool + wantErrContains string + }{ + { + name: "happy path one page", + posts: []*post.Post{{ID: "a", Content: "

hi

"}}, + pageIndex: 0, + totalPages: 1, + baseURL: "https://example.test", + wantErr: false, + }, + { + name: "happy path second page", + posts: []*post.Post{{ID: "b", Content: "

bye

"}}, + pageIndex: 1, + totalPages: 2, + baseURL: "https://example.test", + wantErr: false, + }, + { + name: "invalid template action triggers error", + posts: []*post.Post{{ID: "x", Content: "

y

"}}, + pageIndex: 0, + totalPages: 1, + baseURL: "https://example.test", + wantErr: true, + wantErrContains: "render index.html", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + out := t.TempDir() + cfg := &config.Config{ + OutputDir: out, + BaseURL: tt.baseURL, + Theme: "neon", + } + + var tmpl *template.Template + if tt.wantErr { + tmpl = template.Must(template.New("page").Parse("{{.NonExistent.X}}")) + } else { + tmpl = template.Must(template.New("page").Parse("{{.DefaultTheme}}")) + } + + err := writePage(tmpl, tt.posts, tt.pageIndex, tt.totalPages, cfg, "neon", meta, all) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error, got nil") + } + if tt.wantErrContains != "" && !strings.Contains(err.Error(), tt.wantErrContains) { + t.Fatalf("error %q does not contain %q", err.Error(), tt.wantErrContains) + } + path := filepath.Join(out, pageFilename(tt.pageIndex)) + if _, statErr := os.Stat(path); !os.IsNotExist(statErr) { + t.Fatalf("expected %s to be absent after failed write, got statErr=%v", path, statErr) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + path := filepath.Join(out, pageFilename(tt.pageIndex)) + b, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + got := string(b) + if !strings.HasPrefix(got, "") { + t.Fatalf("expected HTML output, got %q", got) + } + }) + } +} + +func TestWritePage_tempFileCleanedOnError(t *testing.T) { + t.Parallel() + out := t.TempDir() + + path := filepath.Join(out, "index.html") + golden := "golden" + if err := os.WriteFile(path, []byte(golden), 0o644); err != nil { + t.Fatal(err) + } + + cfg := &config.Config{ + OutputDir: out, + BaseURL: "https://example.test", + Theme: "neon", + } + meta, _ := loadThemeMeta("neon") + all, _ := allThemesJSON() + tmpl := template.Must(template.New("page").Parse("{{.NonExistent.X}}")) + + err := writePage(tmpl, []*post.Post{{ID: "a", Content: "

x

"}}, 0, 1, cfg, "neon", meta, all) + if err == nil { + t.Fatal("expected error from broken template") + } + + b, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read existing file: %v", err) + } + if string(b) != golden { + t.Fatalf("existing file was corrupted: got %q, want %q", string(b), golden) + } +} -- cgit v1.2.3