From c1c41c581d20af88b52824cb859449df896b70d8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 28 Mar 2026 04:49:05 +0000 Subject: [PATCH 1/6] Implement LTML multipart render server (serve-ltml) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add SetAssetFS/AssetFS to ltml.Scope with per-scope storage and parent inheritance, so concurrent documents can carry different asset filesystems without sharing global state. - Add per-document rootScope to ltml.Doc; Doc.SetAssetFS sets the FS on this root scope, which all parsed element scopes inherit via the parent chain. - Update ltml.HasScope interface to include SetAssetFS and AssetFS. - Extend StdImage to resolve image bytes via the scope's asset filesystem when one is set and the Writer implements the data-based image methods; falls back to the file-path Writer methods for backward compatibility. - Add github.com/namsral/flag dependency for env-variable-aware flag parsing. - Implement ltml/server (package main): - config.go: Config struct + parseConfig using namsral/flag - overlayfs.go: per-request overlay FS (upper=uploads, lower=base-path) - render.go: LTML parse → SetAssetFS → ltpdf render → temp PDF - handler.go: multipart streaming, upload validation, lifecycle cleanup - main.go: http.Server wiring and startup - Add tests covering handler behaviour, overlay FS semantics, LTML FS threading, and request-temp-dir cleanup. https://claude.ai/code/session_01Fr7DT5rkqVCraSSSEFrWbV --- go.mod | 5 +- go.sum | 2 + ltml/asset_fs_test.go | 172 +++++++++++++++++++ ltml/has_scope.go | 4 + ltml/ltml.go | 23 ++- ltml/scope.go | 21 +++ ltml/server/config.go | 62 +++++++ ltml/server/handler.go | 249 +++++++++++++++++++++++++++ ltml/server/handler_test.go | 312 ++++++++++++++++++++++++++++++++++ ltml/server/main.go | 57 +++++++ ltml/server/overlayfs.go | 106 ++++++++++++ ltml/server/overlayfs_test.go | 109 ++++++++++++ ltml/server/render.go | 50 ++++++ ltml/std_image.go | 60 ++++++- 14 files changed, 1224 insertions(+), 8 deletions(-) create mode 100644 ltml/asset_fs_test.go create mode 100644 ltml/server/config.go create mode 100644 ltml/server/handler.go create mode 100644 ltml/server/handler_test.go create mode 100644 ltml/server/main.go create mode 100644 ltml/server/overlayfs.go create mode 100644 ltml/server/overlayfs_test.go create mode 100644 ltml/server/render.go diff --git a/go.mod b/go.mod index 460b3d0..c9eee9f 100644 --- a/go.mod +++ b/go.mod @@ -7,4 +7,7 @@ require ( golang.org/x/image v0.23.0 ) -require golang.org/x/text v0.21.0 // indirect +require ( + github.com/namsral/flag v1.7.4-pre // indirect + golang.org/x/text v0.21.0 // indirect +) diff --git a/go.sum b/go.sum index fd4da67..207e38d 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,7 @@ github.com/go-text/typesetting v0.3.4 h1:YYurUOtEb9kGSOz4uE3k4OpBGsp1dDL8+fjCeaFamAU= github.com/go-text/typesetting v0.3.4/go.mod h1:4qZCQphq4KSgGTAeI0uMEkVbROgfah8BuyF5LRYr7XY= +github.com/namsral/flag v1.7.4-pre h1:b2ScHhoCUkbsq0d2C15Mv+VU8bl8hAXV8arnWiOHNZs= +github.com/namsral/flag v1.7.4-pre/go.mod h1:OXldTctbM6SWH1K899kPZcf65KxJiD7MsceFUpB5yDo= golang.org/x/image v0.23.0 h1:HseQ7c2OpPKTPVzNjG5fwJsOTCiiwS4QdsYi5XU6H68= golang.org/x/image v0.23.0/go.mod h1:wJJBTdLfCCf3tiHa1fNxpZmUI4mmoZvwMCPP0ddoNKY= golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo= diff --git a/ltml/asset_fs_test.go b/ltml/asset_fs_test.go new file mode 100644 index 0000000..c924289 --- /dev/null +++ b/ltml/asset_fs_test.go @@ -0,0 +1,172 @@ +// Copyright 2026 Brent Rowland. +// Use of this source code is governed by the Apache License, Version 2.0, as described in the LICENSE file. + +package ltml + +import ( + "io/fs" + "testing" + "testing/fstest" +) + +// fakeFS is a simple in-memory filesystem for testing asset FS threading. +func fakeFS(files map[string][]byte) fs.FS { + m := make(fstest.MapFS) + for name, data := range files { + m[name] = &fstest.MapFile{Data: data} + } + return m +} + +// TestDoc_SetAssetFS_IsDocumentLocal verifies that each document carries its +// own asset filesystem and that one document's FS does not affect another. +func TestDoc_SetAssetFS_IsDocumentLocal(t *testing.T) { + fsA := fakeFS(map[string][]byte{"logo.png": []byte("A")}) + fsB := fakeFS(map[string][]byte{"logo.png": []byte("B")}) + + docA, err := Parse([]byte("")) + if err != nil { + t.Fatal(err) + } + docB, err := Parse([]byte("")) + if err != nil { + t.Fatal(err) + } + + docA.SetAssetFS(fsA) + docB.SetAssetFS(fsB) + + // Doc A's scope should see fsA. + gotA := docA.scope().AssetFS() + if gotA == nil { + t.Fatal("docA assetFS is nil") + } + dataA, err := fs.ReadFile(gotA, "logo.png") + if err != nil { + t.Fatalf("docA: reading logo.png: %v", err) + } + if string(dataA) != "A" { + t.Errorf("docA logo.png = %q, want \"A\"", dataA) + } + + // Doc B's scope should see fsB. + gotB := docB.scope().AssetFS() + if gotB == nil { + t.Fatal("docB assetFS is nil") + } + dataB, err := fs.ReadFile(gotB, "logo.png") + if err != nil { + t.Fatalf("docB: reading logo.png: %v", err) + } + if string(dataB) != "B" { + t.Errorf("docB logo.png = %q, want \"B\"", dataB) + } +} + +// TestScope_AssetFS_InheritedByNestedScope verifies that a nested scope +// inherits the asset filesystem from the nearest ancestor that has one set. +func TestScope_AssetFS_InheritedByNestedScope(t *testing.T) { + fsys := fakeFS(map[string][]byte{"img.png": []byte("root")}) + + root := &Scope{} + root.SetAssetFS(fsys) + + child := &Scope{} + child.SetParentScope(root) + + grandchild := &Scope{} + grandchild.SetParentScope(child) + + // Verify inheritance by reading a file through the inherited FS. + got := grandchild.AssetFS() + if got == nil { + t.Fatal("grandchild AssetFS() is nil; expected to inherit from root") + } + data, err := fs.ReadFile(got, "img.png") + if err != nil { + t.Fatalf("reading via inherited FS: %v", err) + } + if string(data) != "root" { + t.Errorf("data = %q, want \"root\"", data) + } +} + +// TestScope_AssetFS_NilWhenNotSet verifies that AssetFS returns nil when no +// ancestor has an asset filesystem configured. +func TestScope_AssetFS_NilWhenNotSet(t *testing.T) { + root := &Scope{} + child := &Scope{} + child.SetParentScope(root) + + if child.AssetFS() != nil { + t.Error("expected nil assetFS when none is set") + } +} + +// dataWriterSpy records calls to PrintImage and ImageDimensions so tests can +// verify that the FS-based code path is taken. +type dataWriterSpy struct { + imageTestWriter + imageDimensionsCalls int + printImageCalls int + lastData []byte +} + +func (s *dataWriterSpy) ImageDimensions(data []byte) (width, height int, err error) { + s.imageDimensionsCalls++ + s.lastData = data + return 100, 50, nil +} + +func (s *dataWriterSpy) PrintImage(data []byte, x, y float64, width, height *float64) (actualWidth, actualHeight float64, err error) { + s.printImageCalls++ + s.lastData = data + return 0, 0, nil +} + +// TestStdImage_DrawContent_UsesAssetFS verifies that when a scope has an asset +// filesystem and the writer supports data-based image methods, DrawContent +// resolves the image via the FS rather than via PrintImageFile. +func TestStdImage_DrawContent_UsesAssetFS(t *testing.T) { + pngBytes := []byte("fake-png-data") + fsys := fakeFS(map[string][]byte{"logo.png": pngBytes}) + + scope := &Scope{} + scope.SetAssetFS(fsys) + + img := &StdImage{src: "logo.png"} + img.SetScope(scope) + + spy := &dataWriterSpy{} + if err := img.DrawContent(spy); err != nil { + t.Fatalf("DrawContent: %v", err) + } + + if spy.printImageCalls != 1 { + t.Errorf("PrintImage calls = %d, want 1", spy.printImageCalls) + } + if len(spy.calls) != 0 { + t.Errorf("PrintImageFile unexpectedly called %d time(s)", len(spy.calls)) + } + if string(spy.lastData) != string(pngBytes) { + t.Errorf("data passed to PrintImage = %q, want %q", spy.lastData, pngBytes) + } +} + +// TestStdImage_DrawContent_FallsBackToFilePath verifies that when no asset +// filesystem is set, DrawContent uses the Writer's file-path–based method. +func TestStdImage_DrawContent_FallsBackToFilePath(t *testing.T) { + img := &StdImage{src: "fixture.jpg"} + w := &imageTestWriter{} + + if err := img.DrawContent(w); err != nil { + t.Fatalf("DrawContent: %v", err) + } + + if len(w.calls) != 1 { + t.Fatalf("PrintImageFile calls = %d, want 1", len(w.calls)) + } + if w.calls[0].filename != "fixture.jpg" { + t.Errorf("filename = %q, want fixture.jpg", w.calls[0].filename) + } +} diff --git a/ltml/has_scope.go b/ltml/has_scope.go index 32fe7ff..7194b79 100644 --- a/ltml/has_scope.go +++ b/ltml/has_scope.go @@ -3,6 +3,8 @@ package ltml +import "io/fs" + type HasScope interface { AddAlias(alias *Alias) error AddLayout(layout *LayoutStyle) error @@ -10,9 +12,11 @@ type HasScope interface { AddRules(rules *Rules) error AddStyle(style Styler) error AliasFor(name string) (alias *Alias, ok bool) + AssetFS() fs.FS EachRuleFor(path string, f func(rule *Rule)) LayoutFor(id string) (layout *LayoutStyle, ok bool) PageStyleFor(id string) (style *PageStyle, ok bool) + SetAssetFS(fsys fs.FS) SetParentScope(parent HasScope) StyleFor(id string) (style Styler, ok bool) } diff --git a/ltml/ltml.go b/ltml/ltml.go index 6266aab..23af3a0 100644 --- a/ltml/ltml.go +++ b/ltml/ltml.go @@ -8,14 +8,16 @@ import ( "encoding/xml" "fmt" "io" + "io/fs" "os" "strings" ) type Doc struct { - ltmls []*StdDocument - stack []interface{} - scopes []HasScope + ltmls []*StdDocument + stack []interface{} + scopes []HasScope + rootScope Scope // per-document root scope; parent = &defaultScope } func (doc *Doc) Parse(b []byte) error { @@ -222,11 +224,24 @@ func (doc *Doc) current() (value interface{}) { return } +// SetAssetFS attaches an asset filesystem to this document's root scope. +// All nested scopes that do not have their own asset filesystem will inherit +// it, so assets looked up during rendering are resolved against fsys first +// and fall through to whatever the renderer's default resolution is. +func (doc *Doc) SetAssetFS(fsys fs.FS) { + doc.rootScope.SetAssetFS(fsys) +} + func (doc *Doc) scope() HasScope { if len(doc.scopes) > 0 { return doc.scopes[len(doc.scopes)-1] } - return &defaultScope + // Return the per-document root scope, which inherits from defaultScope. + // This ensures concurrent documents can carry different asset filesystems. + if doc.rootScope.parent == nil { + doc.rootScope.SetParentScope(&defaultScope) + } + return &doc.rootScope } func Parse(b []byte) (*Doc, error) { diff --git a/ltml/scope.go b/ltml/scope.go index 6a26906..d8e38a8 100644 --- a/ltml/scope.go +++ b/ltml/scope.go @@ -6,6 +6,7 @@ package ltml import ( "bytes" "fmt" + "io/fs" ) type Scope struct { @@ -15,6 +16,7 @@ type Scope struct { layouts map[string]*LayoutStyle pageStyles map[string]*PageStyle rules []*Rules + assetFS fs.FS } func (scope *Scope) AddAlias(alias *Alias) error { @@ -114,6 +116,25 @@ func (scope *Scope) SetParentScope(parent HasScope) { scope.parent = parent } +// SetAssetFS stores the asset filesystem for this scope. Nested scopes that +// do not have their own asset filesystem inherit it from the nearest ancestor +// that has one set. +func (scope *Scope) SetAssetFS(fsys fs.FS) { + scope.assetFS = fsys +} + +// AssetFS returns the nearest asset filesystem in scope, walking up the parent +// chain until one is found. Returns nil if no ancestor has an asset filesystem. +func (scope *Scope) AssetFS() fs.FS { + if scope.assetFS != nil { + return scope.assetFS + } + if scope.parent != nil { + return scope.parent.AssetFS() + } + return nil +} + func (scope *Scope) String() string { var buf bytes.Buffer for name, alias := range scope.aliases { diff --git a/ltml/server/config.go b/ltml/server/config.go new file mode 100644 index 0000000..9383455 --- /dev/null +++ b/ltml/server/config.go @@ -0,0 +1,62 @@ +// Copyright 2026 Brent Rowland. +// Use of this source code is governed by the Apache License, Version 2.0, as described in the LICENSE file. + +package main + +import ( + "fmt" + "os" + "time" + + flag "github.com/namsral/flag" +) + +// Config holds validated runtime configuration for the LTML render server. +type Config struct { + Listen string + BasePath string + MaxUploadBytes int64 + ReadTimeout time.Duration + WriteTimeout time.Duration +} + +// parseConfig reads flags and environment variables via namsral/flag and +// returns a validated Config. It calls flag.Parse internally. If +// configuration is invalid, parseConfig returns a non-nil error describing +// the problem; the caller should print it and exit. +func parseConfig() (*Config, error) { + var ( + listen string + basePath string + maxUploadBytes int64 + readTimeout time.Duration + writeTimeout time.Duration + ) + + flag.StringVar(&listen, "listen", ":8080", "address to listen on (LISTEN)") + flag.StringVar(&basePath, "base-path", "", "path to static asset directory (BASE_PATH, required)") + flag.Int64Var(&maxUploadBytes, "max-upload-bytes", 32<<20, "maximum multipart request size in bytes (MAX_UPLOAD_BYTES)") + flag.DurationVar(&readTimeout, "read-timeout", 0, "HTTP server read timeout, e.g. 30s (READ_TIMEOUT)") + flag.DurationVar(&writeTimeout, "write-timeout", 0, "HTTP server write timeout, e.g. 60s (WRITE_TIMEOUT)") + + flag.Parse() + + if basePath == "" { + return nil, fmt.Errorf("base-path (or BASE_PATH) is required") + } + info, err := os.Stat(basePath) + if err != nil { + return nil, fmt.Errorf("base-path %q: %w", basePath, err) + } + if !info.IsDir() { + return nil, fmt.Errorf("base-path %q is not a directory", basePath) + } + + return &Config{ + Listen: listen, + BasePath: basePath, + MaxUploadBytes: maxUploadBytes, + ReadTimeout: readTimeout, + WriteTimeout: writeTimeout, + }, nil +} diff --git a/ltml/server/handler.go b/ltml/server/handler.go new file mode 100644 index 0000000..0101126 --- /dev/null +++ b/ltml/server/handler.go @@ -0,0 +1,249 @@ +// Copyright 2026 Brent Rowland. +// Use of this source code is governed by the Apache License, Version 2.0, as described in the LICENSE file. + +package main + +import ( + "errors" + "fmt" + "io" + "log" + "mime" + "mime/multipart" + "net/http" + "os" + "path" + "path/filepath" + "strings" +) + +// ltmlContentTypes lists the accepted Content-Type values for the LTML part. +// The first is the preferred private media type; the others are compatibility +// fallbacks (and an empty value, meaning the field was omitted). +var ltmlContentTypes = map[string]bool{ + "application/vnd.rowland.leadtype.ltml+xml": true, + "application/xml": true, + "text/xml": true, + "": true, +} + +// renderHandler is an http.Handler for POST /render. +type renderHandler struct { + cfg *Config +} + +func newRenderHandler(cfg *Config) *renderHandler { + return &renderHandler{cfg: cfg} +} + +func (h *renderHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + + r.Body = http.MaxBytesReader(w, r.Body, h.cfg.MaxUploadBytes) + + mediaType, params, err := mime.ParseMediaType(r.Header.Get("Content-Type")) + if err != nil || !strings.HasPrefix(mediaType, "multipart/") { + http.Error(w, "Content-Type must be multipart/form-data", http.StatusBadRequest) + return + } + boundary := params["boundary"] + if boundary == "" { + http.Error(w, "missing multipart boundary", http.StatusBadRequest) + return + } + + // Create the request-scoped temp directory. Everything lives here; a + // deferred RemoveAll cleans it up regardless of how the request ends. + tmpDir, err := os.MkdirTemp("", "serve-ltml-*") + if err != nil { + log.Printf("serve-ltml: creating temp dir: %v", err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + defer os.RemoveAll(tmpDir) + + uploadDir := filepath.Join(tmpDir, "uploads") + if err := os.Mkdir(uploadDir, 0o700); err != nil { + log.Printf("serve-ltml: creating upload dir: %v", err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + + mr := multipart.NewReader(r.Body, boundary) + + // --- First part: LTML document --- + firstPart, err := mr.NextPart() + if err != nil { + if isMaxBytesError(err) { + http.Error(w, "request too large", http.StatusRequestEntityTooLarge) + } else { + http.Error(w, "missing LTML part", http.StatusBadRequest) + } + return + } + + if firstPart.FormName() != "ltml" { + firstPart.Close() + http.Error(w, `first multipart part must use field name "ltml"`, http.StatusBadRequest) + return + } + + partCT := firstPart.Header.Get("Content-Type") + partMediaType := "" + if partCT != "" { + partMediaType, _, _ = mime.ParseMediaType(partCT) + } + if !ltmlContentTypes[partMediaType] { + firstPart.Close() + http.Error(w, fmt.Sprintf("unsupported LTML part content type: %q", partCT), http.StatusBadRequest) + return + } + + ltmlBytes, err := io.ReadAll(firstPart) + firstPart.Close() + if err != nil { + if isMaxBytesError(err) { + http.Error(w, "request too large", http.StatusRequestEntityTooLarge) + } else { + log.Printf("serve-ltml: reading LTML part: %v", err) + http.Error(w, "error reading LTML", http.StatusBadRequest) + } + return + } + + if len(ltmlBytes) == 0 { + http.Error(w, "LTML part is empty", http.StatusBadRequest) + return + } + + // --- Subsequent parts: uploaded asset files --- + for { + part, err := mr.NextPart() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + if isMaxBytesError(err) { + http.Error(w, "request too large", http.StatusRequestEntityTooLarge) + } else { + log.Printf("serve-ltml: reading multipart: %v", err) + http.Error(w, "bad multipart request", http.StatusBadRequest) + } + return + } + + if part.FormName() != "file" { + part.Close() + http.Error(w, `uploaded file parts must use field name "file"`, http.StatusBadRequest) + return + } + + // Read the raw filename directly from the Content-Disposition header + // rather than using part.FileName(), which calls filepath.Base and + // would strip path components like "assets/" from "assets/logo.png". + filename := rawFilename(part.Header.Get("Content-Disposition")) + destPath, validErr := validateUploadFilename(filename, uploadDir) + if validErr != nil { + part.Close() + http.Error(w, fmt.Sprintf("invalid filename: %v", validErr), http.StatusBadRequest) + return + } + + if err := saveUploadedFile(part, destPath); err != nil { + part.Close() + if isMaxBytesError(err) { + http.Error(w, "request too large", http.StatusRequestEntityTooLarge) + } else { + log.Printf("serve-ltml: saving upload %q: %v", filename, err) + http.Error(w, "error storing uploaded file", http.StatusInternalServerError) + } + return + } + part.Close() + } + + // --- Render --- + baseFSys := os.DirFS(h.cfg.BasePath) + uploadFSys := os.DirFS(uploadDir) + overlay := newOverlayFS(uploadFSys, baseFSys) + + pdfFile, err := renderLTML(ltmlBytes, overlay, tmpDir) + if err != nil { + log.Printf("serve-ltml: render: %v", err) + http.Error(w, "render failed", http.StatusInternalServerError) + return + } + defer pdfFile.Close() + + // --- Stream response --- + w.Header().Set("Content-Type", "application/pdf") + w.Header().Set("Content-Disposition", `inline; filename="output.pdf"`) + if _, err := io.Copy(w, pdfFile); err != nil { + // Headers already sent; can only log. + log.Printf("serve-ltml: streaming PDF: %v", err) + } +} + +// validateUploadFilename checks that filename is a safe relative path and +// returns the absolute destination path under uploadDir. +func validateUploadFilename(filename, uploadDir string) (string, error) { + if filename == "" { + return "", fmt.Errorf("filename must not be empty") + } + if path.IsAbs(filename) { + return "", fmt.Errorf("absolute paths are not allowed") + } + // Reject any path element that is "..". + cleaned := path.Clean(filename) + for _, elem := range strings.Split(cleaned, "/") { + if elem == ".." { + return "", fmt.Errorf("path traversal is not allowed") + } + } + // Verify the cleaned path stays within uploadDir after joining. + dest := filepath.Join(uploadDir, filepath.FromSlash(cleaned)) + if !strings.HasPrefix(dest+string(filepath.Separator), uploadDir+string(filepath.Separator)) { + return "", fmt.Errorf("path escapes upload root") + } + return dest, nil +} + +// saveUploadedFile writes the contents of part to destPath, creating parent +// directories as needed. +func saveUploadedFile(r io.Reader, destPath string) error { + if err := os.MkdirAll(filepath.Dir(destPath), 0o700); err != nil { + return err + } + f, err := os.Create(destPath) + if err != nil { + return err + } + defer f.Close() + _, err = io.Copy(f, r) + return err +} + +// rawFilename extracts the "filename" parameter from a Content-Disposition +// header without stripping directory components. The standard library's +// Part.FileName() calls filepath.Base, which would discard path prefixes like +// "assets/" that we want to preserve for nested asset placement. +func rawFilename(contentDisposition string) string { + if contentDisposition == "" { + return "" + } + _, params, err := mime.ParseMediaType(contentDisposition) + if err != nil { + return "" + } + return params["filename"] +} + +// isMaxBytesError reports whether err signals that the request body size limit +// was exceeded (http.MaxBytesReader). +func isMaxBytesError(err error) bool { + var mbe *http.MaxBytesError + return errors.As(err, &mbe) +} diff --git a/ltml/server/handler_test.go b/ltml/server/handler_test.go new file mode 100644 index 0000000..2865592 --- /dev/null +++ b/ltml/server/handler_test.go @@ -0,0 +1,312 @@ +// Copyright 2026 Brent Rowland. +// Use of this source code is governed by the Apache License, Version 2.0, as described in the LICENSE file. + +package main + +import ( + "bytes" + "fmt" + "io" + "mime/multipart" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +// minimalLTML is a tiny valid LTML document used across handler tests. +const minimalLTML = `` + +// buildMultipart constructs a multipart/form-data body with the given parts. +// Each element of parts is [fieldName, filename, contentType, body]. +// Set filename to "" to omit it; set contentType to "" to omit it. +func buildMultipart(parts [][4]string) (body *bytes.Buffer, contentType string) { + body = &bytes.Buffer{} + mw := multipart.NewWriter(body) + for _, p := range parts { + fieldName, filename, ct, data := p[0], p[1], p[2], p[3] + + h := make(map[string][]string) + cd := fmt.Sprintf(`form-data; name="%s"`, fieldName) + if filename != "" { + cd += fmt.Sprintf(`; filename="%s"`, filename) + } + h["Content-Disposition"] = []string{cd} + if ct != "" { + h["Content-Type"] = []string{ct} + } + + pw, err := mw.CreatePart(h) + if err != nil { + panic(err) + } + io.WriteString(pw, data) + } + mw.Close() + return body, mw.FormDataContentType() +} + +// newHandler returns a renderHandler backed by a temp directory used as base-path. +func newHandler(t *testing.T) (*renderHandler, string) { + t.Helper() + base := t.TempDir() + cfg := &Config{ + Listen: ":0", + BasePath: base, + MaxUploadBytes: 32 << 20, + } + return newRenderHandler(cfg), base +} + +// TestHandler_ValidLTMLOnly tests a well-formed request with only an LTML part. +func TestHandler_ValidLTMLOnly(t *testing.T) { + h, _ := newHandler(t) + + body, ct := buildMultipart([][4]string{ + {"ltml", "", "application/vnd.rowland.leadtype.ltml+xml", minimalLTML}, + }) + req := httptest.NewRequest(http.MethodPost, "/render", body) + req.Header.Set("Content-Type", ct) + + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body: %s", rr.Code, rr.Body.String()) + } + if ct := rr.Header().Get("Content-Type"); ct != "application/pdf" { + t.Errorf("Content-Type = %q, want application/pdf", ct) + } + if cd := rr.Header().Get("Content-Disposition"); cd != `inline; filename="output.pdf"` { + t.Errorf("Content-Disposition = %q", cd) + } + if rr.Body.Len() == 0 { + t.Error("response body is empty, expected PDF bytes") + } +} + +// TestHandler_ValidLTMLWithUploadedAsset tests a request that includes an +// uploaded file alongside the LTML document. +func TestHandler_ValidLTMLWithUploadedAsset(t *testing.T) { + h, _ := newHandler(t) + + body, ct := buildMultipart([][4]string{ + {"ltml", "", "application/vnd.rowland.leadtype.ltml+xml", minimalLTML}, + {"file", "logo.txt", "text/plain", "fake-asset-data"}, + }) + req := httptest.NewRequest(http.MethodPost, "/render", body) + req.Header.Set("Content-Type", ct) + + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body: %s", rr.Code, rr.Body.String()) + } +} + +// TestHandler_MissingLTMLPart tests that a missing LTML part yields 400. +func TestHandler_MissingLTMLPart(t *testing.T) { + h, _ := newHandler(t) + + // No parts at all. + body, ct := buildMultipart([][4]string{}) + req := httptest.NewRequest(http.MethodPost, "/render", body) + req.Header.Set("Content-Type", ct) + + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", rr.Code) + } +} + +// TestHandler_LTMLNotFirstPart tests that putting LTML after another part yields 400. +func TestHandler_LTMLNotFirstPart(t *testing.T) { + h, _ := newHandler(t) + + body, ct := buildMultipart([][4]string{ + {"file", "asset.txt", "text/plain", "data"}, + {"ltml", "", "", minimalLTML}, + }) + req := httptest.NewRequest(http.MethodPost, "/render", body) + req.Header.Set("Content-Type", ct) + + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", rr.Code) + } +} + +// TestHandler_InvalidUploadFilename tests that an invalid uploaded filename yields 400. +func TestHandler_InvalidUploadFilename(t *testing.T) { + h, _ := newHandler(t) + + cases := []struct { + name string + filename string + }{ + {"empty", ""}, + {"absolute", "/etc/passwd"}, + {"dotdot", "../secret"}, + {"escapingDotDot", "foo/../../secret"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + body, ct := buildMultipart([][4]string{ + {"ltml", "", "", minimalLTML}, + {"file", tc.filename, "text/plain", "data"}, + }) + req := httptest.NewRequest(http.MethodPost, "/render", body) + req.Header.Set("Content-Type", ct) + + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusBadRequest { + t.Errorf("filename=%q: status = %d, want 400", tc.filename, rr.Code) + } + }) + } +} + +// TestHandler_OversizedRequest tests that an oversized request yields 413. +func TestHandler_OversizedRequest(t *testing.T) { + base := t.TempDir() + cfg := &Config{ + BasePath: base, + MaxUploadBytes: 10, // tiny limit to trigger the error + } + h := newRenderHandler(cfg) + + body, ct := buildMultipart([][4]string{ + {"ltml", "", "", strings.Repeat("X", 100)}, + }) + req := httptest.NewRequest(http.MethodPost, "/render", body) + req.Header.Set("Content-Type", ct) + + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusRequestEntityTooLarge { + t.Errorf("status = %d, want 413", rr.Code) + } +} + +// TestHandler_NonPostMethod tests that GET requests to /render yield 405. +func TestHandler_NonPostMethod(t *testing.T) { + h, _ := newHandler(t) + + req := httptest.NewRequest(http.MethodGet, "/render", nil) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusMethodNotAllowed { + t.Errorf("status = %d, want 405", rr.Code) + } +} + +// TestHandler_TempDirRemovedAfterSuccess verifies that the request temp +// directory is cleaned up after a successful render. +func TestHandler_TempDirRemovedAfterSuccess(t *testing.T) { + // Intercept os.MkdirTemp by checking that no "serve-ltml-*" dirs remain. + // We check all temp dirs before and after the request. + tmpBase := os.TempDir() + before := countServeLTMLDirs(t, tmpBase) + + h, _ := newHandler(t) + body, ct := buildMultipart([][4]string{ + {"ltml", "", "", minimalLTML}, + }) + req := httptest.NewRequest(http.MethodPost, "/render", body) + req.Header.Set("Content-Type", ct) + + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + if rr.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rr.Code) + } + + after := countServeLTMLDirs(t, tmpBase) + if after != before { + t.Errorf("serve-ltml-* temp dirs after request = %d, want %d (leaked)", after, before) + } +} + +// TestHandler_TempDirRemovedAfterRenderFailure verifies that the request temp +// directory is cleaned up even when rendering fails. +func TestHandler_TempDirRemovedAfterRenderFailure(t *testing.T) { + tmpBase := os.TempDir() + before := countServeLTMLDirs(t, tmpBase) + + h, _ := newHandler(t) + body, ct := buildMultipart([][4]string{ + {"ltml", "", "", ""}, + }) + req := httptest.NewRequest(http.MethodPost, "/render", body) + req.Header.Set("Content-Type", ct) + + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + + // Render should fail (bad LTML or bad XML results in 500). + if rr.Code == http.StatusOK { + t.Log("note: render unexpectedly succeeded; skipping cleanup assertion") + return + } + + after := countServeLTMLDirs(t, tmpBase) + if after != before { + t.Errorf("serve-ltml-* temp dirs after failed request = %d, want %d (leaked)", after, before) + } +} + +func countServeLTMLDirs(t *testing.T, base string) int { + t.Helper() + matches, err := filepath.Glob(filepath.Join(base, "serve-ltml-*")) + if err != nil { + t.Fatalf("globbing temp dirs: %v", err) + } + return len(matches) +} + +// TestValidateUploadFilename covers the filename validation helper directly. +func TestValidateUploadFilename(t *testing.T) { + tmpDir := t.TempDir() + + good := []string{ + "logo.png", + "assets/logo.png", + "a/b/c.txt", + } + for _, name := range good { + t.Run("valid:"+name, func(t *testing.T) { + _, err := validateUploadFilename(name, tmpDir) + if err != nil { + t.Errorf("unexpected error for %q: %v", name, err) + } + }) + } + + bad := []string{ + "", + "/etc/passwd", + "../escape", + "a/../../escape", + } + for _, name := range bad { + t.Run("invalid:"+name, func(t *testing.T) { + _, err := validateUploadFilename(name, tmpDir) + if err == nil { + t.Errorf("expected error for %q, got nil", name) + } + }) + } +} diff --git a/ltml/server/main.go b/ltml/server/main.go new file mode 100644 index 0000000..9acc01d --- /dev/null +++ b/ltml/server/main.go @@ -0,0 +1,57 @@ +// Copyright 2026 Brent Rowland. +// Use of this source code is governed by the Apache License, Version 2.0, as described in the LICENSE file. + +// serve-ltml is an HTTP server that renders LTML documents to PDF. +// +// It accepts POST /render requests with multipart/form-data bodies: +// - The first part must have field name "ltml" and contain the LTML document. +// The preferred Content-Type is application/vnd.rowland.leadtype.ltml+xml; +// application/xml, text/xml, and an empty Content-Type are also accepted. +// - Subsequent parts may have field name "file" and contain asset files whose +// multipart filename is used as the virtual path during rendering. These +// assets shadow same-named files in the configured base-path for the +// duration of the request only. +// +// Configuration is accepted as flags or environment variables: +// +// LISTEN / -listen address to listen on (default :8080) +// BASE_PATH / -base-path path to static asset directory (required) +// MAX_UPLOAD_BYTES / -max-upload-bytes request size cap (default 32 MiB) +// READ_TIMEOUT / -read-timeout HTTP read timeout (default none) +// WRITE_TIMEOUT / -write-timeout HTTP write timeout (default none) +package main + +import ( + "fmt" + "log" + "net/http" + "os" +) + +func main() { + cfg, err := parseConfig() + if err != nil { + fmt.Fprintf(os.Stderr, "serve-ltml: %v\n", err) + os.Exit(1) + } + + mux := http.NewServeMux() + mux.Handle("POST /render", newRenderHandler(cfg)) + // Return 405 for non-POST requests to /render. + mux.HandleFunc("/render", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + }) + + srv := &http.Server{ + Addr: cfg.Listen, + Handler: mux, + ReadTimeout: cfg.ReadTimeout, + WriteTimeout: cfg.WriteTimeout, + } + + log.Printf("serve-ltml: listening on %s (base-path=%s)", cfg.Listen, cfg.BasePath) + if err := srv.ListenAndServe(); err != nil { + fmt.Fprintf(os.Stderr, "serve-ltml: %v\n", err) + os.Exit(1) + } +} diff --git a/ltml/server/overlayfs.go b/ltml/server/overlayfs.go new file mode 100644 index 0000000..db6c95c --- /dev/null +++ b/ltml/server/overlayfs.go @@ -0,0 +1,106 @@ +// Copyright 2026 Brent Rowland. +// Use of this source code is governed by the Apache License, Version 2.0, as described in the LICENSE file. + +package main + +import ( + "errors" + "io" + "io/fs" +) + +// overlayFS is a read-only composite filesystem that consults the upper +// filesystem first and falls back to the lower filesystem. It is created +// per request; each request's uploads populate the upper FS while the +// server-wide static content lives in the lower FS. +type overlayFS struct { + upper fs.FS // request upload dir + lower fs.FS // configured base path +} + +// newOverlayFS constructs a per-request overlay. upper is typically an +// os.DirFS rooted at the request temp directory; lower is the static base-path +// FS configured at startup. +func newOverlayFS(upper, lower fs.FS) *overlayFS { + return &overlayFS{upper: upper, lower: lower} +} + +// Open implements fs.FS. It tries upper first and falls back to lower on +// fs.ErrNotExist. +func (o *overlayFS) Open(name string) (fs.File, error) { + f, err := o.upper.Open(name) + if err == nil { + return f, nil + } + if errors.Is(err, fs.ErrNotExist) { + return o.lower.Open(name) + } + return nil, err +} + +// ReadFile implements fs.ReadFileFS. It tries upper first. +func (o *overlayFS) ReadFile(name string) ([]byte, error) { + data, err := fs.ReadFile(o.upper, name) + if err == nil { + return data, nil + } + if errors.Is(err, fs.ErrNotExist) { + return fs.ReadFile(o.lower, name) + } + return nil, err +} + +// Stat implements fs.StatFS. It tries upper first. +func (o *overlayFS) Stat(name string) (fs.FileInfo, error) { + info, err := fs.Stat(o.upper, name) + if err == nil { + return info, nil + } + if errors.Is(err, fs.ErrNotExist) { + return fs.Stat(o.lower, name) + } + return nil, err +} + +// ReadDir implements fs.ReadDirFS. It merges entries from both filesystems; +// entries in upper shadow same-named entries in lower. +func (o *overlayFS) ReadDir(name string) ([]fs.DirEntry, error) { + upperEntries, upperErr := fs.ReadDir(o.upper, name) + lowerEntries, lowerErr := fs.ReadDir(o.lower, name) + + // If neither has the directory, return whichever error is more informative. + if upperErr != nil && lowerErr != nil { + if errors.Is(upperErr, fs.ErrNotExist) { + return nil, lowerErr + } + return nil, upperErr + } + + // Build a merged set; upper entries shadow lower. + seen := make(map[string]struct{}) + var merged []fs.DirEntry + for _, e := range upperEntries { + seen[e.Name()] = struct{}{} + merged = append(merged, e) + } + for _, e := range lowerEntries { + if _, shadowed := seen[e.Name()]; !shadowed { + merged = append(merged, e) + } + } + return merged, nil +} + +// overlayFile wraps an fs.File to satisfy io.ReadCloser without exposing the +// full fs.File interface where only reading is needed. +type overlayFile struct { + fs.File +} + +var ( + _ fs.FS = (*overlayFS)(nil) + _ fs.ReadFileFS = (*overlayFS)(nil) + _ fs.StatFS = (*overlayFS)(nil) + _ fs.ReadDirFS = (*overlayFS)(nil) + _ io.ReadCloser = (*overlayFile)(nil) +) diff --git a/ltml/server/overlayfs_test.go b/ltml/server/overlayfs_test.go new file mode 100644 index 0000000..477b07f --- /dev/null +++ b/ltml/server/overlayfs_test.go @@ -0,0 +1,109 @@ +// Copyright 2026 Brent Rowland. +// Use of this source code is governed by the Apache License, Version 2.0, as described in the LICENSE file. + +package main + +import ( + "io/fs" + "testing" + "testing/fstest" +) + +func makeMapFS(files map[string]string) fs.FS { + m := make(fstest.MapFS) + for name, data := range files { + m[name] = &fstest.MapFile{Data: []byte(data)} + } + return m +} + +// TestOverlayFS_UploadShadowsBase verifies that a file present in both the +// upload dir and the base path is resolved from the upload dir. +func TestOverlayFS_UploadShadowsBase(t *testing.T) { + upper := makeMapFS(map[string]string{"logo.png": "upload-version"}) + lower := makeMapFS(map[string]string{"logo.png": "base-version"}) + + o := newOverlayFS(upper, lower) + + data, err := fs.ReadFile(o, "logo.png") + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if string(data) != "upload-version" { + t.Errorf("got %q, want upload-version", data) + } +} + +// TestOverlayFS_FallsBackToBase verifies that a file absent from the upload +// dir is resolved from the base path. +func TestOverlayFS_FallsBackToBase(t *testing.T) { + upper := makeMapFS(map[string]string{}) + lower := makeMapFS(map[string]string{"static.css": "base-content"}) + + o := newOverlayFS(upper, lower) + + data, err := fs.ReadFile(o, "static.css") + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if string(data) != "base-content" { + t.Errorf("got %q, want base-content", data) + } +} + +// TestOverlayFS_ConcurrentRequestsDoNotShare verifies that two overlay +// instances created for different requests do not share state. +func TestOverlayFS_ConcurrentRequestsDoNotShare(t *testing.T) { + upperA := makeMapFS(map[string]string{"logo.png": "request-A"}) + upperB := makeMapFS(map[string]string{"logo.png": "request-B"}) + lower := makeMapFS(map[string]string{}) + + ovA := newOverlayFS(upperA, lower) + ovB := newOverlayFS(upperB, lower) + + dataA, err := fs.ReadFile(ovA, "logo.png") + if err != nil { + t.Fatalf("request A ReadFile: %v", err) + } + dataB, err := fs.ReadFile(ovB, "logo.png") + if err != nil { + t.Fatalf("request B ReadFile: %v", err) + } + + if string(dataA) != "request-A" { + t.Errorf("request A: got %q, want request-A", dataA) + } + if string(dataB) != "request-B" { + t.Errorf("request B: got %q, want request-B", dataB) + } +} + +// TestOverlayFS_ReadDir_MergesEntries verifies that ReadDir includes entries +// from both upper and lower, with upper entries shadowing lower. +func TestOverlayFS_ReadDir_MergesEntries(t *testing.T) { + upper := makeMapFS(map[string]string{"a.txt": "upper-a", "b.txt": "upper-b"}) + lower := makeMapFS(map[string]string{"b.txt": "lower-b", "c.txt": "lower-c"}) + + o := newOverlayFS(upper, lower) + + entries, err := fs.ReadDir(o, ".") + if err != nil { + t.Fatalf("ReadDir: %v", err) + } + + names := make(map[string]bool) + for _, e := range entries { + names[e.Name()] = true + } + + for _, want := range []string{"a.txt", "b.txt", "c.txt"} { + if !names[want] { + t.Errorf("missing entry %q", want) + } + } + + // b.txt should come from upper (shadow), so count should be 3 not 4. + if len(entries) != 3 { + t.Errorf("entry count = %d, want 3", len(entries)) + } +} diff --git a/ltml/server/render.go b/ltml/server/render.go new file mode 100644 index 0000000..1d31e71 --- /dev/null +++ b/ltml/server/render.go @@ -0,0 +1,50 @@ +// Copyright 2026 Brent Rowland. +// Use of this source code is governed by the Apache License, Version 2.0, as described in the LICENSE file. + +package main + +import ( + "fmt" + "io" + "os" + + "github.com/rowland/leadtype/ltml" + "github.com/rowland/leadtype/ltml/ltpdf" +) + +// renderLTML parses ltmlBytes, attaches overlay as the asset filesystem, +// renders the document to a temp PDF file inside tmpDir, and returns an +// open *os.File positioned at the beginning of the PDF. The caller is +// responsible for closing the file; the file itself lives inside tmpDir +// and will be removed when tmpDir is cleaned up. +func renderLTML(ltmlBytes []byte, overlay *overlayFS, tmpDir string) (*os.File, error) { + doc, err := ltml.Parse(ltmlBytes) + if err != nil { + return nil, fmt.Errorf("parsing LTML: %w", err) + } + + doc.SetAssetFS(overlay) + + w := ltpdf.NewDocWriter() + + if err := doc.Print(w); err != nil { + return nil, fmt.Errorf("rendering LTML: %w", err) + } + + tmpPDF, err := os.CreateTemp(tmpDir, "output-*.pdf") + if err != nil { + return nil, fmt.Errorf("creating temp PDF: %w", err) + } + + if _, err := w.WriteTo(tmpPDF); err != nil { + tmpPDF.Close() + return nil, fmt.Errorf("writing PDF: %w", err) + } + + if _, err := tmpPDF.Seek(0, io.SeekStart); err != nil { + tmpPDF.Close() + return nil, fmt.Errorf("seeking PDF: %w", err) + } + + return tmpPDF, nil +} diff --git a/ltml/std_image.go b/ltml/std_image.go index af761aa..783f3a7 100644 --- a/ltml/std_image.go +++ b/ltml/std_image.go @@ -3,17 +3,56 @@ package ltml -import "fmt" +import ( + "fmt" + "io" +) + +// dataImageWriter is an optional extension of Writer whose implementations +// can render image data supplied as raw bytes rather than a filename. This +// allows the asset filesystem seam to resolve files without altering the +// Writer interface. +type dataImageWriter interface { + ImageDimensions(data []byte) (width, height int, err error) + PrintImage(data []byte, x, y float64, width, height *float64) (actualWidth, actualHeight float64, err error) +} type StdImage struct { StdWidget src string } +// readAsset opens img.src via the scope's asset filesystem when one is set, +// returning the raw bytes. Returns nil, nil when no asset filesystem is +// attached; callers should then fall back to file-path–based Writer methods. +func (img *StdImage) readAsset() ([]byte, error) { + if img.scope == nil { + return nil, nil + } + fsys := img.scope.AssetFS() + if fsys == nil { + return nil, nil + } + f, err := fsys.Open(img.src) + if err != nil { + return nil, err + } + defer f.Close() + return io.ReadAll(f) +} + func (img *StdImage) DrawContent(w Writer) error { if img.src == "" { return fmt.Errorf("image src must be specified") } + if dw, ok := w.(dataImageWriter); ok { + if data, err := img.readAsset(); err != nil { + return err + } else if data != nil { + _, _, err = dw.PrintImage(data, ContentLeft(img), ContentTop(img), img.widthForWriter(), img.heightForWriter()) + return err + } + } _, _, err := w.PrintImageFile(img.src, ContentLeft(img), ContentTop(img), img.widthForWriter(), img.heightForWriter()) return err } @@ -22,7 +61,7 @@ func (img *StdImage) PreferredHeight(w Writer) float64 { if img.height != 0 { return img.height } - infoWidth, infoHeight, err := w.ImageDimensionsFromFile(img.src) + infoWidth, infoHeight, err := img.imageDimensions(w) if err != nil || infoWidth == 0 { return NonContentHeight(img) } @@ -36,7 +75,7 @@ func (img *StdImage) PreferredWidth(w Writer) float64 { if img.width != 0 { return img.width } - infoWidth, infoHeight, err := w.ImageDimensionsFromFile(img.src) + infoWidth, infoHeight, err := img.imageDimensions(w) if err != nil || infoHeight == 0 { return NonContentWidth(img) } @@ -46,6 +85,20 @@ func (img *StdImage) PreferredWidth(w Writer) float64 { return float64(infoWidth) + NonContentWidth(img) } +// imageDimensions returns the pixel dimensions of img.src, using the scope's +// asset filesystem when available and falling back to the Writer's file-based +// method otherwise. +func (img *StdImage) imageDimensions(w Writer) (width, height int, err error) { + if dw, ok := w.(dataImageWriter); ok { + if data, readErr := img.readAsset(); readErr != nil { + return 0, 0, readErr + } else if data != nil { + return dw.ImageDimensions(data) + } + } + return w.ImageDimensionsFromFile(img.src) +} + func (img *StdImage) SetAttrs(attrs map[string]string) { img.StdWidget.SetAttrs(attrs) if src, ok := attrs["src"]; ok { @@ -81,3 +134,4 @@ var _ HasAttrs = (*StdImage)(nil) var _ Identifier = (*StdImage)(nil) var _ Printer = (*StdImage)(nil) var _ WantsContainer = (*StdImage)(nil) +var _ WantsScope = (*StdImage)(nil) From 5856cb25efb38c55cf3d62f26515ae40501b0241 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 28 Mar 2026 05:02:00 +0000 Subject: [PATCH 2/6] Move serve-ltml from ltml/server to cmd/serve-ltml Aligns with the rest of the command layout (cmd/render-ltml). https://claude.ai/code/session_01Fr7DT5rkqVCraSSSEFrWbV --- {ltml/server => cmd/serve-ltml}/config.go | 0 {ltml/server => cmd/serve-ltml}/handler.go | 0 {ltml/server => cmd/serve-ltml}/handler_test.go | 0 {ltml/server => cmd/serve-ltml}/main.go | 0 {ltml/server => cmd/serve-ltml}/overlayfs.go | 0 {ltml/server => cmd/serve-ltml}/overlayfs_test.go | 0 {ltml/server => cmd/serve-ltml}/render.go | 0 7 files changed, 0 insertions(+), 0 deletions(-) rename {ltml/server => cmd/serve-ltml}/config.go (100%) rename {ltml/server => cmd/serve-ltml}/handler.go (100%) rename {ltml/server => cmd/serve-ltml}/handler_test.go (100%) rename {ltml/server => cmd/serve-ltml}/main.go (100%) rename {ltml/server => cmd/serve-ltml}/overlayfs.go (100%) rename {ltml/server => cmd/serve-ltml}/overlayfs_test.go (100%) rename {ltml/server => cmd/serve-ltml}/render.go (100%) diff --git a/ltml/server/config.go b/cmd/serve-ltml/config.go similarity index 100% rename from ltml/server/config.go rename to cmd/serve-ltml/config.go diff --git a/ltml/server/handler.go b/cmd/serve-ltml/handler.go similarity index 100% rename from ltml/server/handler.go rename to cmd/serve-ltml/handler.go diff --git a/ltml/server/handler_test.go b/cmd/serve-ltml/handler_test.go similarity index 100% rename from ltml/server/handler_test.go rename to cmd/serve-ltml/handler_test.go diff --git a/ltml/server/main.go b/cmd/serve-ltml/main.go similarity index 100% rename from ltml/server/main.go rename to cmd/serve-ltml/main.go diff --git a/ltml/server/overlayfs.go b/cmd/serve-ltml/overlayfs.go similarity index 100% rename from ltml/server/overlayfs.go rename to cmd/serve-ltml/overlayfs.go diff --git a/ltml/server/overlayfs_test.go b/cmd/serve-ltml/overlayfs_test.go similarity index 100% rename from ltml/server/overlayfs_test.go rename to cmd/serve-ltml/overlayfs_test.go diff --git a/ltml/server/render.go b/cmd/serve-ltml/render.go similarity index 100% rename from ltml/server/render.go rename to cmd/serve-ltml/render.go From 279979820e39b6a5e04bd04220f841d2c828fea2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 28 Mar 2026 05:20:22 +0000 Subject: [PATCH 3/6] Fix four issues found during audit of serve-ltml overlayfs: sort ReadDir results by name (fs.ReadDirFS contract requires it) overlayfs: remove unused overlayFile type handler: require Content-Type multipart/form-data exactly, not multipart/* main: replace Go 1.22 method-routing syntax with a plain /render pattern; renderHandler already enforces POST and returns 405 for other methods https://claude.ai/code/session_01Fr7DT5rkqVCraSSSEFrWbV --- cmd/serve-ltml/handler.go | 2 +- cmd/serve-ltml/main.go | 7 ++----- cmd/serve-ltml/overlayfs.go | 13 +++++-------- cmd/serve-ltml/overlayfs_test.go | 26 ++++++++++++-------------- 4 files changed, 20 insertions(+), 28 deletions(-) diff --git a/cmd/serve-ltml/handler.go b/cmd/serve-ltml/handler.go index 0101126..58e87ec 100644 --- a/cmd/serve-ltml/handler.go +++ b/cmd/serve-ltml/handler.go @@ -45,7 +45,7 @@ func (h *renderHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { r.Body = http.MaxBytesReader(w, r.Body, h.cfg.MaxUploadBytes) mediaType, params, err := mime.ParseMediaType(r.Header.Get("Content-Type")) - if err != nil || !strings.HasPrefix(mediaType, "multipart/") { + if err != nil || mediaType != "multipart/form-data" { http.Error(w, "Content-Type must be multipart/form-data", http.StatusBadRequest) return } diff --git a/cmd/serve-ltml/main.go b/cmd/serve-ltml/main.go index 9acc01d..8cb8efe 100644 --- a/cmd/serve-ltml/main.go +++ b/cmd/serve-ltml/main.go @@ -36,11 +36,8 @@ func main() { } mux := http.NewServeMux() - mux.Handle("POST /render", newRenderHandler(cfg)) - // Return 405 for non-POST requests to /render. - mux.HandleFunc("/render", func(w http.ResponseWriter, r *http.Request) { - http.Error(w, "method not allowed", http.StatusMethodNotAllowed) - }) + // renderHandler returns 405 for non-POST requests, so a single pattern suffices. + mux.Handle("/render", newRenderHandler(cfg)) srv := &http.Server{ Addr: cfg.Listen, diff --git a/cmd/serve-ltml/overlayfs.go b/cmd/serve-ltml/overlayfs.go index db6c95c..6332945 100644 --- a/cmd/serve-ltml/overlayfs.go +++ b/cmd/serve-ltml/overlayfs.go @@ -5,8 +5,8 @@ package main import ( "errors" - "io" "io/fs" + "sort" ) // overlayFS is a read-only composite filesystem that consults the upper @@ -88,19 +88,16 @@ func (o *overlayFS) ReadDir(name string) ([]fs.DirEntry, error) { merged = append(merged, e) } } + // fs.ReadDirFS requires entries to be sorted by name. + sort.Slice(merged, func(i, j int) bool { + return merged[i].Name() < merged[j].Name() + }) return merged, nil } -// overlayFile wraps an fs.File to satisfy io.ReadCloser without exposing the -// full fs.File interface where only reading is needed. -type overlayFile struct { - fs.File -} - var ( _ fs.FS = (*overlayFS)(nil) _ fs.ReadFileFS = (*overlayFS)(nil) _ fs.StatFS = (*overlayFS)(nil) _ fs.ReadDirFS = (*overlayFS)(nil) - _ io.ReadCloser = (*overlayFile)(nil) ) diff --git a/cmd/serve-ltml/overlayfs_test.go b/cmd/serve-ltml/overlayfs_test.go index 477b07f..f77ddca 100644 --- a/cmd/serve-ltml/overlayfs_test.go +++ b/cmd/serve-ltml/overlayfs_test.go @@ -78,9 +78,10 @@ func TestOverlayFS_ConcurrentRequestsDoNotShare(t *testing.T) { } } -// TestOverlayFS_ReadDir_MergesEntries verifies that ReadDir includes entries -// from both upper and lower, with upper entries shadowing lower. -func TestOverlayFS_ReadDir_MergesEntries(t *testing.T) { +// TestOverlayFS_ReadDir_MergesAndSortsEntries verifies that ReadDir includes +// entries from both upper and lower (with upper shadowing lower) and that the +// result is sorted by name, as required by the fs.ReadDirFS contract. +func TestOverlayFS_ReadDir_MergesAndSortsEntries(t *testing.T) { upper := makeMapFS(map[string]string{"a.txt": "upper-a", "b.txt": "upper-b"}) lower := makeMapFS(map[string]string{"b.txt": "lower-b", "c.txt": "lower-c"}) @@ -91,19 +92,16 @@ func TestOverlayFS_ReadDir_MergesEntries(t *testing.T) { t.Fatalf("ReadDir: %v", err) } - names := make(map[string]bool) - for _, e := range entries { - names[e.Name()] = true + // b.txt from upper shadows lower's b.txt, so exactly 3 entries expected. + if len(entries) != 3 { + t.Fatalf("entry count = %d, want 3", len(entries)) } - for _, want := range []string{"a.txt", "b.txt", "c.txt"} { - if !names[want] { - t.Errorf("missing entry %q", want) + // Entries must be sorted by name (fs.ReadDirFS contract). + want := []string{"a.txt", "b.txt", "c.txt"} + for i, e := range entries { + if e.Name() != want[i] { + t.Errorf("entries[%d].Name() = %q, want %q", i, e.Name(), want[i]) } } - - // b.txt should come from upper (shadow), so count should be 3 not 4. - if len(entries) != 3 { - t.Errorf("entry count = %d, want 3", len(entries)) - } } From 16d232a179bc37f88ce60db64aca0163b1bdf550 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 28 Mar 2026 05:32:01 +0000 Subject: [PATCH 4/6] Move overlayFS to internal/overlayfs; align render-ltml with serve-ltml Extract overlayfs.FS to internal/overlayfs so both commands share it. Rewrite cmd/render-ltml to use doc.SetAssetFS instead of os.Chdir: - Extra files are still symlinked into a temp dir, but the dir is presented as an fs.FS upper layer rather than the process working directory. - When an extra file has the same base name as an assets-dir file, the extra file silently shadows it (last -e flag wins on true name collisions). - No asset filesystem is set when neither -a nor -e are given, preserving backward-compatible behaviour for plain file-path rendering. Update cmd/serve-ltml/overlayfs.go to delegate to internal/overlayfs. Move detailed overlay tests from cmd/serve-ltml to internal/overlayfs. https://claude.ai/code/session_01Fr7DT5rkqVCraSSSEFrWbV --- cmd/render-ltml/main.go | 106 ++++++++++++++++----------- cmd/serve-ltml/overlayfs.go | 97 ++---------------------- cmd/serve-ltml/overlayfs_test.go | 92 +++-------------------- internal/overlayfs/overlayfs.go | 97 ++++++++++++++++++++++++ internal/overlayfs/overlayfs_test.go | 92 +++++++++++++++++++++++ 5 files changed, 268 insertions(+), 216 deletions(-) create mode 100644 internal/overlayfs/overlayfs.go create mode 100644 internal/overlayfs/overlayfs_test.go diff --git a/cmd/render-ltml/main.go b/cmd/render-ltml/main.go index e80db30..385d862 100644 --- a/cmd/render-ltml/main.go +++ b/cmd/render-ltml/main.go @@ -7,9 +7,11 @@ import ( "flag" "fmt" "io" + "io/fs" "os" "path/filepath" + "github.com/rowland/leadtype/internal/overlayfs" "github.com/rowland/leadtype/ltml" "github.com/rowland/leadtype/ltml/ltpdf" ) @@ -57,7 +59,6 @@ func main() { } func run(inputFile, assetsDir, outputPath string, extraFiles []string) error { - // Resolve paths before potentially changing directory. absInput, err := filepath.Abs(inputFile) if err != nil { return fmt.Errorf("resolving input: %w", err) @@ -71,24 +72,24 @@ func run(inputFile, assetsDir, outputPath string, extraFiles []string) error { } } - // Set up asset working directory when assets or extra files are provided. - if assetsDir != "" || len(extraFiles) > 0 { - workDir, cleanup, err := setupWorkDir(assetsDir, extraFiles) - if err != nil { - return err - } - defer cleanup() - if err := os.Chdir(workDir); err != nil { - return fmt.Errorf("chdir to work dir: %w", err) - } - } - // Parse LTML. doc, err := ltml.ParseFile(absInput) if err != nil { return fmt.Errorf("parsing %s: %w", inputFile, err) } + // Build and attach an asset filesystem when assets or extra files are provided. + assetFS, cleanup, err := buildAssetFS(assetsDir, extraFiles) + if err != nil { + return err + } + if cleanup != nil { + defer cleanup() + } + if assetFS != nil { + doc.SetAssetFS(assetFS) + } + // Render to PDF. w := ltpdf.NewDocWriter() if err := doc.Print(w); err != nil { @@ -114,49 +115,66 @@ func run(inputFile, assetsDir, outputPath string, extraFiles []string) error { return nil } -// setupWorkDir creates a temporary directory populated with symlinks to the -// contents of assetsDir and each file in extraFiles. The caller must invoke -// the returned cleanup function when rendering is complete. -func setupWorkDir(assetsDir string, extraFiles []string) (string, func(), error) { - tmpDir, err := os.MkdirTemp("", "render-ltml-*") - if err != nil { - return "", nil, fmt.Errorf("creating work dir: %w", err) +// buildAssetFS constructs an fs.FS that covers assetsDir (lower layer) and the +// named extraFiles (upper layer, each stored under its base name). Extra files +// shadow same-named entries in assetsDir rather than erroring on conflict. +// +// Returns nil, nil, nil when neither assetsDir nor extraFiles are provided. +// When a non-nil cleanup function is returned, the caller must invoke it after +// rendering is complete. +func buildAssetFS(assetsDir string, extraFiles []string) (fs.FS, func(), error) { + hasAssets := assetsDir != "" + hasExtras := len(extraFiles) > 0 + + if !hasAssets && !hasExtras { + return nil, nil, nil } - cleanup := func() { os.RemoveAll(tmpDir) } - if assetsDir != "" { - absAssets, err := filepath.Abs(assetsDir) + // Place extra files in a temp directory so they can be addressed as an + // os.DirFS. Symlinks keep memory use low for large files. + var extraDir string + var cleanup func() + if hasExtras { + tmpDir, err := os.MkdirTemp("", "render-ltml-*") if err != nil { - cleanup() - return "", nil, fmt.Errorf("resolving assets dir: %w", err) - } - entries, err := os.ReadDir(absAssets) - if err != nil { - cleanup() - return "", nil, fmt.Errorf("reading assets dir: %w", err) + return nil, nil, fmt.Errorf("creating work dir: %w", err) } - for _, entry := range entries { - src := filepath.Join(absAssets, entry.Name()) - dst := filepath.Join(tmpDir, entry.Name()) - if err := os.Symlink(src, dst); err != nil { + cleanup = func() { os.RemoveAll(tmpDir) } + extraDir = tmpDir + + for _, f := range extraFiles { + abs, err := filepath.Abs(f) + if err != nil { + cleanup() + return nil, nil, fmt.Errorf("resolving extra file %s: %w", f, err) + } + dst := filepath.Join(extraDir, filepath.Base(f)) + // If two extra files share a base name, the last one wins. + os.Remove(dst) + if err := os.Symlink(abs, dst); err != nil { cleanup() - return "", nil, fmt.Errorf("linking asset %s: %w", entry.Name(), err) + return nil, nil, fmt.Errorf("linking extra file %s: %w", filepath.Base(f), err) } } } - for _, f := range extraFiles { - abs, err := filepath.Abs(f) + switch { + case hasExtras && hasAssets: + absAssets, err := filepath.Abs(assetsDir) if err != nil { cleanup() - return "", nil, fmt.Errorf("resolving extra file %s: %w", f, err) + return nil, nil, fmt.Errorf("resolving assets dir: %w", err) } - dst := filepath.Join(tmpDir, filepath.Base(f)) - if err := os.Symlink(abs, dst); err != nil { - cleanup() - return "", nil, fmt.Errorf("linking extra file %s: %w", filepath.Base(f), err) + return overlayfs.New(os.DirFS(extraDir), os.DirFS(absAssets)), cleanup, nil + + case hasExtras: + return os.DirFS(extraDir), cleanup, nil + + default: // hasAssets only + absAssets, err := filepath.Abs(assetsDir) + if err != nil { + return nil, nil, fmt.Errorf("resolving assets dir: %w", err) } + return os.DirFS(absAssets), nil, nil } - - return tmpDir, cleanup, nil } diff --git a/cmd/serve-ltml/overlayfs.go b/cmd/serve-ltml/overlayfs.go index 6332945..c7ee9e3 100644 --- a/cmd/serve-ltml/overlayfs.go +++ b/cmd/serve-ltml/overlayfs.go @@ -4,100 +4,17 @@ package main import ( - "errors" "io/fs" - "sort" + + "github.com/rowland/leadtype/internal/overlayfs" ) -// overlayFS is a read-only composite filesystem that consults the upper -// filesystem first and falls back to the lower filesystem. It is created -// per request; each request's uploads populate the upper FS while the -// server-wide static content lives in the lower FS. -type overlayFS struct { - upper fs.FS // request upload dir - lower fs.FS // configured base path -} +// overlayFS is an alias for the shared overlay filesystem type. Upper is the +// per-request upload directory; lower is the server-wide static base path. +type overlayFS = overlayfs.FS // newOverlayFS constructs a per-request overlay. upper is typically an -// os.DirFS rooted at the request temp directory; lower is the static base-path -// FS configured at startup. +// os.DirFS rooted at the request upload directory; lower is the base-path FS. func newOverlayFS(upper, lower fs.FS) *overlayFS { - return &overlayFS{upper: upper, lower: lower} -} - -// Open implements fs.FS. It tries upper first and falls back to lower on -// fs.ErrNotExist. -func (o *overlayFS) Open(name string) (fs.File, error) { - f, err := o.upper.Open(name) - if err == nil { - return f, nil - } - if errors.Is(err, fs.ErrNotExist) { - return o.lower.Open(name) - } - return nil, err -} - -// ReadFile implements fs.ReadFileFS. It tries upper first. -func (o *overlayFS) ReadFile(name string) ([]byte, error) { - data, err := fs.ReadFile(o.upper, name) - if err == nil { - return data, nil - } - if errors.Is(err, fs.ErrNotExist) { - return fs.ReadFile(o.lower, name) - } - return nil, err -} - -// Stat implements fs.StatFS. It tries upper first. -func (o *overlayFS) Stat(name string) (fs.FileInfo, error) { - info, err := fs.Stat(o.upper, name) - if err == nil { - return info, nil - } - if errors.Is(err, fs.ErrNotExist) { - return fs.Stat(o.lower, name) - } - return nil, err + return overlayfs.New(upper, lower) } - -// ReadDir implements fs.ReadDirFS. It merges entries from both filesystems; -// entries in upper shadow same-named entries in lower. -func (o *overlayFS) ReadDir(name string) ([]fs.DirEntry, error) { - upperEntries, upperErr := fs.ReadDir(o.upper, name) - lowerEntries, lowerErr := fs.ReadDir(o.lower, name) - - // If neither has the directory, return whichever error is more informative. - if upperErr != nil && lowerErr != nil { - if errors.Is(upperErr, fs.ErrNotExist) { - return nil, lowerErr - } - return nil, upperErr - } - - // Build a merged set; upper entries shadow lower. - seen := make(map[string]struct{}) - var merged []fs.DirEntry - for _, e := range upperEntries { - seen[e.Name()] = struct{}{} - merged = append(merged, e) - } - for _, e := range lowerEntries { - if _, shadowed := seen[e.Name()]; !shadowed { - merged = append(merged, e) - } - } - // fs.ReadDirFS requires entries to be sorted by name. - sort.Slice(merged, func(i, j int) bool { - return merged[i].Name() < merged[j].Name() - }) - return merged, nil -} - -var ( - _ fs.FS = (*overlayFS)(nil) - _ fs.ReadFileFS = (*overlayFS)(nil) - _ fs.StatFS = (*overlayFS)(nil) - _ fs.ReadDirFS = (*overlayFS)(nil) -) diff --git a/cmd/serve-ltml/overlayfs_test.go b/cmd/serve-ltml/overlayfs_test.go index f77ddca..7da374e 100644 --- a/cmd/serve-ltml/overlayfs_test.go +++ b/cmd/serve-ltml/overlayfs_test.go @@ -17,91 +17,19 @@ func makeMapFS(files map[string]string) fs.FS { return m } -// TestOverlayFS_UploadShadowsBase verifies that a file present in both the -// upload dir and the base path is resolved from the upload dir. -func TestOverlayFS_UploadShadowsBase(t *testing.T) { - upper := makeMapFS(map[string]string{"logo.png": "upload-version"}) - lower := makeMapFS(map[string]string{"logo.png": "base-version"}) +// TestNewOverlayFS_Wiring verifies that newOverlayFS correctly wires upper and +// lower so that the upper layer takes precedence. Detailed overlay semantics +// are tested in internal/overlayfs. +func TestNewOverlayFS_Wiring(t *testing.T) { + upper := makeMapFS(map[string]string{"f.txt": "upper"}) + lower := makeMapFS(map[string]string{"f.txt": "lower", "g.txt": "lower-only"}) o := newOverlayFS(upper, lower) - data, err := fs.ReadFile(o, "logo.png") - if err != nil { - t.Fatalf("ReadFile: %v", err) + if data, err := fs.ReadFile(o, "f.txt"); err != nil || string(data) != "upper" { + t.Errorf("f.txt: got %q, %v; want upper, nil", data, err) } - if string(data) != "upload-version" { - t.Errorf("got %q, want upload-version", data) - } -} - -// TestOverlayFS_FallsBackToBase verifies that a file absent from the upload -// dir is resolved from the base path. -func TestOverlayFS_FallsBackToBase(t *testing.T) { - upper := makeMapFS(map[string]string{}) - lower := makeMapFS(map[string]string{"static.css": "base-content"}) - - o := newOverlayFS(upper, lower) - - data, err := fs.ReadFile(o, "static.css") - if err != nil { - t.Fatalf("ReadFile: %v", err) - } - if string(data) != "base-content" { - t.Errorf("got %q, want base-content", data) - } -} - -// TestOverlayFS_ConcurrentRequestsDoNotShare verifies that two overlay -// instances created for different requests do not share state. -func TestOverlayFS_ConcurrentRequestsDoNotShare(t *testing.T) { - upperA := makeMapFS(map[string]string{"logo.png": "request-A"}) - upperB := makeMapFS(map[string]string{"logo.png": "request-B"}) - lower := makeMapFS(map[string]string{}) - - ovA := newOverlayFS(upperA, lower) - ovB := newOverlayFS(upperB, lower) - - dataA, err := fs.ReadFile(ovA, "logo.png") - if err != nil { - t.Fatalf("request A ReadFile: %v", err) - } - dataB, err := fs.ReadFile(ovB, "logo.png") - if err != nil { - t.Fatalf("request B ReadFile: %v", err) - } - - if string(dataA) != "request-A" { - t.Errorf("request A: got %q, want request-A", dataA) - } - if string(dataB) != "request-B" { - t.Errorf("request B: got %q, want request-B", dataB) - } -} - -// TestOverlayFS_ReadDir_MergesAndSortsEntries verifies that ReadDir includes -// entries from both upper and lower (with upper shadowing lower) and that the -// result is sorted by name, as required by the fs.ReadDirFS contract. -func TestOverlayFS_ReadDir_MergesAndSortsEntries(t *testing.T) { - upper := makeMapFS(map[string]string{"a.txt": "upper-a", "b.txt": "upper-b"}) - lower := makeMapFS(map[string]string{"b.txt": "lower-b", "c.txt": "lower-c"}) - - o := newOverlayFS(upper, lower) - - entries, err := fs.ReadDir(o, ".") - if err != nil { - t.Fatalf("ReadDir: %v", err) - } - - // b.txt from upper shadows lower's b.txt, so exactly 3 entries expected. - if len(entries) != 3 { - t.Fatalf("entry count = %d, want 3", len(entries)) - } - - // Entries must be sorted by name (fs.ReadDirFS contract). - want := []string{"a.txt", "b.txt", "c.txt"} - for i, e := range entries { - if e.Name() != want[i] { - t.Errorf("entries[%d].Name() = %q, want %q", i, e.Name(), want[i]) - } + if data, err := fs.ReadFile(o, "g.txt"); err != nil || string(data) != "lower-only" { + t.Errorf("g.txt: got %q, %v; want lower-only, nil", data, err) } } diff --git a/internal/overlayfs/overlayfs.go b/internal/overlayfs/overlayfs.go new file mode 100644 index 0000000..3f03c78 --- /dev/null +++ b/internal/overlayfs/overlayfs.go @@ -0,0 +1,97 @@ +// Copyright 2026 Brent Rowland. +// Use of this source code is governed by the Apache License, Version 2.0, as described in the LICENSE file. + +// Package overlayfs provides a read-only composite fs.FS that resolves names +// against an upper filesystem first and falls back to a lower filesystem. +package overlayfs + +import ( + "errors" + "io/fs" + "sort" +) + +// FS is a read-only composite filesystem. Lookups are tried against upper +// first; on fs.ErrNotExist the lookup falls through to lower. +type FS struct { + upper fs.FS + lower fs.FS +} + +// New returns an FS that consults upper before lower for every lookup. +func New(upper, lower fs.FS) *FS { + return &FS{upper: upper, lower: lower} +} + +// Open implements fs.FS. +func (o *FS) Open(name string) (fs.File, error) { + f, err := o.upper.Open(name) + if err == nil { + return f, nil + } + if errors.Is(err, fs.ErrNotExist) { + return o.lower.Open(name) + } + return nil, err +} + +// ReadFile implements fs.ReadFileFS. +func (o *FS) ReadFile(name string) ([]byte, error) { + data, err := fs.ReadFile(o.upper, name) + if err == nil { + return data, nil + } + if errors.Is(err, fs.ErrNotExist) { + return fs.ReadFile(o.lower, name) + } + return nil, err +} + +// Stat implements fs.StatFS. +func (o *FS) Stat(name string) (fs.FileInfo, error) { + info, err := fs.Stat(o.upper, name) + if err == nil { + return info, nil + } + if errors.Is(err, fs.ErrNotExist) { + return fs.Stat(o.lower, name) + } + return nil, err +} + +// ReadDir implements fs.ReadDirFS. Entries from upper shadow same-named entries +// from lower; the merged result is sorted by name as required by the contract. +func (o *FS) ReadDir(name string) ([]fs.DirEntry, error) { + upperEntries, upperErr := fs.ReadDir(o.upper, name) + lowerEntries, lowerErr := fs.ReadDir(o.lower, name) + + if upperErr != nil && lowerErr != nil { + if errors.Is(upperErr, fs.ErrNotExist) { + return nil, lowerErr + } + return nil, upperErr + } + + seen := make(map[string]struct{}) + var merged []fs.DirEntry + for _, e := range upperEntries { + seen[e.Name()] = struct{}{} + merged = append(merged, e) + } + for _, e := range lowerEntries { + if _, shadowed := seen[e.Name()]; !shadowed { + merged = append(merged, e) + } + } + sort.Slice(merged, func(i, j int) bool { + return merged[i].Name() < merged[j].Name() + }) + return merged, nil +} + +var ( + _ fs.FS = (*FS)(nil) + _ fs.ReadFileFS = (*FS)(nil) + _ fs.StatFS = (*FS)(nil) + _ fs.ReadDirFS = (*FS)(nil) +) diff --git a/internal/overlayfs/overlayfs_test.go b/internal/overlayfs/overlayfs_test.go new file mode 100644 index 0000000..1073608 --- /dev/null +++ b/internal/overlayfs/overlayfs_test.go @@ -0,0 +1,92 @@ +// Copyright 2026 Brent Rowland. +// Use of this source code is governed by the Apache License, Version 2.0, as described in the LICENSE file. + +package overlayfs_test + +import ( + "io/fs" + "testing" + "testing/fstest" + + "github.com/rowland/leadtype/internal/overlayfs" +) + +func makeMapFS(files map[string]string) fs.FS { + m := make(fstest.MapFS) + for name, data := range files { + m[name] = &fstest.MapFile{Data: []byte(data)} + } + return m +} + +func TestOverlayFS_UpperShadowsLower(t *testing.T) { + upper := makeMapFS(map[string]string{"logo.png": "upper-version"}) + lower := makeMapFS(map[string]string{"logo.png": "lower-version"}) + + o := overlayfs.New(upper, lower) + + data, err := fs.ReadFile(o, "logo.png") + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if string(data) != "upper-version" { + t.Errorf("got %q, want upper-version", data) + } +} + +func TestOverlayFS_FallsBackToLower(t *testing.T) { + upper := makeMapFS(map[string]string{}) + lower := makeMapFS(map[string]string{"static.css": "lower-content"}) + + o := overlayfs.New(upper, lower) + + data, err := fs.ReadFile(o, "static.css") + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if string(data) != "lower-content" { + t.Errorf("got %q, want lower-content", data) + } +} + +func TestOverlayFS_IndependentInstances(t *testing.T) { + upperA := makeMapFS(map[string]string{"logo.png": "A"}) + upperB := makeMapFS(map[string]string{"logo.png": "B"}) + lower := makeMapFS(map[string]string{}) + + ovA := overlayfs.New(upperA, lower) + ovB := overlayfs.New(upperB, lower) + + dataA, _ := fs.ReadFile(ovA, "logo.png") + dataB, _ := fs.ReadFile(ovB, "logo.png") + + if string(dataA) != "A" { + t.Errorf("instance A: got %q, want A", dataA) + } + if string(dataB) != "B" { + t.Errorf("instance B: got %q, want B", dataB) + } +} + +func TestOverlayFS_ReadDir_MergesAndSortsEntries(t *testing.T) { + upper := makeMapFS(map[string]string{"a.txt": "upper-a", "b.txt": "upper-b"}) + lower := makeMapFS(map[string]string{"b.txt": "lower-b", "c.txt": "lower-c"}) + + o := overlayfs.New(upper, lower) + + entries, err := fs.ReadDir(o, ".") + if err != nil { + t.Fatalf("ReadDir: %v", err) + } + + if len(entries) != 3 { + t.Fatalf("entry count = %d, want 3", len(entries)) + } + + want := []string{"a.txt", "b.txt", "c.txt"} + for i, e := range entries { + if e.Name() != want[i] { + t.Errorf("entries[%d].Name() = %q, want %q", i, e.Name(), want[i]) + } + } +} From 7072b078ce31d59e3a2d9de774a37120fd8a2752 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 28 Mar 2026 05:45:27 +0000 Subject: [PATCH 5/6] Add README.md for render-ltml and serve-ltml commands https://claude.ai/code/session_01Fr7DT5rkqVCraSSSEFrWbV --- cmd/render-ltml/README.md | 47 ++++++++++++++++++++ cmd/serve-ltml/README.md | 92 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 cmd/render-ltml/README.md create mode 100644 cmd/serve-ltml/README.md diff --git a/cmd/render-ltml/README.md b/cmd/render-ltml/README.md new file mode 100644 index 0000000..102fdde --- /dev/null +++ b/cmd/render-ltml/README.md @@ -0,0 +1,47 @@ +# render-ltml + +`render-ltml` converts an LTML document to PDF and writes the result to a file or stdout. + +## Usage + +``` +render-ltml [flags] +``` + +### Flags + +| Flag | Short | Description | +|------|-------|-------------| +| `-assets ` | `-a` | Directory of static assets available during rendering | +| `-extra ` | `-e` | Additional asset file (may be repeated) | +| `-output ` | `-o` | Write PDF to this file instead of stdout | + +### Asset resolution + +When `-assets` and/or `-extra` are given, a virtual filesystem is constructed and attached to the document before rendering. Images and other assets referenced in the LTML are resolved through this filesystem: + +- Files supplied with `-extra` form the **upper layer** and shadow same-named files from `-assets`. +- Files in the `-assets` directory form the **lower layer** and are used when an asset is not supplied as an extra file. +- When neither flag is given, asset paths are resolved by the PDF writer directly (relative to the working directory). + +If the same base name is given more than once via `-extra`, the last occurrence wins. + +## Examples + +Render to stdout and pipe into a PDF viewer: + +```sh +render-ltml report.ltml | zathura - +``` + +Render to a file with a directory of shared assets: + +```sh +render-ltml -a ./assets -o report.pdf report.ltml +``` + +Override one asset without touching the shared directory: + +```sh +render-ltml -a ./assets -e ./branded/logo.png -o report.pdf report.ltml +``` diff --git a/cmd/serve-ltml/README.md b/cmd/serve-ltml/README.md new file mode 100644 index 0000000..479fd72 --- /dev/null +++ b/cmd/serve-ltml/README.md @@ -0,0 +1,92 @@ +# serve-ltml + +`serve-ltml` is an HTTP server that renders LTML documents to PDF on demand. Clients submit an LTML document and optional asset files in a single `multipart/form-data` request and receive a PDF response. + +## Usage + +``` +serve-ltml [flags] +``` + +Every flag can also be set via the corresponding environment variable. + +### Flags and environment variables + +| Flag | Environment variable | Default | Description | +|------|----------------------|---------|-------------| +| `-listen ` | `LISTEN` | `:8080` | Address to listen on | +| `-base-path ` | `BASE_PATH` | *(required)* | Directory of static assets available to all requests | +| `-max-upload-bytes ` | `MAX_UPLOAD_BYTES` | `33554432` (32 MiB) | Maximum request body size | +| `-read-timeout ` | `READ_TIMEOUT` | none | HTTP server read timeout (e.g. `30s`) | +| `-write-timeout ` | `WRITE_TIMEOUT` | none | HTTP server write timeout (e.g. `60s`) | + +`BASE_PATH` must exist and be a directory; the server refuses to start otherwise. + +## API + +### `POST /render` + +Render an LTML document to PDF. + +**Request** + +`Content-Type: multipart/form-data` + +| Part | Field name | Required | Description | +|------|------------|----------|-------------| +| LTML document | `ltml` | Yes | Must be the **first** part. Preferred content type: `application/vnd.rowland.leadtype.ltml+xml`; `application/xml`, `text/xml`, and no content type are also accepted. | +| Asset file | `file` | No | May be repeated. The part's `filename` parameter is used as the virtual asset path (e.g. `logo.png` or `assets/logo.png`). | + +**Response** + +| Status | Meaning | +|--------|---------| +| `200 OK` | PDF rendered successfully. Body is the PDF; `Content-Type: application/pdf`; `Content-Disposition: inline; filename="output.pdf"`. | +| `400 Bad Request` | Malformed multipart body, missing or misplaced `ltml` part, empty LTML, or invalid upload filename. | +| `405 Method Not Allowed` | Request method is not `POST`. | +| `413 Request Entity Too Large` | Request body exceeds `-max-upload-bytes`. | +| `500 Internal Server Error` | Temp-file, parse, render, or stream failure. | + +### Asset resolution + +Uploaded files form a **per-request upper layer** that shadows same-named files in the configured `base-path` for the duration of that request only. Parallel requests never share upload state. Uploaded filenames must be non-empty relative paths that do not escape the upload root (absolute paths and `..` components are rejected). + +## Examples + +Start the server: + +```sh +serve-ltml -base-path /var/lib/ltml/assets +``` + +Or with environment variables: + +```sh +BASE_PATH=/var/lib/ltml/assets READ_TIMEOUT=30s WRITE_TIMEOUT=60s serve-ltml +``` + +Render a document with no uploaded assets: + +```sh +curl -s \ + -F 'ltml=@report.ltml;type=application/vnd.rowland.leadtype.ltml+xml' \ + http://localhost:8080/render -o report.pdf +``` + +Render with an asset that overrides the server's base-path copy: + +```sh +curl -s \ + -F 'ltml=@report.ltml;type=application/vnd.rowland.leadtype.ltml+xml' \ + -F 'file=@./branded/logo.png;filename=logo.png' \ + http://localhost:8080/render -o report.pdf +``` + +Place an asset at a nested path: + +```sh +curl -s \ + -F 'ltml=@report.ltml;type=application/vnd.rowland.leadtype.ltml+xml' \ + -F 'file=@./img/logo.png;filename=assets/logo.png' \ + http://localhost:8080/render -o report.pdf +``` From 5bbe23749ced6b13be9630c086b2a10809b8e541 Mon Sep 17 00:00:00 2001 From: Brent Rowland Date: Sat, 28 Mar 2026 11:03:05 -0700 Subject: [PATCH 6/6] Unify asset FS handling in pdf layer --- Makefile | 12 +- cmd/render-ltml/README.md | 4 +- cmd/render-ltml/main.go | 17 +-- cmd/render-ltml/main_test.go | 67 +++++++++++ cmd/serve-ltml/README.md | 2 +- cmd/serve-ltml/handler.go | 25 +--- cmd/serve-ltml/handler_test.go | 8 ++ cmd/serve-ltml/render.go | 12 +- ltml/asset_fs_test.go | 172 --------------------------- ltml/has_scope.go | 4 - ltml/image_asset_integration_test.go | 91 ++++++++++++++ ltml/ltml.go | 9 -- ltml/scope.go | 21 ---- ltml/std_image.go | 47 -------- pdf/doc_writer.go | 112 ++++++++++++++--- pdf/doc_writer_test.go | 123 +++++++++++++++++++ pdf/images.go | 99 +-------------- pdf/images_test.go | 109 ----------------- pdf/page_writer.go | 81 +++++++++---- pdf/page_writer_test.go | 45 +++++++ 20 files changed, 528 insertions(+), 532 deletions(-) create mode 100644 cmd/render-ltml/main_test.go delete mode 100644 ltml/asset_fs_test.go create mode 100644 ltml/image_asset_integration_test.go diff --git a/Makefile b/Makefile index fa7fcb6..3788628 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,16 @@ -.PHONY: test samples +.PHONY: binaries test samples SAMPLES := $(sort $(wildcard samples/*/*.go)) +BIN_DIR := bin +BINARY_PKGS := ./cmd/render-ltml ./cmd/serve-ltml ./ttdump + +binaries: + @mkdir -p $(BIN_DIR) + @for pkg in $(BINARY_PKGS); do \ + name=$$(basename $$pkg); \ + echo "==> go build -o $(BIN_DIR)/$$name $$pkg"; \ + go build -o $(BIN_DIR)/$$name $$pkg || exit $$?; \ + done test: go test ./... diff --git a/cmd/render-ltml/README.md b/cmd/render-ltml/README.md index 102fdde..78b3d96 100644 --- a/cmd/render-ltml/README.md +++ b/cmd/render-ltml/README.md @@ -18,12 +18,14 @@ render-ltml [flags] ### Asset resolution -When `-assets` and/or `-extra` are given, a virtual filesystem is constructed and attached to the document before rendering. Images and other assets referenced in the LTML are resolved through this filesystem: +When `-assets` and/or `-extra` are given, a virtual filesystem is constructed and attached to the PDF writer before rendering. Asset-backed PDF operations resolve through this filesystem: - Files supplied with `-extra` form the **upper layer** and shadow same-named files from `-assets`. - Files in the `-assets` directory form the **lower layer** and are used when an asset is not supplied as an extra file. - When neither flag is given, asset paths are resolved by the PDF writer directly (relative to the working directory). +When an asset filesystem is attached, asset names must be clean relative `fs.FS` paths such as `logo.png` or `assets/logo.png`. Paths like `./logo.png`, `a/../logo.png`, or absolute paths are rejected. + If the same base name is given more than once via `-extra`, the last occurrence wins. ## Examples diff --git a/cmd/render-ltml/main.go b/cmd/render-ltml/main.go index 385d862..6b094e3 100644 --- a/cmd/render-ltml/main.go +++ b/cmd/render-ltml/main.go @@ -79,19 +79,19 @@ func run(inputFile, assetsDir, outputPath string, extraFiles []string) error { } // Build and attach an asset filesystem when assets or extra files are provided. - assetFS, cleanup, err := buildAssetFS(assetsDir, extraFiles) + assetFS, cleanup, err := buildOptionalAssetFS(assetsDir, extraFiles) if err != nil { return err } if cleanup != nil { defer cleanup() } - if assetFS != nil { - doc.SetAssetFS(assetFS) - } // Render to PDF. w := ltpdf.NewDocWriter() + if assetFS != nil { + w.SetAssetFS(assetFS) + } if err := doc.Print(w); err != nil { return fmt.Errorf("rendering: %w", err) } @@ -115,14 +115,15 @@ func run(inputFile, assetsDir, outputPath string, extraFiles []string) error { return nil } -// buildAssetFS constructs an fs.FS that covers assetsDir (lower layer) and the -// named extraFiles (upper layer, each stored under its base name). Extra files -// shadow same-named entries in assetsDir rather than erroring on conflict. +// buildOptionalAssetFS constructs an optional fs.FS that covers assetsDir +// (lower layer) and the named extraFiles (upper layer, each stored under its +// base name). Extra files shadow same-named entries in assetsDir rather than +// erroring on conflict. // // Returns nil, nil, nil when neither assetsDir nor extraFiles are provided. // When a non-nil cleanup function is returned, the caller must invoke it after // rendering is complete. -func buildAssetFS(assetsDir string, extraFiles []string) (fs.FS, func(), error) { +func buildOptionalAssetFS(assetsDir string, extraFiles []string) (fs.FS, func(), error) { hasAssets := assetsDir != "" hasExtras := len(extraFiles) > 0 diff --git a/cmd/render-ltml/main_test.go b/cmd/render-ltml/main_test.go new file mode 100644 index 0000000..ba8a0e6 --- /dev/null +++ b/cmd/render-ltml/main_test.go @@ -0,0 +1,67 @@ +// Copyright 2026 Brent Rowland. +// Use of this source code is governed by the Apache License, Version 2.0, as described in the LICENSE file. + +package main + +import ( + "io/fs" + "os" + "path/filepath" + "testing" +) + +func TestBuildOptionalAssetFS_ExtraOverridesAssetsDir(t *testing.T) { + assetsDir := t.TempDir() + if err := os.WriteFile(filepath.Join(assetsDir, "logo.txt"), []byte("lower"), 0o600); err != nil { + t.Fatal(err) + } + + extraDir := t.TempDir() + extraFile := filepath.Join(extraDir, "logo.txt") + if err := os.WriteFile(extraFile, []byte("upper"), 0o600); err != nil { + t.Fatal(err) + } + + assetFS, cleanup, err := buildOptionalAssetFS(assetsDir, []string{extraFile}) + if err != nil { + t.Fatal(err) + } + if cleanup != nil { + defer cleanup() + } + + data, err := fs.ReadFile(assetFS, "logo.txt") + if err != nil { + t.Fatal(err) + } + if string(data) != "upper" { + t.Fatalf("expected upper override, got %q", data) + } +} + +func TestBuildOptionalAssetFS_PreservesNestedAssetPathsFromAssetsDir(t *testing.T) { + assetsDir := t.TempDir() + nestedDir := filepath.Join(assetsDir, "assets") + if err := os.MkdirAll(nestedDir, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(nestedDir, "logo.txt"), []byte("nested"), 0o600); err != nil { + t.Fatal(err) + } + + assetFS, cleanup, err := buildOptionalAssetFS(assetsDir, nil) + if err != nil { + t.Fatal(err) + } + if cleanup != nil { + defer cleanup() + } + + data, err := fs.ReadFile(assetFS, "assets/logo.txt") + if err != nil { + t.Fatal(err) + } + if string(data) != "nested" { + t.Fatalf("expected nested asset, got %q", data) + } +} diff --git a/cmd/serve-ltml/README.md b/cmd/serve-ltml/README.md index 479fd72..810d7bd 100644 --- a/cmd/serve-ltml/README.md +++ b/cmd/serve-ltml/README.md @@ -49,7 +49,7 @@ Render an LTML document to PDF. ### Asset resolution -Uploaded files form a **per-request upper layer** that shadows same-named files in the configured `base-path` for the duration of that request only. Parallel requests never share upload state. Uploaded filenames must be non-empty relative paths that do not escape the upload root (absolute paths and `..` components are rejected). +Uploaded files form a **per-request upper layer** that shadows same-named files in the configured `base-path` for the duration of that request only. Parallel requests never share upload state. Uploaded filenames must be clean relative `fs.FS` paths such as `logo.png` or `assets/logo.png`; empty names, `.`, paths containing `.` / `..` segments, and absolute paths are rejected. ## Examples diff --git a/cmd/serve-ltml/handler.go b/cmd/serve-ltml/handler.go index 58e87ec..f65f16a 100644 --- a/cmd/serve-ltml/handler.go +++ b/cmd/serve-ltml/handler.go @@ -7,14 +7,13 @@ import ( "errors" "fmt" "io" + "io/fs" "log" "mime" "mime/multipart" "net/http" "os" - "path" "path/filepath" - "strings" ) // ltmlContentTypes lists the accepted Content-Type values for the LTML part. @@ -187,28 +186,16 @@ func (h *renderHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } } -// validateUploadFilename checks that filename is a safe relative path and -// returns the absolute destination path under uploadDir. +// validateUploadFilename checks that filename is a clean fs.FS-relative path +// and returns the absolute destination path under uploadDir. func validateUploadFilename(filename, uploadDir string) (string, error) { if filename == "" { return "", fmt.Errorf("filename must not be empty") } - if path.IsAbs(filename) { - return "", fmt.Errorf("absolute paths are not allowed") + if filename == "." || !fs.ValidPath(filename) { + return "", fmt.Errorf("filename must be a clean relative asset path") } - // Reject any path element that is "..". - cleaned := path.Clean(filename) - for _, elem := range strings.Split(cleaned, "/") { - if elem == ".." { - return "", fmt.Errorf("path traversal is not allowed") - } - } - // Verify the cleaned path stays within uploadDir after joining. - dest := filepath.Join(uploadDir, filepath.FromSlash(cleaned)) - if !strings.HasPrefix(dest+string(filepath.Separator), uploadDir+string(filepath.Separator)) { - return "", fmt.Errorf("path escapes upload root") - } - return dest, nil + return filepath.Join(uploadDir, filepath.FromSlash(filename)), nil } // saveUploadedFile writes the contents of part to destPath, creating parent diff --git a/cmd/serve-ltml/handler_test.go b/cmd/serve-ltml/handler_test.go index 2865592..8ef350d 100644 --- a/cmd/serve-ltml/handler_test.go +++ b/cmd/serve-ltml/handler_test.go @@ -153,7 +153,11 @@ func TestHandler_InvalidUploadFilename(t *testing.T) { }{ {"empty", ""}, {"absolute", "/etc/passwd"}, + {"dot", "."}, + {"dotSlash", "./logo.png"}, {"dotdot", "../secret"}, + {"normalizedDotDot", "a/../logo.png"}, + {"normalizedDir", "a/.."}, {"escapingDotDot", "foo/../../secret"}, } @@ -297,8 +301,12 @@ func TestValidateUploadFilename(t *testing.T) { bad := []string{ "", + ".", + "./logo.png", "/etc/passwd", "../escape", + "a/../logo.png", + "a/..", "a/../../escape", } for _, name := range bad { diff --git a/cmd/serve-ltml/render.go b/cmd/serve-ltml/render.go index 1d31e71..2e08771 100644 --- a/cmd/serve-ltml/render.go +++ b/cmd/serve-ltml/render.go @@ -12,20 +12,24 @@ import ( "github.com/rowland/leadtype/ltml/ltpdf" ) -// renderLTML parses ltmlBytes, attaches overlay as the asset filesystem, -// renders the document to a temp PDF file inside tmpDir, and returns an +// renderLTML parses ltmlBytes, renders the document using overlay as the +// writer's asset filesystem, and returns a temp PDF file inside tmpDir. +// The returned file is positioned at the beginning of the PDF. // open *os.File positioned at the beginning of the PDF. The caller is // responsible for closing the file; the file itself lives inside tmpDir // and will be removed when tmpDir is cleaned up. func renderLTML(ltmlBytes []byte, overlay *overlayFS, tmpDir string) (*os.File, error) { + if overlay == nil { + return nil, fmt.Errorf("missing asset filesystem") + } + doc, err := ltml.Parse(ltmlBytes) if err != nil { return nil, fmt.Errorf("parsing LTML: %w", err) } - doc.SetAssetFS(overlay) - w := ltpdf.NewDocWriter() + w.SetAssetFS(overlay) if err := doc.Print(w); err != nil { return nil, fmt.Errorf("rendering LTML: %w", err) diff --git a/ltml/asset_fs_test.go b/ltml/asset_fs_test.go deleted file mode 100644 index c924289..0000000 --- a/ltml/asset_fs_test.go +++ /dev/null @@ -1,172 +0,0 @@ -// Copyright 2026 Brent Rowland. -// Use of this source code is governed by the Apache License, Version 2.0, as described in the LICENSE file. - -package ltml - -import ( - "io/fs" - "testing" - "testing/fstest" -) - -// fakeFS is a simple in-memory filesystem for testing asset FS threading. -func fakeFS(files map[string][]byte) fs.FS { - m := make(fstest.MapFS) - for name, data := range files { - m[name] = &fstest.MapFile{Data: data} - } - return m -} - -// TestDoc_SetAssetFS_IsDocumentLocal verifies that each document carries its -// own asset filesystem and that one document's FS does not affect another. -func TestDoc_SetAssetFS_IsDocumentLocal(t *testing.T) { - fsA := fakeFS(map[string][]byte{"logo.png": []byte("A")}) - fsB := fakeFS(map[string][]byte{"logo.png": []byte("B")}) - - docA, err := Parse([]byte("")) - if err != nil { - t.Fatal(err) - } - docB, err := Parse([]byte("")) - if err != nil { - t.Fatal(err) - } - - docA.SetAssetFS(fsA) - docB.SetAssetFS(fsB) - - // Doc A's scope should see fsA. - gotA := docA.scope().AssetFS() - if gotA == nil { - t.Fatal("docA assetFS is nil") - } - dataA, err := fs.ReadFile(gotA, "logo.png") - if err != nil { - t.Fatalf("docA: reading logo.png: %v", err) - } - if string(dataA) != "A" { - t.Errorf("docA logo.png = %q, want \"A\"", dataA) - } - - // Doc B's scope should see fsB. - gotB := docB.scope().AssetFS() - if gotB == nil { - t.Fatal("docB assetFS is nil") - } - dataB, err := fs.ReadFile(gotB, "logo.png") - if err != nil { - t.Fatalf("docB: reading logo.png: %v", err) - } - if string(dataB) != "B" { - t.Errorf("docB logo.png = %q, want \"B\"", dataB) - } -} - -// TestScope_AssetFS_InheritedByNestedScope verifies that a nested scope -// inherits the asset filesystem from the nearest ancestor that has one set. -func TestScope_AssetFS_InheritedByNestedScope(t *testing.T) { - fsys := fakeFS(map[string][]byte{"img.png": []byte("root")}) - - root := &Scope{} - root.SetAssetFS(fsys) - - child := &Scope{} - child.SetParentScope(root) - - grandchild := &Scope{} - grandchild.SetParentScope(child) - - // Verify inheritance by reading a file through the inherited FS. - got := grandchild.AssetFS() - if got == nil { - t.Fatal("grandchild AssetFS() is nil; expected to inherit from root") - } - data, err := fs.ReadFile(got, "img.png") - if err != nil { - t.Fatalf("reading via inherited FS: %v", err) - } - if string(data) != "root" { - t.Errorf("data = %q, want \"root\"", data) - } -} - -// TestScope_AssetFS_NilWhenNotSet verifies that AssetFS returns nil when no -// ancestor has an asset filesystem configured. -func TestScope_AssetFS_NilWhenNotSet(t *testing.T) { - root := &Scope{} - child := &Scope{} - child.SetParentScope(root) - - if child.AssetFS() != nil { - t.Error("expected nil assetFS when none is set") - } -} - -// dataWriterSpy records calls to PrintImage and ImageDimensions so tests can -// verify that the FS-based code path is taken. -type dataWriterSpy struct { - imageTestWriter - imageDimensionsCalls int - printImageCalls int - lastData []byte -} - -func (s *dataWriterSpy) ImageDimensions(data []byte) (width, height int, err error) { - s.imageDimensionsCalls++ - s.lastData = data - return 100, 50, nil -} - -func (s *dataWriterSpy) PrintImage(data []byte, x, y float64, width, height *float64) (actualWidth, actualHeight float64, err error) { - s.printImageCalls++ - s.lastData = data - return 0, 0, nil -} - -// TestStdImage_DrawContent_UsesAssetFS verifies that when a scope has an asset -// filesystem and the writer supports data-based image methods, DrawContent -// resolves the image via the FS rather than via PrintImageFile. -func TestStdImage_DrawContent_UsesAssetFS(t *testing.T) { - pngBytes := []byte("fake-png-data") - fsys := fakeFS(map[string][]byte{"logo.png": pngBytes}) - - scope := &Scope{} - scope.SetAssetFS(fsys) - - img := &StdImage{src: "logo.png"} - img.SetScope(scope) - - spy := &dataWriterSpy{} - if err := img.DrawContent(spy); err != nil { - t.Fatalf("DrawContent: %v", err) - } - - if spy.printImageCalls != 1 { - t.Errorf("PrintImage calls = %d, want 1", spy.printImageCalls) - } - if len(spy.calls) != 0 { - t.Errorf("PrintImageFile unexpectedly called %d time(s)", len(spy.calls)) - } - if string(spy.lastData) != string(pngBytes) { - t.Errorf("data passed to PrintImage = %q, want %q", spy.lastData, pngBytes) - } -} - -// TestStdImage_DrawContent_FallsBackToFilePath verifies that when no asset -// filesystem is set, DrawContent uses the Writer's file-path–based method. -func TestStdImage_DrawContent_FallsBackToFilePath(t *testing.T) { - img := &StdImage{src: "fixture.jpg"} - w := &imageTestWriter{} - - if err := img.DrawContent(w); err != nil { - t.Fatalf("DrawContent: %v", err) - } - - if len(w.calls) != 1 { - t.Fatalf("PrintImageFile calls = %d, want 1", len(w.calls)) - } - if w.calls[0].filename != "fixture.jpg" { - t.Errorf("filename = %q, want fixture.jpg", w.calls[0].filename) - } -} diff --git a/ltml/has_scope.go b/ltml/has_scope.go index 7194b79..32fe7ff 100644 --- a/ltml/has_scope.go +++ b/ltml/has_scope.go @@ -3,8 +3,6 @@ package ltml -import "io/fs" - type HasScope interface { AddAlias(alias *Alias) error AddLayout(layout *LayoutStyle) error @@ -12,11 +10,9 @@ type HasScope interface { AddRules(rules *Rules) error AddStyle(style Styler) error AliasFor(name string) (alias *Alias, ok bool) - AssetFS() fs.FS EachRuleFor(path string, f func(rule *Rule)) LayoutFor(id string) (layout *LayoutStyle, ok bool) PageStyleFor(id string) (style *PageStyle, ok bool) - SetAssetFS(fsys fs.FS) SetParentScope(parent HasScope) StyleFor(id string) (style Styler, ok bool) } diff --git a/ltml/image_asset_integration_test.go b/ltml/image_asset_integration_test.go new file mode 100644 index 0000000..b089716 --- /dev/null +++ b/ltml/image_asset_integration_test.go @@ -0,0 +1,91 @@ +// Copyright 2026 Brent Rowland. +// Use of this source code is governed by the Apache License, Version 2.0, as described in the LICENSE file. + +package ltml + +import ( + "bytes" + "image" + "image/color" + "image/png" + "strings" + "testing" + "testing/fstest" + + "github.com/rowland/leadtype/ltml/ltpdf" +) + +func encodeTestPNG(t *testing.T) []byte { + t.Helper() + img := image.NewRGBA(image.Rect(0, 0, 2, 1)) + img.Set(0, 0, color.RGBA{R: 0x10, G: 0x20, B: 0x30, A: 0xFF}) + img.Set(1, 0, color.RGBA{R: 0x40, G: 0x50, B: 0x60, A: 0xFF}) + + var buf bytes.Buffer + if err := png.Encode(&buf, img); err != nil { + t.Fatal(err) + } + return buf.Bytes() +} + +func renderWithAssetFS(t *testing.T, doc *Doc) string { + t.Helper() + imageFS := fstest.MapFS{ + "logo.png": &fstest.MapFile{Data: encodeTestPNG(t)}, + } + + w := ltpdf.NewDocWriter() + w.SetAssetFS(imageFS) + if err := doc.Print(w); err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + if _, err := w.WriteTo(&buf); err != nil { + t.Fatal(err) + } + return buf.String() +} + +func renderDirectPDFWithAssetFS(t *testing.T) string { + t.Helper() + w := ltpdf.NewDocWriter() + w.SetAssetFS(fstest.MapFS{ + "logo.png": &fstest.MapFile{Data: encodeTestPNG(t)}, + }) + w.NewPage() + if _, _, err := w.PrintImageFile("logo.png", 1, 1, nil, nil); err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + if _, err := w.WriteTo(&buf); err != nil { + t.Fatal(err) + } + return buf.String() +} + +func TestStdImage_UsesWriterAssetFS(t *testing.T) { + doc, err := Parse([]byte(` + + + + +`)) + if err != nil { + t.Fatal(err) + } + + ltmlPDF := renderWithAssetFS(t, doc) + directPDF := renderDirectPDFWithAssetFS(t) + + for _, pdf := range []string{ltmlPDF, directPDF} { + if !strings.Contains(pdf, "/Subtype /Image") { + t.Fatalf("expected rendered PDF to contain an image object, got:\n%s", pdf) + } + if !strings.Contains(pdf, "/Im0 Do\n") { + t.Fatalf("expected rendered PDF to draw an image, got:\n%s", pdf) + } + } + if strings.Count(ltmlPDF, "/Subtype /Image") != strings.Count(directPDF, "/Subtype /Image") { + t.Fatalf("expected LTML and direct PDF paths to embed the same number of image objects") + } +} diff --git a/ltml/ltml.go b/ltml/ltml.go index 23af3a0..91653bc 100644 --- a/ltml/ltml.go +++ b/ltml/ltml.go @@ -8,7 +8,6 @@ import ( "encoding/xml" "fmt" "io" - "io/fs" "os" "strings" ) @@ -224,14 +223,6 @@ func (doc *Doc) current() (value interface{}) { return } -// SetAssetFS attaches an asset filesystem to this document's root scope. -// All nested scopes that do not have their own asset filesystem will inherit -// it, so assets looked up during rendering are resolved against fsys first -// and fall through to whatever the renderer's default resolution is. -func (doc *Doc) SetAssetFS(fsys fs.FS) { - doc.rootScope.SetAssetFS(fsys) -} - func (doc *Doc) scope() HasScope { if len(doc.scopes) > 0 { return doc.scopes[len(doc.scopes)-1] diff --git a/ltml/scope.go b/ltml/scope.go index d8e38a8..6a26906 100644 --- a/ltml/scope.go +++ b/ltml/scope.go @@ -6,7 +6,6 @@ package ltml import ( "bytes" "fmt" - "io/fs" ) type Scope struct { @@ -16,7 +15,6 @@ type Scope struct { layouts map[string]*LayoutStyle pageStyles map[string]*PageStyle rules []*Rules - assetFS fs.FS } func (scope *Scope) AddAlias(alias *Alias) error { @@ -116,25 +114,6 @@ func (scope *Scope) SetParentScope(parent HasScope) { scope.parent = parent } -// SetAssetFS stores the asset filesystem for this scope. Nested scopes that -// do not have their own asset filesystem inherit it from the nearest ancestor -// that has one set. -func (scope *Scope) SetAssetFS(fsys fs.FS) { - scope.assetFS = fsys -} - -// AssetFS returns the nearest asset filesystem in scope, walking up the parent -// chain until one is found. Returns nil if no ancestor has an asset filesystem. -func (scope *Scope) AssetFS() fs.FS { - if scope.assetFS != nil { - return scope.assetFS - } - if scope.parent != nil { - return scope.parent.AssetFS() - } - return nil -} - func (scope *Scope) String() string { var buf bytes.Buffer for name, alias := range scope.aliases { diff --git a/ltml/std_image.go b/ltml/std_image.go index 783f3a7..b14be20 100644 --- a/ltml/std_image.go +++ b/ltml/std_image.go @@ -5,54 +5,17 @@ package ltml import ( "fmt" - "io" ) -// dataImageWriter is an optional extension of Writer whose implementations -// can render image data supplied as raw bytes rather than a filename. This -// allows the asset filesystem seam to resolve files without altering the -// Writer interface. -type dataImageWriter interface { - ImageDimensions(data []byte) (width, height int, err error) - PrintImage(data []byte, x, y float64, width, height *float64) (actualWidth, actualHeight float64, err error) -} - type StdImage struct { StdWidget src string } -// readAsset opens img.src via the scope's asset filesystem when one is set, -// returning the raw bytes. Returns nil, nil when no asset filesystem is -// attached; callers should then fall back to file-path–based Writer methods. -func (img *StdImage) readAsset() ([]byte, error) { - if img.scope == nil { - return nil, nil - } - fsys := img.scope.AssetFS() - if fsys == nil { - return nil, nil - } - f, err := fsys.Open(img.src) - if err != nil { - return nil, err - } - defer f.Close() - return io.ReadAll(f) -} - func (img *StdImage) DrawContent(w Writer) error { if img.src == "" { return fmt.Errorf("image src must be specified") } - if dw, ok := w.(dataImageWriter); ok { - if data, err := img.readAsset(); err != nil { - return err - } else if data != nil { - _, _, err = dw.PrintImage(data, ContentLeft(img), ContentTop(img), img.widthForWriter(), img.heightForWriter()) - return err - } - } _, _, err := w.PrintImageFile(img.src, ContentLeft(img), ContentTop(img), img.widthForWriter(), img.heightForWriter()) return err } @@ -85,17 +48,7 @@ func (img *StdImage) PreferredWidth(w Writer) float64 { return float64(infoWidth) + NonContentWidth(img) } -// imageDimensions returns the pixel dimensions of img.src, using the scope's -// asset filesystem when available and falling back to the Writer's file-based -// method otherwise. func (img *StdImage) imageDimensions(w Writer) (width, height int, err error) { - if dw, ok := w.(dataImageWriter); ok { - if data, readErr := img.readAsset(); readErr != nil { - return 0, 0, readErr - } else if data != nil { - return dw.ImageDimensions(data) - } - } return w.ImageDimensionsFromFile(img.src) } diff --git a/pdf/doc_writer.go b/pdf/doc_writer.go index e8cac22..4dd5766 100644 --- a/pdf/doc_writer.go +++ b/pdf/doc_writer.go @@ -7,6 +7,8 @@ import ( "crypto/rand" "fmt" "io" + "io/fs" + "os" "github.com/rowland/leadtype/codepage" "github.com/rowland/leadtype/colors" @@ -16,24 +18,25 @@ import ( ) type DocWriter struct { - pages []*PageWriter - nextSeq func() int - file *file - catalog *catalog - resources *resources - pagesAcross int - pagesDown int - curPage *PageWriter - options options.Options - fontSources font.FontSources - fontKeys map[string]string - fontEncodings map[string]*fontEncoding - glyphRecorders map[string]*glyphRecorder // keyed by font PostScript name - unicodeFonts map[string]*font.Font // PostScript name → font, for width lookup at Close - cidFonts map[string]*cidFont // PostScript name → CID font, for /W update at Close - type0Fonts map[string]*type0Font // PostScript name → Type0 font, for ToUnicode at Close - fontDescriptors map[string]*fontDescriptor // PostScript name → descriptor, for FontFile2 at Close - images map[string]*cachedImage + pages []*PageWriter + nextSeq func() int + file *file + catalog *catalog + resources *resources + pagesAcross int + pagesDown int + curPage *PageWriter + options options.Options + fontSources font.FontSources + fontKeys map[string]string + fontEncodings map[string]*fontEncoding + glyphRecorders map[string]*glyphRecorder // keyed by font PostScript name + unicodeFonts map[string]*font.Font // PostScript name → font, for width lookup at Close + cidFonts map[string]*cidFont // PostScript name → CID font, for /W update at Close + type0Fonts map[string]*type0Font // PostScript name → Type0 font, for ToUnicode at Close + fontDescriptors map[string]*fontDescriptor // PostScript name → descriptor, for FontFile2 at Close + images map[string]*cachedImage + assetFS fs.FS compressPages bool compressToUnicode bool compressEmbeddedFonts bool @@ -118,6 +121,27 @@ func (dw *DocWriter) FontColor() colors.Color { return dw.CurPage().FontColor() } +func (dw *DocWriter) SetAssetFS(assetFS fs.FS) { + dw.assetFS = assetFS +} + +func (dw *DocWriter) readImageFile(filename string) ([]byte, error) { + if dw.assetFS == nil { + return os.ReadFile(filename) + } + if !validAssetPath(filename) { + return nil, fmt.Errorf("invalid asset path %q", filename) + } + info, err := fs.Stat(dw.assetFS, filename) + if err != nil { + return nil, err + } + if info.IsDir() { + return nil, fmt.Errorf("asset path %q is a directory", filename) + } + return fs.ReadFile(dw.assetFS, filename) +} + func useStandardEncoding(family string) bool { switch family { case "Symbol", "ZapfDingbats": @@ -525,7 +549,11 @@ func (dw *DocWriter) ImageDimensions(data []byte) (width, height int, err error) } func (dw *DocWriter) ImageDimensionsFromFile(filename string) (width, height int, err error) { - return imageDimensionsFromFile(filename) + data, err := dw.readImageFile(filename) + if err != nil { + return 0, 0, err + } + return imageDimensions(data) } func (dw *DocWriter) Path(fn func()) error { @@ -572,6 +600,52 @@ func (dw *DocWriter) PrintImageFile(filename string, x, y float64, width, height return dw.CurPage().PrintImageFile(filename, x, y, width, height) } +func (dw *DocWriter) loadImage(data []byte, key string) (*pdfImage, string, error) { + if cached, ok := dw.images[key]; ok { + return cached.image, cached.name, nil + } + decoded, err := decodeImage(data) + if err != nil { + return nil, "", err + } + components := decoded.info.components + if len(decoded.alphaData) > 0 && (components == imageComponentsGrayAlpha || components == imageComponentsRGBA) { + components-- + } + colorSpace, err := imageColorSpace(components) + if err != nil { + return nil, "", err + } + image := newPDFImage(dw.nextSeq(), 0, decoded.data) + image.setDimensions(decoded.info.width, decoded.info.height) + image.setBitsPerComponent(decoded.info.bitsPerComponent) + image.setColorSpace(colorSpace) + if decoded.filter == "FlateDecode" { + if err := image.compress(); err != nil { + return nil, "", err + } + } else if decoded.filter != "" { + image.setFilter(decoded.filter) + } + if len(decoded.alphaData) > 0 { + mask := newPDFImage(dw.nextSeq(), 0, decoded.alphaData) + mask.setDimensions(decoded.info.width, decoded.info.height) + mask.setBitsPerComponent(decoded.info.bitsPerComponent) + mask.setColorSpace("DeviceGray") + if err := mask.compress(); err != nil { + return nil, "", err + } + dw.file.body.add(mask) + image.setSMask(&indirectObjectRef{mask}) + } + + name := fmt.Sprintf("Im%d", len(dw.images)) + dw.file.body.add(image) + dw.resources.setXObject(name, &indirectObjectRef{image}) + dw.images[key] = &cachedImage{image: image, name: name} + return image, name, nil +} + func (dw *DocWriter) ResetFonts() { dw.CurPage().ResetFonts() } diff --git a/pdf/doc_writer_test.go b/pdf/doc_writer_test.go index d564191..3e8ef5f 100644 --- a/pdf/doc_writer_test.go +++ b/pdf/doc_writer_test.go @@ -5,8 +5,11 @@ package pdf import ( "bytes" + "image" + "io/fs" "strings" "testing" + "testing/fstest" "github.com/rowland/leadtype/afm_fonts" "github.com/rowland/leadtype/codepage" @@ -202,6 +205,126 @@ func TestDocWriter_PageWidth(t *testing.T) { expectF(t, 8500, dw.PageWidth()) } +func TestDocWriter_PrintImageFile_Integration(t *testing.T) { + var buf bytes.Buffer + + dw := NewDocWriter() + dw.SetUnits("in") + dw.NewPage() + actualWidth, actualHeight, err := dw.PrintImageFile("testdata/testimg.jpg", 1, 1, floatPtr(2.0), nil) + if err != nil { + t.Fatal(err) + } + expectFdelta(t, 2.0, actualWidth, 0.0001) + if actualHeight <= 0 { + t.Fatalf("expected positive inferred height, got %f", actualHeight) + } + if _, err := dw.WriteTo(&buf); err != nil { + t.Fatal(err) + } + pdf := buf.String() + for _, fragment := range []string{ + "/Subtype /Image", + "/Filter /DCTDecode", + "/ColorSpace /DeviceRGB", + "/XObject <<", + "/Im0 ", + " cm\n", + "/Im0 Do\n", + } { + if !strings.Contains(pdf, fragment) { + t.Fatalf("expected generated PDF to contain %q, got:\n%s", fragment, pdf) + } + } +} + +func TestDocWriter_PrintImageFile_PNG_Integration(t *testing.T) { + var buf bytes.Buffer + + dw := NewDocWriter() + dw.SetUnits("in") + dw.NewPage() + actualWidth, actualHeight, err := dw.PrintImageFile("testdata/eidetic.png", 1, 1, floatPtr(2.0), nil) + if err != nil { + t.Fatal(err) + } + expectFdelta(t, 2.0, actualWidth, 0.0001) + if actualHeight <= 0 { + t.Fatalf("expected positive inferred height, got %f", actualHeight) + } + if _, err := dw.WriteTo(&buf); err != nil { + t.Fatal(err) + } + pdf := buf.String() + for _, fragment := range []string{ + "/Subtype /Image", + "/Filter /FlateDecode", + "/ColorSpace /DeviceRGB", + "/XObject <<", + "/Im0 ", + " cm\n", + "/Im0 Do\n", + } { + if !strings.Contains(pdf, fragment) { + t.Fatalf("expected generated PDF to contain %q, got:\n%s", fragment, pdf) + } + } +} + +func TestDocWriter_PrintImageFile_UsesAssetFS(t *testing.T) { + img := image.NewRGBA(image.Rect(0, 0, 3, 2)) + data := mustEncodePNG(t, img) + + dw := NewDocWriter() + dw.SetAssetFS(fstest.MapFS{ + "logo.png": &fstest.MapFile{Data: data}, + }) + dw.NewPage() + + width, height, err := dw.PrintImageFile("logo.png", 1, 1, nil, nil) + if err != nil { + t.Fatal(err) + } + if width != 3 || height != 2 { + t.Fatalf("expected intrinsic size 3x2, got %.2fx%.2f", width, height) + } +} + +func TestDocWriter_ImageDimensionsFromFile_UsesAssetFS(t *testing.T) { + img := image.NewGray(image.Rect(0, 0, 4, 3)) + data := mustEncodePNG(t, img) + + dw := NewDocWriter() + dw.SetAssetFS(fstest.MapFS{ + "nested/logo.png": &fstest.MapFile{Data: data}, + }) + + width, height, err := dw.ImageDimensionsFromFile("nested/logo.png") + if err != nil { + t.Fatal(err) + } + if width != 4 || height != 3 { + t.Fatalf("expected dimensions 4x3, got %dx%d", width, height) + } +} + +func TestDocWriter_ImageAssetPathValidation(t *testing.T) { + dw := NewDocWriter() + dw.SetAssetFS(fstest.MapFS{ + "logo.png": &fstest.MapFile{Data: mustEncodePNG(t, image.NewRGBA(image.Rect(0, 0, 1, 1)))}, + "dir": &fstest.MapFile{Mode: fs.ModeDir}, + }) + + bad := []string{"", ".", "./logo.png", "a/../logo.png", "/tmp/logo.png", "dir"} + for _, name := range bad { + t.Run(name, func(t *testing.T) { + if _, _, err := dw.PrintImageFile(name, 1, 1, nil, nil); err == nil { + t.Fatalf("expected error for %q, got nil", name) + } + }) + } +} + func TestDocWriter_SetFont_TrueType(t *testing.T) { skipIfNoTTFFonts(t) dw := NewDocWriter() diff --git a/pdf/images.go b/pdf/images.go index 422b48b..9696ef7 100644 --- a/pdf/images.go +++ b/pdf/images.go @@ -11,7 +11,7 @@ import ( "fmt" "image/color" "image/png" - "os" + "io/fs" ) var errUnsupportedImageFormat = errors.New("unsupported image format") @@ -112,6 +112,10 @@ func imageKey(data []byte) string { } } +func validAssetPath(name string) bool { + return name != "." && fs.ValidPath(name) +} + func isJPEG(image []byte) bool { return len(image) >= 2 && image[0] == 0xFF && image[1] == 0xD8 } @@ -227,10 +231,6 @@ func pngInfo(data []byte) (imageInfo, error) { }, nil } -func readImageFile(filename string) ([]byte, error) { - return os.ReadFile(filename) -} - func imageInfoForData(data []byte) (imageInfo, error) { if isJPEG(data) { return jpegInfo(data) @@ -249,14 +249,6 @@ func imageDimensions(data []byte) (width, height int, err error) { return info.width, info.height, nil } -func imageDimensionsFromFile(filename string) (width, height int, err error) { - data, err := readImageFile(filename) - if err != nil { - return 0, 0, err - } - return imageDimensions(data) -} - func imageSizeInPoints(info imageInfo, units *units, width, height *float64) (float64, float64) { if width == nil && height == nil { return float64(info.width), float64(info.height) @@ -359,84 +351,3 @@ func decodeImage(data []byte) (decodedImage, error) { } return decodedImage{}, errUnsupportedImageFormat } - -func (dw *DocWriter) loadImage(data []byte, key string) (*pdfImage, string, error) { - if cached, ok := dw.images[key]; ok { - return cached.image, cached.name, nil - } - decoded, err := decodeImage(data) - if err != nil { - return nil, "", err - } - components := decoded.info.components - if len(decoded.alphaData) > 0 && (components == imageComponentsGrayAlpha || components == imageComponentsRGBA) { - components-- - } - colorSpace, err := imageColorSpace(components) - if err != nil { - return nil, "", err - } - image := newPDFImage(dw.nextSeq(), 0, decoded.data) - image.setDimensions(decoded.info.width, decoded.info.height) - image.setBitsPerComponent(decoded.info.bitsPerComponent) - image.setColorSpace(colorSpace) - if decoded.filter == "FlateDecode" { - if err := image.compress(); err != nil { - return nil, "", err - } - } else if decoded.filter != "" { - image.setFilter(decoded.filter) - } - if len(decoded.alphaData) > 0 { - mask := newPDFImage(dw.nextSeq(), 0, decoded.alphaData) - mask.setDimensions(decoded.info.width, decoded.info.height) - mask.setBitsPerComponent(decoded.info.bitsPerComponent) - mask.setColorSpace("DeviceGray") - if err := mask.compress(); err != nil { - return nil, "", err - } - dw.file.body.add(mask) - image.setSMask(&indirectObjectRef{mask}) - } - - name := fmt.Sprintf("Im%d", len(dw.images)) - dw.file.body.add(image) - dw.resources.setXObject(name, &indirectObjectRef{image}) - dw.images[key] = &cachedImage{image: image, name: name} - return image, name, nil -} - -func (pw *PageWriter) PrintImage(data []byte, x, y float64, width, height *float64) (actualWidth, actualHeight float64, err error) { - key := imageKey(data) - image, name, err := pw.dw.loadImage(data, key) - if err != nil { - return 0, 0, err - } - xpts := pw.units.toPts(x) - ypts := pw.units.toPts(y) - info := imageInfo{ - width: image.width, - height: image.height, - bitsPerComponent: image.bitsPerComponent, - } - wpts, hpts := imageSizeInPoints(info, pw.units, width, height) - if pw.inPath { - pw.endPath() - } - if pw.inText { - pw.endText() - } - if pw.inGraph { - pw.endGraph() - } - writeImageXObject(pw.mw, pw.gw, name, xpts, ypts, wpts, hpts, pw.pageHeight) - return pw.units.fromPts(wpts), pw.units.fromPts(hpts), nil -} - -func (pw *PageWriter) PrintImageFile(filename string, x, y float64, width, height *float64) (actualWidth, actualHeight float64, err error) { - data, err := readImageFile(filename) - if err != nil { - return 0, 0, err - } - return pw.PrintImage(data, x, y, width, height) -} diff --git a/pdf/images_test.go b/pdf/images_test.go index c6c143f..977c9cd 100644 --- a/pdf/images_test.go +++ b/pdf/images_test.go @@ -206,82 +206,6 @@ func TestDecodePNG_Gray(t *testing.T) { } } -func TestPageWriter_PrintImage_DefaultSize(t *testing.T) { - data := mustReadTestImage(t) - cfg, err := jpeg.DecodeConfig(bytes.NewReader(data)) - if err != nil { - t.Fatal(err) - } - - dw := NewDocWriter() - pw := newPageWriter(dw, options.Options{}) - width, height, err := pw.PrintImage(data, 10, 20, nil, nil) - if err != nil { - t.Fatal(err) - } - if width != float64(cfg.Width) || height != float64(cfg.Height) { - t.Fatalf("expected intrinsic size %dx%d, got %.2fx%.2f", cfg.Width, cfg.Height, width, height) - } - if !strings.Contains(pw.stream.String(), "/Im0 Do\n") { - t.Fatalf("expected image draw operator, got:\n%s", pw.stream.String()) - } - if dw.resources.xObjects["Im0"] == nil { - t.Fatal("expected image XObject registered in resources") - } -} - -func TestPageWriter_PrintImage_AspectRatio(t *testing.T) { - data := mustReadTestImage(t) - cfg, err := jpeg.DecodeConfig(bytes.NewReader(data)) - if err != nil { - t.Fatal(err) - } - - dw := NewDocWriter() - pw := newPageWriter(dw, options.Options{"units": "in"}) - width := 2.0 - actualWidth, actualHeight, err := pw.PrintImage(data, 1, 2, floatPtr(width), nil) - if err != nil { - t.Fatal(err) - } - expectedHeight := width * float64(cfg.Height) / float64(cfg.Width) - expectFdelta(t, width, actualWidth, 0.0001) - expectFdelta(t, expectedHeight, actualHeight, 0.0001) -} - -func TestDocWriter_PrintImageFile_Integration(t *testing.T) { - var buf bytes.Buffer - - dw := NewDocWriter() - dw.SetUnits("in") - dw.NewPage() - actualWidth, actualHeight, err := dw.PrintImageFile("testdata/testimg.jpg", 1, 1, floatPtr(2.0), nil) - if err != nil { - t.Fatal(err) - } - expectFdelta(t, 2.0, actualWidth, 0.0001) - if actualHeight <= 0 { - t.Fatalf("expected positive inferred height, got %f", actualHeight) - } - if _, err := dw.WriteTo(&buf); err != nil { - t.Fatal(err) - } - pdf := buf.String() - for _, fragment := range []string{ - "/Subtype /Image", - "/Filter /DCTDecode", - "/ColorSpace /DeviceRGB", - "/XObject <<", - "/Im0 ", - " cm\n", - "/Im0 Do\n", - } { - if !strings.Contains(pdf, fragment) { - t.Fatalf("expected generated PDF to contain %q, got:\n%s", fragment, pdf) - } - } -} - func TestPageWriter_PrintImage_PNG_DefaultSize(t *testing.T) { img := image.NewRGBA(image.Rect(0, 0, 3, 2)) data := mustEncodePNG(t, img) @@ -333,39 +257,6 @@ func TestDocWriter_PrintImage_PNG_RGBA_Integration(t *testing.T) { } } -func TestDocWriter_PrintImageFile_PNG_Integration(t *testing.T) { - var buf bytes.Buffer - - dw := NewDocWriter() - dw.SetUnits("in") - dw.NewPage() - actualWidth, actualHeight, err := dw.PrintImageFile("testdata/eidetic.png", 1, 1, floatPtr(2.0), nil) - if err != nil { - t.Fatal(err) - } - expectFdelta(t, 2.0, actualWidth, 0.0001) - if actualHeight <= 0 { - t.Fatalf("expected positive inferred height, got %f", actualHeight) - } - if _, err := dw.WriteTo(&buf); err != nil { - t.Fatal(err) - } - pdf := buf.String() - for _, fragment := range []string{ - "/Subtype /Image", - "/Filter /FlateDecode", - "/ColorSpace /DeviceRGB", - "/XObject <<", - "/Im0 ", - " cm\n", - "/Im0 Do\n", - } { - if !strings.Contains(pdf, fragment) { - t.Fatalf("expected generated PDF to contain %q, got:\n%s", fragment, pdf) - } - } -} - func TestDocWriter_PrintImage_PNG_Golden(t *testing.T) { var buf bytes.Buffer diff --git a/pdf/page_writer.go b/pdf/page_writer.go index 432b831..2c043e0 100644 --- a/pdf/page_writer.go +++ b/pdf/page_writer.go @@ -31,30 +31,30 @@ const ( type PageWriter struct { drawState - autoPath bool - dw *DocWriter - fonts []*font.Font - gw *graphWriter - inGraph bool - inPath bool - inText bool - isClosed bool - keepOrigin bool - last drawState - line *rich_text.RichText - lineHeight float64 - mw *miscWriter - options options.Options - origin Location - page *page - pageHeight float64 - pageWidth float64 - pathStates []pathState - stream bytes.Buffer - tw *textWriter - units *units + autoPath bool + dw *DocWriter + fonts []*font.Font + gw *graphWriter + inGraph bool + inPath bool + inText bool + isClosed bool + keepOrigin bool + last drawState + line *rich_text.RichText + lineHeight float64 + mw *miscWriter + options options.Options + origin Location + page *page + pageHeight float64 + pageWidth float64 + pathStates []pathState + stream bytes.Buffer + tw *textWriter + units *units vTextAlignPts float64 - flushing boolean + flushing boolean } type pathState struct { @@ -1121,6 +1121,41 @@ func (pw *PageWriter) Print(text string) (err error) { return pw.print(text) } +func (pw *PageWriter) PrintImage(data []byte, x, y float64, width, height *float64) (actualWidth, actualHeight float64, err error) { + key := imageKey(data) + image, name, err := pw.dw.loadImage(data, key) + if err != nil { + return 0, 0, err + } + xpts := pw.units.toPts(x) + ypts := pw.units.toPts(y) + info := imageInfo{ + width: image.width, + height: image.height, + bitsPerComponent: image.bitsPerComponent, + } + wpts, hpts := imageSizeInPoints(info, pw.units, width, height) + if pw.inPath { + pw.endPath() + } + if pw.inText { + pw.endText() + } + if pw.inGraph { + pw.endGraph() + } + writeImageXObject(pw.mw, pw.gw, name, xpts, ypts, wpts, hpts, pw.pageHeight) + return pw.units.fromPts(wpts), pw.units.fromPts(hpts), nil +} + +func (pw *PageWriter) PrintImageFile(filename string, x, y float64, width, height *float64) (actualWidth, actualHeight float64, err error) { + data, err := pw.dw.readImageFile(filename) + if err != nil { + return 0, 0, err + } + return pw.PrintImage(data, x, y, width, height) +} + func (pw *PageWriter) print(text string) (err error) { piece, err := pw.richTextForString(text) if err != nil { diff --git a/pdf/page_writer_test.go b/pdf/page_writer_test.go index aa5f077..8ac8ea0 100644 --- a/pdf/page_writer_test.go +++ b/pdf/page_writer_test.go @@ -4,7 +4,9 @@ package pdf import ( + "bytes" "fmt" + "image/jpeg" "strings" "testing" @@ -148,6 +150,49 @@ func TestPageWriter_Line(t *testing.T) { expectS(t, "72 720 m\n216 720 l\nS\n", pw.stream.String()) } +func TestPageWriter_PrintImage_DefaultSize(t *testing.T) { + data := mustReadTestImage(t) + cfg, err := jpeg.DecodeConfig(bytes.NewReader(data)) + if err != nil { + t.Fatal(err) + } + + dw := NewDocWriter() + pw := newPageWriter(dw, options.Options{}) + width, height, err := pw.PrintImage(data, 10, 20, nil, nil) + if err != nil { + t.Fatal(err) + } + if width != float64(cfg.Width) || height != float64(cfg.Height) { + t.Fatalf("expected intrinsic size %dx%d, got %.2fx%.2f", cfg.Width, cfg.Height, width, height) + } + if !strings.Contains(pw.stream.String(), "/Im0 Do\n") { + t.Fatalf("expected image draw operator, got:\n%s", pw.stream.String()) + } + if dw.resources.xObjects["Im0"] == nil { + t.Fatal("expected image XObject registered in resources") + } +} + +func TestPageWriter_PrintImage_AspectRatio(t *testing.T) { + data := mustReadTestImage(t) + cfg, err := jpeg.DecodeConfig(bytes.NewReader(data)) + if err != nil { + t.Fatal(err) + } + + dw := NewDocWriter() + pw := newPageWriter(dw, options.Options{"units": "in"}) + width := 2.0 + actualWidth, actualHeight, err := pw.PrintImage(data, 1, 2, floatPtr(width), nil) + if err != nil { + t.Fatal(err) + } + expectedHeight := width * float64(cfg.Height) / float64(cfg.Width) + expectFdelta(t, width, actualWidth, 0.0001) + expectFdelta(t, expectedHeight, actualHeight, 0.0001) +} + func TestPageWriter_PointsForCircle(t *testing.T) { dw := NewDocWriter() pw := newPageWriter(dw, options.Options{})