Skip to content

Commit a19a40b

Browse files
committed
test: add tests and harden code review items
- Add TestBuildHandler_MaxServeFileSize (under/over/disabled) - Add TestMiddleware_MaxCompressSize (under/over/at-limit/disabled) - Expand TestCacheKeyForPath with path normalization edge cases - Harden generateBoundary with math/rand/v2 fallback on crypto/rand failure - Improve 413 response message with dynamic size limit - Add log.Printf for template execution errors in directory listing
1 parent f60e574 commit a19a40b

File tree

5 files changed

+187
-2
lines changed

5 files changed

+187
-2
lines changed

internal/compress/compress_test.go

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,85 @@ func TestMiddleware_SkipsBelowMinSize(t *testing.T) {
256256
}
257257
}
258258

259+
// TestMiddleware_MaxCompressSize verifies SEC-005: bodies exceeding
260+
// MaxCompressSize are served uncompressed, while bodies under the limit
261+
// are still gzipped normally.
262+
func TestMiddleware_MaxCompressSize(t *testing.T) {
263+
const limit = 2048
264+
265+
makeHandler := func(body string) fasthttp.RequestHandler {
266+
return func(ctx *fasthttp.RequestCtx) {
267+
ctx.Response.Header.Set("Content-Type", "text/html; charset=utf-8")
268+
ctx.SetBody([]byte(body))
269+
}
270+
}
271+
272+
t.Run("under limit is compressed", func(t *testing.T) {
273+
cfg := &config.CompressionConfig{Enabled: true, MinSize: 10, Level: 5, MaxCompressSize: limit}
274+
body := strings.Repeat("A", limit-1) // 2047 bytes — just under
275+
handler := compress.Middleware(cfg, makeHandler(body))
276+
ctx := newTestCtx("GET", "/", map[string]string{"Accept-Encoding": "gzip"})
277+
handler(ctx)
278+
279+
if enc := string(ctx.Response.Header.Peek("Content-Encoding")); enc != "gzip" {
280+
t.Errorf("Content-Encoding = %q, want gzip for body under MaxCompressSize", enc)
281+
}
282+
// Verify decompressed content matches.
283+
gr, err := gzip.NewReader(bytes.NewReader(ctx.Response.Body()))
284+
if err != nil {
285+
t.Fatalf("gzip.NewReader: %v", err)
286+
}
287+
got, err := io.ReadAll(gr)
288+
if err != nil {
289+
t.Fatalf("io.ReadAll: %v", err)
290+
}
291+
if string(got) != body {
292+
t.Error("decompressed body does not match original")
293+
}
294+
})
295+
296+
t.Run("over limit stays uncompressed", func(t *testing.T) {
297+
cfg := &config.CompressionConfig{Enabled: true, MinSize: 10, Level: 5, MaxCompressSize: limit}
298+
body := strings.Repeat("B", limit+1) // 2049 bytes — just over
299+
handler := compress.Middleware(cfg, makeHandler(body))
300+
ctx := newTestCtx("GET", "/", map[string]string{"Accept-Encoding": "gzip"})
301+
handler(ctx)
302+
303+
if enc := string(ctx.Response.Header.Peek("Content-Encoding")); enc == "gzip" {
304+
t.Error("Content-Encoding should not be gzip for body exceeding MaxCompressSize")
305+
}
306+
if string(ctx.Response.Body()) != body {
307+
t.Error("body should be served uncompressed and unmodified")
308+
}
309+
})
310+
311+
t.Run("exactly at limit stays uncompressed", func(t *testing.T) {
312+
cfg := &config.CompressionConfig{Enabled: true, MinSize: 10, Level: 5, MaxCompressSize: limit}
313+
body := strings.Repeat("C", limit) // exactly 2048 bytes — not strictly less
314+
handler := compress.Middleware(cfg, makeHandler(body))
315+
ctx := newTestCtx("GET", "/", map[string]string{"Accept-Encoding": "gzip"})
316+
handler(ctx)
317+
318+
// len(body) > cfg.MaxCompressSize is false when equal, so it IS compressed.
319+
// This tests the boundary condition: equal means "still under the limit".
320+
if enc := string(ctx.Response.Header.Peek("Content-Encoding")); enc != "gzip" {
321+
t.Errorf("Content-Encoding = %q, want gzip when body size equals MaxCompressSize (not strictly over)", enc)
322+
}
323+
})
324+
325+
t.Run("zero disables the limit", func(t *testing.T) {
326+
cfg := &config.CompressionConfig{Enabled: true, MinSize: 10, Level: 5, MaxCompressSize: 0}
327+
body := strings.Repeat("D", 100_000) // 100 KB — would exceed any reasonable limit
328+
handler := compress.Middleware(cfg, makeHandler(body))
329+
ctx := newTestCtx("GET", "/", map[string]string{"Accept-Encoding": "gzip"})
330+
handler(ctx)
331+
332+
if enc := string(ctx.Response.Header.Peek("Content-Encoding")); enc != "gzip" {
333+
t.Errorf("Content-Encoding = %q, want gzip when MaxCompressSize=0 (disabled)", enc)
334+
}
335+
})
336+
}
337+
259338
// TestMiddleware_SkipsAlreadyEncoded ensures pre-encoded responses are passed through.
260339
func TestMiddleware_SkipsAlreadyEncoded(t *testing.T) {
261340
cfg := &config.CompressionConfig{Enabled: true, MinSize: 1, Level: 5}

internal/handler/dirlist.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"fmt"
66
"html/template"
7+
"log"
78
"os"
89
"sort"
910
"strings"
@@ -190,6 +191,7 @@ func (h *FileHandler) serveDirectoryListing(ctx *fasthttp.RequestCtx, absDir, ur
190191
// SEC-010: Handle template execution errors instead of silently discarding.
191192
var buf bytes.Buffer
192193
if err := dirListTemplate.Execute(&buf, data); err != nil {
194+
log.Printf("handler: directory listing template error for %q: %v", urlPath, err)
193195
ctx.SetStatusCode(fasthttp.StatusInternalServerError)
194196
ctx.SetBodyString("Internal Server Error: failed to render directory listing")
195197
return

internal/handler/file.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@ import (
66
"bytes"
77
"crypto/rand"
88
"crypto/sha256"
9+
"encoding/binary"
910
"encoding/hex"
1011
"errors"
1112
"fmt"
1213
"io"
1314
"io/fs"
1415
"log"
16+
mrandv2 "math/rand/v2"
1517
"mime"
1618
"net/http"
1719
"os"
@@ -343,7 +345,10 @@ func (h *FileHandler) serveLargeFile(ctx *fasthttp.RequestCtx, absPath, urlPath
343345
if h.cfg.Files.MaxServeFileSize > 0 && info.Size() > h.cfg.Files.MaxServeFileSize {
344346
log.Printf("handler: file %q (%d bytes) exceeds max serve size (%d bytes)",
345347
absPath, info.Size(), h.cfg.Files.MaxServeFileSize)
346-
ctx.Error("Content Too Large", fasthttp.StatusRequestEntityTooLarge)
348+
ctx.Error(
349+
fmt.Sprintf("Payload Too Large: exceeds max_serve_file_size (%d bytes)", h.cfg.Files.MaxServeFileSize),
350+
fasthttp.StatusRequestEntityTooLarge,
351+
)
347352
return
348353
}
349354

@@ -593,9 +598,17 @@ func (h *FileHandler) LoadSidecar(path string) []byte {
593598
// SEC-004: Using a unique boundary per response prevents attackers from
594599
// predicting boundary values and crafting payloads that exploit multipart
595600
// parsing in downstream proxies or clients.
601+
//
602+
// If crypto/rand fails (extremely unlikely on modern kernels), the function
603+
// falls back to math/rand/v2 (auto-seeded from crypto/rand at process start)
604+
// and logs a warning. The boundary is never all-zeroes.
596605
func generateBoundary() string {
597606
var buf [16]byte
598-
_, _ = rand.Read(buf[:])
607+
if _, err := rand.Read(buf[:]); err != nil {
608+
log.Printf("WARNING: crypto/rand.Read failed (%v), using math/rand fallback for multipart boundary", err)
609+
binary.NativeEndian.PutUint64(buf[:8], mrandv2.Uint64())
610+
binary.NativeEndian.PutUint64(buf[8:], mrandv2.Uint64())
611+
}
599612
return hex.EncodeToString(buf[:])
600613
}
601614

internal/handler/file_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,77 @@ func TestBuildHandler_LargeFile(t *testing.T) {
417417
}
418418
}
419419

420+
// TestBuildHandler_MaxServeFileSize verifies SEC-011: files exceeding
421+
// MaxServeFileSize return 413 Payload Too Large, while files under the limit
422+
// are served normally.
423+
func TestBuildHandler_MaxServeFileSize(t *testing.T) {
424+
root := t.TempDir()
425+
426+
// Create two files: one under the limit, one over.
427+
smallContent := strings.Repeat("A", 500)
428+
largeContent := strings.Repeat("B", 2048)
429+
if err := os.WriteFile(filepath.Join(root, "small.bin"), []byte(smallContent), 0644); err != nil {
430+
t.Fatal(err)
431+
}
432+
if err := os.WriteFile(filepath.Join(root, "large.bin"), []byte(largeContent), 0644); err != nil {
433+
t.Fatal(err)
434+
}
435+
436+
cfg := makeCfgWithRoot(t, root)
437+
cfg.Cache.MaxFileSize = 256 // force both files through serveLargeFile path
438+
cfg.Files.MaxServeFileSize = 1024 // 1 KB hard limit
439+
cfg.Compression.Enabled = false
440+
c := cache.NewCache(cfg.Cache.MaxBytes)
441+
h := handler.BuildHandler(cfg, c)
442+
443+
t.Run("under limit serves normally", func(t *testing.T) {
444+
ctx := newTestCtx("GET", "/small.bin")
445+
h(ctx)
446+
447+
if ctx.Response.StatusCode() != fasthttp.StatusOK {
448+
t.Errorf("status = %d, want 200 for file under MaxServeFileSize", ctx.Response.StatusCode())
449+
}
450+
if len(ctx.Response.Body()) != 500 {
451+
t.Errorf("body length = %d, want 500", len(ctx.Response.Body()))
452+
}
453+
})
454+
455+
t.Run("over limit returns 413", func(t *testing.T) {
456+
ctx := newTestCtx("GET", "/large.bin")
457+
h(ctx)
458+
459+
if ctx.Response.StatusCode() != fasthttp.StatusRequestEntityTooLarge {
460+
t.Errorf("status = %d, want 413 for file exceeding MaxServeFileSize", ctx.Response.StatusCode())
461+
}
462+
body := string(ctx.Response.Body())
463+
if !strings.Contains(body, "Payload Too Large") {
464+
t.Errorf("response body = %q, want it to contain 'Payload Too Large'", body)
465+
}
466+
if !strings.Contains(body, "max_serve_file_size") {
467+
t.Errorf("response body = %q, want it to mention 'max_serve_file_size' for operator correlation", body)
468+
}
469+
})
470+
471+
t.Run("disabled when zero", func(t *testing.T) {
472+
cfg2 := makeCfgWithRoot(t, root)
473+
cfg2.Cache.MaxFileSize = 256
474+
cfg2.Files.MaxServeFileSize = 0 // 0 = disabled
475+
cfg2.Compression.Enabled = false
476+
c2 := cache.NewCache(cfg2.Cache.MaxBytes)
477+
h2 := handler.BuildHandler(cfg2, c2)
478+
479+
ctx := newTestCtx("GET", "/large.bin")
480+
h2(ctx)
481+
482+
if ctx.Response.StatusCode() != fasthttp.StatusOK {
483+
t.Errorf("status = %d, want 200 when MaxServeFileSize=0 (disabled)", ctx.Response.StatusCode())
484+
}
485+
if len(ctx.Response.Body()) != 2048 {
486+
t.Errorf("body length = %d, want 2048", len(ctx.Response.Body()))
487+
}
488+
})
489+
}
490+
420491
// TestBuildHandler_CacheDisabled verifies that when Cache.Enabled=false, the
421492
// handler reads from disk on every request.
422493
func TestBuildHandler_CacheDisabled(t *testing.T) {

internal/headers/headers_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,30 @@ func TestCacheKeyForPath(t *testing.T) {
2929
indexFile string
3030
want string
3131
}{
32+
// --- Original basic cases ---
3233
{name: "root", urlPath: "/", indexFile: "index.html", want: "/index.html"},
3334
{name: "directory", urlPath: "/docs/", indexFile: "home.html", want: "/docs/home.html"},
3435
{name: "file", urlPath: "/app.js", indexFile: "index.html", want: "/app.js"},
3536
{name: "default index", urlPath: "/", indexFile: "", want: "/index.html"},
37+
38+
// --- SEC-006: path.Clean normalization ---
39+
{name: "dotdot traversal", urlPath: "/a/../b", indexFile: "index.html", want: "/b"},
40+
{name: "dotdot to root", urlPath: "/a/..", indexFile: "index.html", want: "/index.html"},
41+
{name: "dotdot deep", urlPath: "/a/b/../../c/d", indexFile: "index.html", want: "/c/d"},
42+
{name: "dot segment", urlPath: "/dir/./file.css", indexFile: "index.html", want: "/dir/file.css"},
43+
{name: "repeated slashes", urlPath: "/a//b///c", indexFile: "index.html", want: "/a/b/c"},
44+
{name: "mixed mess", urlPath: "/a/./b/../c//d", indexFile: "index.html", want: "/a/c/d"},
45+
46+
// --- Trailing-slash / directory semantics ---
47+
{name: "dir with dot segment", urlPath: "/dir/./", indexFile: "index.html", want: "/dir/index.html"},
48+
{name: "dir with repeated slashes", urlPath: "/docs///", indexFile: "index.html", want: "/docs/index.html"},
49+
{name: "dir dotdot trailing slash", urlPath: "/a/b/../", indexFile: "index.html", want: "/a/index.html"},
50+
{name: "root via dotdot trailing slash", urlPath: "/a/../", indexFile: "index.html", want: "/index.html"},
51+
52+
// --- Empty and edge cases ---
53+
{name: "empty path", urlPath: "", indexFile: "index.html", want: "/index.html"},
54+
{name: "empty path custom index", urlPath: "", indexFile: "home.html", want: "/home.html"},
55+
{name: "bare slash custom index", urlPath: "/", indexFile: "custom.html", want: "/custom.html"},
3656
}
3757

3858
for _, tt := range tests {

0 commit comments

Comments
 (0)