Conversation
Summary by CodeRabbit
WalkthroughPDF inventory reports can now be generated for individual, selected, or all entities. The backend renders themed reports with optional photos and metadata, while authenticated frontend actions open or download the resulting PDFs. ChangesPDF export flow
Sequence Diagram(s)sequenceDiagram
actor User
participant Frontend
participant API
participant PDFExportService
participant BlobStorage
User->>Frontend: Select PDF export
Frontend->>API: Request authenticated PDF export
API->>PDFExportService: Export items with options
PDFExportService->>BlobStorage: Read optional attachments
BlobStorage-->>PDFExportService: Attachment data
PDFExportService-->>API: PDF bytes and filename
API-->>Frontend: PDF response
Frontend-->>User: Open or download report
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
backend/app/api/handlers/v1/v1_ctrl_pdf_export.go (2)
53-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated option parsing across the three export handlers.
The identical
theme/photos/ownerblock appears again at Lines 124-128 and 195-199. A smallpdfOptsFromRequest(r)helper keeps the default-truephotossemantics in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/api/handlers/v1/v1_ctrl_pdf_export.go` around lines 53 - 57, Extract the duplicated theme, photos, and owner query parsing from the three PDF export handlers into a shared pdfOptsFromRequest helper. Update each handler to use this helper, preserving the existing default-true IncludePhotos behavior when the photos parameter is not "false".
20-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHeader-injection stripping is solid; consider RFC 6266
filename*for non-ASCII.CR/LF/quote removal correctly closes the
Content-Dispositioninjection vector from user-controlled asset IDs — nice. Item names/asset IDs can still contain non-ASCII bytes, which some clients mangle in the plainfilename=form; adding afilename*=UTF-8''<pct-encoded>parameter inwritePDFResponsemakes downloads reliable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/api/handlers/v1/v1_ctrl_pdf_export.go` around lines 20 - 28, Update writePDFResponse to include an RFC 6266 filename* parameter alongside the sanitized filename= value, using UTF-8 percent-encoding for non-ASCII names and preserving the existing sanitizeFilename behavior for header-injection protection.backend/app/api/routes.go (1)
179-193: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAuth wiring is correct; consider a rate limiter on the export routes.
All four routes sit behind
userMW(token + tenant + role), so group scoping and authorization are sound. Since a single full-inventory PDF can pull hundreds of DB rows plus every photo blob, wrapping these with a limiter — as done for/notifiers/testviaa.notifierTestLimiter.middlewareon Line 225 — would prevent an authenticated user from turning exports into a cheap DoS.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/api/routes.go` around lines 179 - 193, Add rate-limiter middleware to all PDF export routes in the entities export block, including the single-entity, bulk, full-inventory, and theme-listing handlers, while preserving the existing userMW authorization chain. Reuse the established limiter pattern and existing limiter symbol used by the notifier test route rather than introducing a new limiter implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/app/api/handlers/v1/v1_ctrl_pdf_export.go`:
- Around line 158-192: Update the PDF export flow around QueryByGroup to enforce
PDFExportMaxItems before loading the full inventory: first fetch only enough
data to determine whether the count exceeds the cap (for example, a count or
maxItems+1 result), return the existing 400 error immediately when exceeded,
then perform the full export query only for accepted collections. Preserve the
existing empty-result handling and entity ID collection behavior.
In `@backend/internal/core/services/pdf_export.go`:
- Around line 763-795: The photo grid rendering in the attachment loop should
constrain each image to the fixed imgH cell instead of passing zero height to
pdf.ImageOptions. Update the ImageOptions call to use bounded dimensions that
preserve the image aspect ratio and fit within colW by imgH, keeping the border
aligned with the rendered image area.
- Around line 266-275: Update PDFExportOptions to carry the collection/group
currency, then use that value throughout the PDF export flow—including the total
estimated value and the other currency outputs near the referenced
sections—instead of hardcoding "$". Thread the currency through the relevant
export callers and formatting paths so every monetary value in the document
reflects the collection currency.
- Around line 818-843: Refactor the attachment-read flow in the export method so
the blob bucket is opened and closed once per export rather than inside each
attachment iteration. In the per-attachment read path, limit buffering with
io.LimitReader using MaxImageBytes+1, then reject objects exceeding
MaxImageBytes before retaining their data; preserve the existing error context
and cleanup behavior.
- Around line 152-192: Update ExportMultipleItems to avoid per-item repository
calls by using batched item and maintenance retrieval, while preserving
skipped-invalid-item behavior. Bound the export with a context deadline and
propagate that context through all repository and page-generation work. Change
the PDF generation flow to write directly to the response writer via pdf.Output
rather than buffering the complete document in memory, including attachment
handling used by addItemPages.
- Around line 810-816: Update PDFExportService.readAttachment’s traversal
validation to inspect path components rather than using strings.Contains on the
entire cleaned path. Reject only segments exactly equal to "..", while allowing
legitimate filenames such as "photo..jpg", and preserve the existing
invalid-path error behavior.
- Around line 405-418: Replace the core-font usage in addItemHeaderBar and all
other text-rendering paths in PDFExportService with a consistently registered
embedded UTF-8-compatible font. Ensure the font is registered before rendering
and use its family/style everywhere text is emitted, including item names,
locations, notes, descriptions, warranty details, sold notes, and attachment
titles.
- Around line 428-443: Update the primary-photo embedding flow around
RegisterImageOptionsReader and ImageOptions to reject unsupported MIME types
rather than defaulting them to JPEG, and check for fpdf errors immediately after
registration and placement. Clear any latched pdf error and return false so a
failed image is skipped without allowing the later pdf.Output to fail the entire
report.
In `@backend/internal/sys/config/conf.go`:
- Around line 76-79: Ensure PDFExportMaxItems cannot be zero or negative by
validating it during configuration startup or clamping it to the default value
before export handling. Update the configuration initialization for
PDFExportMaxItems, or the handlers that consume it, while preserving the
existing positive configured value.
In `@frontend/lib/api/classes/items.ts`:
- Around line 258-286: Update UserClient.exportBulkPDF to use the shared
Requests API client instead of raw fetch, ensuring its bearer-token
authentication and standard error handling are applied to /entities/export/pdf
while preserving the existing request body, query parameters, and return shape.
---
Nitpick comments:
In `@backend/app/api/handlers/v1/v1_ctrl_pdf_export.go`:
- Around line 53-57: Extract the duplicated theme, photos, and owner query
parsing from the three PDF export handlers into a shared pdfOptsFromRequest
helper. Update each handler to use this helper, preserving the existing
default-true IncludePhotos behavior when the photos parameter is not "false".
- Around line 20-28: Update writePDFResponse to include an RFC 6266 filename*
parameter alongside the sanitized filename= value, using UTF-8 percent-encoding
for non-ASCII names and preserving the existing sanitizeFilename behavior for
header-injection protection.
In `@backend/app/api/routes.go`:
- Around line 179-193: Add rate-limiter middleware to all PDF export routes in
the entities export block, including the single-entity, bulk, full-inventory,
and theme-listing handlers, while preserving the existing userMW authorization
chain. Reuse the established limiter pattern and existing limiter symbol used by
the notifier test route rather than introducing a new limiter implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 95d6b8d3-21bb-4740-8a9e-768760f87c55
⛔ Files ignored due to path filters (1)
backend/go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
backend/app/api/handlers/v1/v1_ctrl_pdf_export.gobackend/app/api/routes.gobackend/go.modbackend/internal/core/services/pdf_export.gobackend/internal/sys/config/conf.gofrontend/lib/api/classes/items.tsfrontend/locales/en.jsonfrontend/pages/collection/index/tools.vuefrontend/pages/item/[id]/index.vue
| // Query all items (not locations) for the user's group with | ||
| // pagination disabled (-1 = all results) | ||
| itemsOnly := false | ||
| allItems, err := ctrl.repo.Entities.QueryByGroup(ctx, ctx.GID, repo.EntityQuery{ | ||
| IsLocation: &itemsOnly, | ||
| Page: -1, | ||
| PageSize: -1, | ||
| }) | ||
| if err != nil { | ||
| log.Err(err).Msg("failed to query entities for PDF export") | ||
| return validate.NewRequestError(err, http.StatusInternalServerError) | ||
| } | ||
|
|
||
| if len(allItems.Items) == 0 { | ||
| return validate.NewRequestError( | ||
| fmt.Errorf("no items found to export"), | ||
| http.StatusNotFound, | ||
| ) | ||
| } | ||
|
|
||
| // Enforce the configurable export limit to prevent excessive memory | ||
| // usage and timeouts | ||
| maxItems := ctrl.config.Options.PDFExportMaxItems | ||
| if len(allItems.Items) > maxItems { | ||
| return validate.NewRequestError( | ||
| fmt.Errorf("too many items to export (%d); maximum is %d (configurable via HBOX_OPTIONS_PDF_EXPORT_MAX_ITEMS) — use bulk export with specific item IDs instead", len(allItems.Items), maxItems), | ||
| http.StatusBadRequest, | ||
| ) | ||
| } | ||
|
|
||
| // Collect all entity IDs from the query result | ||
| entityIDs := make([]uuid.UUID, len(allItems.Items)) | ||
| for i, item := range allItems.Items { | ||
| entityIDs[i] = item.ID | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Full inventory is loaded before the cap is enforced.
QueryByGroup with PageSize: -1 materializes every entity (with edges) into memory, and only then does the handler reject the request for exceeding maxItems. On a large collection this is a free memory spike for a request that returns 400. Fetch a count (or query maxItems+1 rows) first and bail out before hydrating everything.
Security note: these endpoints are the most expensive authenticated operations in the API and currently have no rate limiter, unlike /notifiers/test. An authenticated user can trivially loop full-inventory exports to exhaust CPU/memory.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/api/handlers/v1/v1_ctrl_pdf_export.go` around lines 158 - 192,
Update the PDF export flow around QueryByGroup to enforce PDFExportMaxItems
before loading the full inventory: first fetch only enough data to determine
whether the count exceeds the cap (for example, a count or maxItems+1 result),
return the existing 400 error immediately when exceeded, then perform the full
export query only for accepted collections. Preserve the existing empty-result
handling and entity ID collection behavior.
| func (svc *PDFExportService) ExportMultipleItems( | ||
| ctx context.Context, groupID uuid.UUID, itemIDs []uuid.UUID, opts PDFExportOptions, | ||
| ) ([]byte, string, error) { | ||
| // Fetch all requested items individually. | ||
| // NOTE: This is an N+1 query pattern. A batch-fetch method (e.g., GetManyByGroup) | ||
| // would be more efficient but does not currently exist in the repository layer. | ||
| // This is acceptable for typical export sizes but should be optimized if exports | ||
| // of hundreds of items become common. | ||
| var items []repo.EntityOut | ||
| for _, id := range itemIDs { | ||
| item, err := svc.repo.Entities.GetOneByGroup(ctx, groupID, id) | ||
| if err != nil { | ||
| log.Warn().Err(err).Str("itemID", id.String()).Msg("skipping item in PDF export") | ||
| continue | ||
| } | ||
| items = append(items, item) | ||
| } | ||
|
|
||
| if len(items) == 0 { | ||
| return nil, "", fmt.Errorf("no valid items found for export") | ||
| } | ||
|
|
||
| theme := getTheme(opts.Theme) | ||
| pdf := fpdf.New("P", "mm", "A4", "") | ||
| pdf.SetAutoPageBreak(true, 20) | ||
|
|
||
| // Cover page | ||
| svc.addCoverPage(pdf, theme, opts, items) | ||
|
|
||
| // Summary page with item table (only for multi-item exports) | ||
| svc.addSummaryPage(pdf, theme, items) | ||
|
|
||
| // Per-item detail pages | ||
| for _, item := range items { | ||
| maintenance, err := svc.repo.MaintEntry.GetMaintenanceByItemID(ctx, groupID, item.ID, repo.MaintenanceFilters{}) | ||
| if err != nil { | ||
| log.Warn().Err(err).Msg("failed to get maintenance for item, continuing") | ||
| maintenance = nil | ||
| } | ||
| svc.addItemPages(ctx, pdf, theme, opts, item, maintenance) | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Bulk export is a request-thread memory and query amplifier.
With the default cap of 500 items this performs ~500 item fetches + ~500 maintenance fetches sequentially, reads every attachment fully into RAM, and buffers the entire PDF before responding. A few concurrent exports can starve the DB pool and spike memory. Consider batching the item/maintenance fetches, streaming pdf.Output directly to the response writer, and bounding the work with a context deadline.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/core/services/pdf_export.go` around lines 152 - 192, Update
ExportMultipleItems to avoid per-item repository calls by using batched item and
maintenance retrieval, while preserving skipped-invalid-item behavior. Bound the
export with a context deadline and propagate that context through all repository
and page-generation work. Change the PDF generation flow to write directly to
the response writer via pdf.Output rather than buffering the complete document
in memory, including attachment handling used by addItemPages.
| if totalValue > 0 { | ||
| pdf.SetY(yPos + 20) | ||
| pdf.SetFont("Helvetica", "B", 16) | ||
| pdf.SetTextColor(theme.HeaderR, theme.HeaderG, theme.HeaderB) | ||
| pdf.CellFormat(pageW, 10, fmt.Sprintf("Total Estimated Value: $%.2f", totalValue), "", 1, "C", false, 0, "") | ||
|
|
||
| pdf.SetFont("Helvetica", "", 12) | ||
| pdf.SetTextColor(80, 80, 80) | ||
| pdf.CellFormat(pageW, 8, fmt.Sprintf("%d of %d items insured", insuredCount, len(items)), "", 1, "C", false, 0, "") | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Currency symbol is hardcoded to $.
Non-USD collections will get incorrect values in a document meant for insurance use. Same pattern repeats at Lines 343, 363, 527, 680, and 709 — threading the group currency through PDFExportOptions fixes all of them in one place.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/core/services/pdf_export.go` around lines 266 - 275, Update
PDFExportOptions to carry the collection/group currency, then use that value
throughout the PDF export flow—including the total estimated value and the other
currency outputs near the referenced sections—instead of hardcoding "$". Thread
the currency through the relevant export callers and formatting paths so every
monetary value in the document reflects the collection currency.
| func (svc *PDFExportService) addItemHeaderBar(pdf *fpdf.Fpdf, theme PDFTheme, item repo.EntityOut, marginL, contentW float64) { | ||
| pageW, _ := pdf.GetPageSize() | ||
| pdf.SetFillColor(theme.HeaderR, theme.HeaderG, theme.HeaderB) | ||
| pdf.Rect(0, 10, pageW, 16, "F") | ||
| pdf.SetTextColor(255, 255, 255) | ||
| pdf.SetFont("Helvetica", "B", theme.HeaderFontSize) | ||
| pdf.SetY(12) | ||
| pdf.SetX(marginL) | ||
| headerText := item.Name | ||
| if !item.AssetID.Nil() { | ||
| headerText = fmt.Sprintf("%s | Asset ID: %s", item.Name, item.AssetID.String()) | ||
| } | ||
| pdf.CellFormat(contentW, 12, headerText, "", 1, "L", false, 0, "") | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm fpdf version and inspect its unicode/UTF-8 font APIs
fd -t f 'go.mod' backend --exec grep -n 'fpdf'
curl -s "https://codeberg.org/api/v1/repos/go-pdf/fpdf/tags" | head -c 2000Repository: sysadminsmedia/homebox
Length of output: 2192
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate pdf_export.go and related helper funcs =="
fd -a 'pdf_export\.go$' .
for f in $(fd 'pdf_export\.go$' .); do
echo "--- outline $f"
ast-grep outline "$f" --view compact || true
echo "--- size $f"
wc -l "$f"
echo "--- text-writing calls in $f"
rg -n 'CellFormat|Cell\(|MultiCell|Text\(|Write\(|UnicodeTranslatorFromDescriptor|AddUTF8Font|SetFont|SetTextColor' "$f" || true
done
echo
echo "== inspect relevant sections =="
for f in $(fd 'pdf_export\.go$' .); do
echo "--- $f"
cat -n "$f" | sed -n '1,560p'
doneRepository: sysadminsmedia/homebox
Length of output: 40550
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== remaining pdf_export.go text sections =="
cat -n backend/internal/core/services/pdf_export.go | sed -n '800,915p'
echo
echo "== repo data model fields for text fields =="
rg -n "type EntityOut|type (Tag|Location|CustomField|Field|Purchase|Warranty|Sold|Note|Description|Entity)|Notes|Tags|Location|Description|WarrantyDetails|PurchaseFrom|SoldNotes|Manufacturer|ModelNumber|SerialNumber|Attachment|Title" backend/internal/data backend/internal/core/services -g '*.go' | head -n 300
echo
echo "== deterministic UTF-8 cp1252 mojibake probe for representative Cyrillic/Accented text =="
python3 - <<'PY'
def cp1252_to_latin1(s):
return s.encode("cp1252").decode("latin1")
samples = [
"Кириллица example",
"中文示例",
"émojis 📦 and café",
]
for s in samples:
b = s.encode("utf-8")
mojibake = b.decode("cp1252")
recovered = mojibake.encode("cp1252").decode("utf-8")
print(s, "=>", mojibake, "recovered:", recovered, "match:", s == recovered)
PYRepository: sysadminsmedia/homebox
Length of output: 42378
🌐 Web query:
go-pdf fpdf core fonts encoding CP1252 UnicodeTranslator FromDescriptor AddUTF8Font
💡 Result:
In the Go FPDF library (typically used via github.com/go-pdf/fpdf or the original jung-kurt/gofpdf), font handling is divided into two primary approaches based on whether you are using legacy, non-UTF-8 encodings or modern UTF-8 TrueType fonts [1][2]. Core Fonts and CP1252 Encoding For standard/non-UTF-8 fonts (such as Type1 or non-UTF-8 TrueType), the library uses font definition files (JSON) and encoding maps [1][2]. 1. UnicodeTranslatorFromDescriptor(encoding string): This method creates a translator function used to convert UTF-8 input strings into the specific character set required by the font [3]. For example, when using a custom font that supports Cyrillic via CP1251, you would use: tr:= pdf.UnicodeTranslatorFromDescriptor("cp1251") pdf.Cell(0, 10, tr("русский текст")) [3] 2. Encoding Files: Files like cp1252.map or cp1251.map are typically used by the makefont utility to generate the necessary JSON font definition files from TrueType or Type1 font files [1][2]. UTF-8 Support For modern requirements, the library supports UTF-8 directly using TrueType fonts, bypassing the need for manual character set translation [1][2]. 1. AddUTF8Font(familyName, style, fileName string): This is the primary method to register a TrueType font for UTF-8 support [1][2]. You do not need to generate a JSON definition file for these fonts; you point directly to the.ttf file [1][2]. 2. Usage: After calling AddUTF8Font, you use SetFont to apply it to your document [1][2]. Note that it is often necessary to specify the directory containing the font files when initializing the PDF object (e.g., fpdf.New("", "", "", fontDir)) to ensure the library can locate them [4]. Summary of Key Differences - UnicodeTranslatorFromDescriptor: Used for legacy compatibility when mapping UTF-8 strings to specific 8-bit encodings (like CP1252 or CP1251) [3]. - AddUTF8Font: The standard, recommended way to handle modern text by loading TrueType fonts directly for full UTF-8 support [1][2]. The method FromDescriptor is generally associated with the translation mechanism for legacy encodings, while AddUTF8Font handles native UTF-8 TrueType font loading [1][3][2].
Citations:
- 1: https://github.com/go-pdf/fpdf/blob/main/doc.go
- 2: https://github.com/jung-kurt/gofpdf
- 3: https://stackoverflow.com/questions/35308271/golang-create-pdf-with-cyrillic
- 4: https://stackoverflow.com/questions/63724418/how-to-specify-font-url-to-gofpdf-font
Use an UTF-8 font for PDF exports.
The export uses Helvetica core fonts, whose strings are interpreted through the font’s encoding (cp1252-family), so Unicode item names, locations, tags, notes, descriptions, warranty details, sold notes, and attachment titles containing non-ASCII UTF-8 render as mojibake. Register an embedded UTF-8 font for the text renderers, or use a UTF-8-compatible font consistently throughout pdf_export.go, not just in addItemHeaderBar.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/core/services/pdf_export.go` around lines 405 - 418, Replace
the core-font usage in addItemHeaderBar and all other text-rendering paths in
PDFExportService with a consistently registered embedded UTF-8-compatible font.
Ensure the font is registered before rendering and use its family/style
everywhere text is emitted, including item names, locations, notes,
descriptions, warranty details, sold notes, and attachment titles.
| imgBytes, imgType, err := svc.readAttachment(ctx, att) | ||
| if err != nil { | ||
| log.Warn().Err(err).Msg("failed to read primary photo for PDF") | ||
| return false | ||
| } | ||
| // Skip images that exceed the size limit to prevent excessive memory usage | ||
| if len(imgBytes) > MaxImageBytes { | ||
| log.Warn().Int("bytes", len(imgBytes)).Msg("primary photo exceeds max image size, skipping embed") | ||
| return false | ||
| } | ||
| // Register the image and place it on the right side, 60mm wide, proportional height | ||
| imgName := fmt.Sprintf("primary_%s", att.ID.String()) | ||
| pdf.RegisterImageOptionsReader(imgName, fpdf.ImageOptions{ImageType: imgType}, bytes.NewReader(imgBytes)) | ||
| imgW := 60.0 | ||
| pdf.ImageOptions(imgName, pageW-marginL-imgW, photoY, imgW, 0, false, fpdf.ImageOptions{ImageType: imgType}, 0, "") | ||
| return true |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify fpdf error-state API surface (Err/Error/ClearError) for the pinned version
rg -n 'fpdf' backend/go.mod backend/go.sum | head -20Repository: sysadminsmedia/homebox
Length of output: 416
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== fpdf dependency files =="
sed -n '1,120p' backend/go.mod
echo
echo "== pdf_export relevant sections =="
cat -n backend/internal/core/services/pdf_export.go | sed -n '410,460p'
echo
cat -n backend/internal/core/services/pdf_export.go | sed -n '840,885p'
echo
echo "== fpdf Err/Error/ClearError docs via github source for v0.12.0 =="
python3 - <<'PY'
import urllib.request
urls = [
"https://github.com/jung-kurt/gofpdf/raw/v0.12.0/fpdf.go",
]
for url in urls:
try:
data = urllib.request.urlopen(url, timeout=20).read().decode("utf-8", "replace")
except Exception as e:
print(f"ERROR {url}: {e}")
continue
lines = data.splitlines()
for target in ["func (f *Fpdf) Err", "func (f *Fpdf) Error", "func (f *Fpdf) CleanUp", "func (f *Fpdf) ClearError"]:
print(f"\n-- {target} --")
for i,l in enumerate(lines):
if target in l:
start=max(0,i-8); end=min(len(lines),i+40)
for n in range(start,end):
print(f"{n+1}\t{lines[n]}")
print()
break
else:
print("not found")
# ImageOptions and RegisterImageOptionsReader also
for target in ["func (f *Fpdf) RegisterImageOptionsReader", "func (f *Fpdf) ImageOptions"]:
print(f"\n-- target {target} --")
for i,l in enumerate(lines):
if target in l:
start=max(0,i-15); end=min(len(lines),i+120)
for n in range(start,end):
print(f"{n+1}\t{lines[n]}")
print()
break
else:
print("not found")
PYRepository: sysadminsmedia/homebox
Length of output: 10763
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== find fpdf module sources in cache =="
find "$GOPATH/pkg/mod" -path '*codeberg.org/go-pdf/fpdf@v0.12.0*' -name 'fpdf.go' 2>/dev/null | head -5 || true
if [ -n "$HOME/go/pkg/mod" ]; then
FIND="$HOME/go/pkg/mod"
elif [ -n "$GOPATH/pkg/mod" ]; then
FIND="$GOPATH/pkg/mod"
else
FIND=""
fi
echo
echo "== inspect fpdf module sources if available =="
if [ -n "$FIND" ]; then
for f in $(find "$FIND" -path '*codeberg.org/go-pdf/fpdf@v0.12.0*' \( -name 'fpdf.go' -o -name 'fpdf.go' -o -name 'image*.go' -o -name 'util.go' \) 2>/dev/null | head -20); do
echo "--- $f ---"
if grep -qE 'func \(f \*Fpdf\) (Err|Error|ClearError|RegisterImageOptionsReader|ImageOptions)' "$f"; then
rg -n -C 10 'func \(f \*Fpdf\) (Err|Error|ClearError|RegisterImageOptionsReader|ImageOptions)' "$f"
fi
done
else
echo "no module cache found"
fi
echo
echo "== inspect image parsing methods =="
python3 - <<'PY'
import urllib.request, ssl
ctx = ssl._create_unverified_context()
urls = {
"pdf_export": "https://raw.githubusercontent.com/sysadminsmedia/homebox/main/backend/internal/core/services/pdf_export.go",
"fpdf": "https://raw.githubusercontent.com/jung-kurt/gofpdf/v0.12.0/fpdf.go",
"image_opts": "https://raw.githubusercontent.com/jung-kurt/gofpdf/v0.12.0/image_opt.go",
}
for name, url in urls.items():
try:
data = urllib.request.urlopen(url, context=ctx, timeout=20).read().decode()
except Exception as e:
print(f"ERROR {url}: {e}")
continue
print(f"\n== {name}: error-state API ==")
for needle in ["func (f *Fpdf) Err", "func (f *Fpdf) Error", "func (f *Fpdf) ClearError",
"func (f *Fpdf) RegisterImageOptionsReader", "func (f *Fpdf) ImageOptions"]:
idx = data.find(needle)
if idx != -1:
print(f"\n--- {needle} ---")
start = data.rfind("\n", 0, max(0, idx-800))
end = data.find("\n}", idx) if "}" in data[idx:] else idx+2500
print(data[start:end] if start != -1 else data[idx:end])
else:
print(f"not found {needle}")
if name != "pdf_export":
print(f"\n== {name}: other image option mentions ==")
for line_no, line in enumerate(data.splitlines(), 1):
if "Err()" in line or "ImageOptions" in line or "RegisterImage" in line or "CleanUp" in line:
print(f"{line_no}: {line}")
PYRepository: sysadminsmedia/homebox
Length of output: 4452
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== RegisterImageOptionsReader body =="
sed -n '3530,3595p' "$HOME/go/pkg/mod/codeberg.org/go-pdf/fpdf@v0.12.0/fpdf.go"
echo
echo "== error-latching helpers and Output callers =="
rg -n -C 4 'SetErrorf|SetError|Err\(\)|Err\(|ClearError|func \(.*Output|func \(.*OutputToString' "$HOME/go/pkg/mod/codeberg.org/go-pdf/fpdf@v0.12.0/fpdf.go" "$HOME/go/pkg/mod/codeberg.org/go-pdf/fpdf@v0.12.0/*.go" | head -220
echo
echo "== fpdf go-version header =="
sed -n '1,30p' "$HOME/go/pkg/mod/codeberg.org/go-pdf/fpdf@v0.12.0/go.mod"Repository: sysadminsmedia/homebox
Length of output: 14774
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== pdf export uses/images output locations =="
rg -n -C 3 'Output\(|addItemPrimaryPhoto|ReadAll|Attachment' backend/internal/core/services/pdf_export.go backend/internal/core/services/*.go
echo
echo "== parse image error-latching helpers =="
rg -n -C 8 'func \(f \*Fpdf\) parse(jpg|png|gif)|generateImageID|Unsupported image|image type should be specified|unsupported image type' "$HOME/go/pkg/mod/codeberg.org/go-pdf/fpdf@v0.12.0"
echo
echo "== behavioral source slice for corrupt JPEG header =="
python3 - <<'PY'
from pathlib import Path
p = Path("/home/jailuser/go/pkg/mod/codeberg.org/go-pdf/fpdf@v0.12.0/fpdf.go")
s = p.read_text()
start = s.find("func (f *Fpdf) parsejpg")
if start == -1:
s2 = Path("/home/jailuser/go/pkg/mod/codeberg.org/go-pdf/fpdf@v0.12.0/parser.go").read_text()
start = s2.find("func (f *Fpdf) parsejpg")
text = s2
else:
text = s
if start != -1:
print(text[start:start+800])
else:
print("parsejpg not found")
PYRepository: sysadminsmedia/homebox
Length of output: 50378
Skip or clear failed image registrations before exporting.
fpdf latches errors after RegisterImageOptionsReader / ImageOptions, so an unsupported MIME that falls through to "jpg" or a corrupt image can delay failure until pdf.Output and return a 500 for the whole report. Clear pdf.Err()/pdf.Error() after registration, and prefer rejecting unknown MIMEs instead of defaulting to JPEG.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/core/services/pdf_export.go` around lines 428 - 443, Update
the primary-photo embedding flow around RegisterImageOptionsReader and
ImageOptions to reject unsupported MIME types rather than defaulting them to
JPEG, and check for fpdf errors immediately after registration and placement.
Clear any latched pdf error and return false so a failed image is skipped
without allowing the later pdf.Output to fail the entire report.
| for i, att := range attachments { | ||
| // Start a new row every 2 images | ||
| col := i % 2 | ||
| if col == 0 && i > 0 { | ||
| pdf.Ln(imgH + 5) | ||
| } | ||
| if col == 0 && pdf.GetY()+imgH > 270 { | ||
| pdf.AddPage() | ||
| } | ||
|
|
||
| imgBytes, imgType, err := svc.readAttachment(ctx, att) | ||
| if err != nil { | ||
| log.Warn().Err(err).Str("attachment", att.ID.String()).Msg("failed to read attachment for photo grid") | ||
| continue | ||
| } | ||
| // Skip images that exceed the size limit to prevent excessive memory usage | ||
| if len(imgBytes) > MaxImageBytes { | ||
| log.Warn().Str("attachment", att.ID.String()).Int("bytes", len(imgBytes)).Msg("image exceeds max size, skipping") | ||
| continue | ||
| } | ||
|
|
||
| imgName := fmt.Sprintf("grid_%s", att.ID.String()) | ||
| pdf.RegisterImageOptionsReader(imgName, fpdf.ImageOptions{ImageType: imgType}, bytes.NewReader(imgBytes)) | ||
|
|
||
| x := marginL + float64(col)*(colW+5) | ||
| y := pdf.GetY() | ||
|
|
||
| pdf.ImageOptions(imgName, x, y, colW, 0, false, fpdf.ImageOptions{ImageType: imgType}, 0, "") | ||
|
|
||
| // Draw a light border around the image | ||
| pdf.SetDrawColor(200, 200, 200) | ||
| pdf.SetLineWidth(0.3) | ||
| pdf.Rect(x, y, colW, imgH, "D") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Auto-height images can overflow the fixed 70mm grid cell.
ImageOptions(..., colW, 0, ...) scales height by aspect ratio, so portrait photos render taller than the imgH border and overlap the following row and caption. Pass a bounded height (or compute the scaled size from the registered ImageInfoType) so images fit the cell.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/core/services/pdf_export.go` around lines 763 - 795, The
photo grid rendering in the attachment loop should constrain each image to the
fixed imgH cell instead of passing zero height to pdf.ImageOptions. Update the
ImageOptions call to use bounded dimensions that preserve the image aspect ratio
and fit within colW by imgH, keeping the border aligned with the rendered image
area.
| func (svc *PDFExportService) readAttachment(ctx context.Context, att repo.ItemAttachment) ([]byte, string, error) { | ||
| // Defensive path traversal check: ensure the attachment path does not | ||
| // contain ".." components that could escape the expected storage directory. | ||
| cleanPath := filepath.ToSlash(filepath.Clean(att.Path)) | ||
| if strings.Contains(cleanPath, "..") { | ||
| return nil, "", fmt.Errorf("invalid attachment path (directory traversal detected): %s", att.Path) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Traversal guard is a substring match — good instinct, blunt instrument.
strings.Contains(cleanPath, "..") also rejects legitimate names like photo..jpg. Since paths are stored server-side, keep the defense but make it segment-based so it only blocks real traversal:
🔒️ Suggested tightening
- cleanPath := filepath.ToSlash(filepath.Clean(att.Path))
- if strings.Contains(cleanPath, "..") {
- return nil, "", fmt.Errorf("invalid attachment path (directory traversal detected): %s", att.Path)
- }
+ cleanPath := path.Clean(filepath.ToSlash(att.Path))
+ if cleanPath == ".." || strings.HasPrefix(cleanPath, "../") || path.IsAbs(cleanPath) {
+ return nil, "", fmt.Errorf("invalid attachment path (directory traversal detected): %s", att.Path)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func (svc *PDFExportService) readAttachment(ctx context.Context, att repo.ItemAttachment) ([]byte, string, error) { | |
| // Defensive path traversal check: ensure the attachment path does not | |
| // contain ".." components that could escape the expected storage directory. | |
| cleanPath := filepath.ToSlash(filepath.Clean(att.Path)) | |
| if strings.Contains(cleanPath, "..") { | |
| return nil, "", fmt.Errorf("invalid attachment path (directory traversal detected): %s", att.Path) | |
| } | |
| func (svc *PDFExportService) readAttachment(ctx context.Context, att repo.ItemAttachment) ([]byte, string, error) { | |
| // Defensive path traversal check: ensure the attachment path does not | |
| // contain ".." components that could escape the expected storage directory. | |
| cleanPath := path.Clean(filepath.ToSlash(att.Path)) | |
| if cleanPath == ".." || strings.HasPrefix(cleanPath, "../") || path.IsAbs(cleanPath) { | |
| return nil, "", fmt.Errorf("invalid attachment path (directory traversal detected): %s", att.Path) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/core/services/pdf_export.go` around lines 810 - 816, Update
PDFExportService.readAttachment’s traversal validation to inspect path
components rather than using strings.Contains on the entire cleaned path. Reject
only segments exactly equal to "..", while allowing legitimate filenames such as
"photo..jpg", and preserve the existing invalid-path error behavior.
| // Open the blob storage bucket using the configured connection string | ||
| bucket, err := blob.OpenBucket(ctx, svc.repo.Attachments.GetConnString()) | ||
| if err != nil { | ||
| return nil, "", fmt.Errorf("failed to open bucket: %w", err) | ||
| } | ||
| defer func() { | ||
| if cerr := bucket.Close(); cerr != nil { | ||
| log.Warn().Err(cerr).Msg("failed to close blob bucket after attachment read") | ||
| } | ||
| }() | ||
|
|
||
| // Read the full file from storage | ||
| reader, err := bucket.NewReader(ctx, svc.repo.Attachments.GetFullPath(att.Path), nil) | ||
| if err != nil { | ||
| return nil, "", fmt.Errorf("failed to read attachment: %w", err) | ||
| } | ||
| defer func() { | ||
| if cerr := reader.Close(); cerr != nil { | ||
| log.Warn().Err(cerr).Str("attachment", att.ID.String()).Msg("failed to close attachment reader") | ||
| } | ||
| }() | ||
|
|
||
| data, err := io.ReadAll(reader) | ||
| if err != nil { | ||
| return nil, "", fmt.Errorf("failed to read attachment data: %w", err) | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Bucket is opened per attachment and the size cap is enforced after the full read.
For a 500-item export with photos this opens/closes a blob bucket hundreds of times, and io.ReadAll loads arbitrarily large objects into memory before MaxImageBytes is ever checked — so the cap doesn't bound peak memory. Open the bucket once per export and use io.LimitReader(reader, MaxImageBytes+1) (or reader.Size()) to reject oversized objects before buffering them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/core/services/pdf_export.go` around lines 818 - 843,
Refactor the attachment-read flow in the export method so the blob bucket is
opened and closed once per export rather than inside each attachment iteration.
In the per-attachment read path, limit buffering with io.LimitReader using
MaxImageBytes+1, then reject objects exceeding MaxImageBytes before retaining
their data; preserve the existing error context and cleanup behavior.
| // PDFExportMaxItems caps the number of items in a single PDF export to | ||
| // prevent excessive memory usage and request timeouts. Raise on powerful | ||
| // systems or lower on constrained ones. | ||
| PDFExportMaxItems int `yaml:"pdf_export_max_items" conf:"default:500"` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
No floor on PDFExportMaxItems.
HBOX_OPTIONS_PDF_EXPORT_MAX_ITEMS=0 (or a negative value) silently rejects every export with "maximum is 0". Either validate at startup or clamp to the default in the handlers when the value is <= 0.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/sys/config/conf.go` around lines 76 - 79, Ensure
PDFExportMaxItems cannot be zero or negative by validating it during
configuration startup or clamping it to the default value before export
handling. Update the configuration initialization for PDFExportMaxItems, or the
handlers that consume it, while preserving the existing positive configured
value.
| async exportBulkPDF( | ||
| itemIds: string[], | ||
| options: { theme?: string; photos?: boolean; owner?: string } = {} | ||
| ): Promise<{ data: Blob | null; error: boolean }> { | ||
| const params: Record<string, string> = {}; | ||
| if (options.theme) params.theme = options.theme; | ||
| if (options.photos === false) params.photos = "false"; | ||
| if (options.owner) params.owner = options.owner; | ||
|
|
||
| const url = route("/entities/export/pdf", params); | ||
|
|
||
| try { | ||
| const response = await fetch(url, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| credentials: "same-origin", | ||
| body: JSON.stringify({ itemIds }), | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| return { data: null, error: true }; | ||
| } | ||
|
|
||
| const blob = await response.blob(); | ||
| return { data: blob, error: false }; | ||
| } catch { | ||
| return { data: null, error: true }; | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Determine how the shared Requests client authenticates, and whether cookie auth is accepted server-side
fd -t f 'requests.ts' frontend --exec cat -n
rg -n -C3 'Authorization|Bearer|hb\.auth|Cookie' frontend/lib/api/base backend/app/api/middleware.goRepository: sysadminsmedia/homebox
Length of output: 7702
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== items.ts exportBulkPDF context =="
fd -t f 'items.ts' frontend/lib/api/classes --exec sh -c 'wc -l "$1"; echo "--- $1"; sed -n "230,300p" "$1"' sh {}
echo
echo "== base api construction/usages =="
rg -n -C3 'new Requests|Requests\(|exportBulkPDF|bulk.*export|/api.*v1|/api/v1|auth' frontend/lib/api frontend --glob '*.{ts,vue}'
echo
echo "== backend auth middleware / export handlers context =="
fd -t f 'middleware\.go|.*export.*|.*item.*' backend/app/api | sed -n '1,120p'
rg -n -C5 'mwAuthToken|GetCookies|access_token|Bulk|Bulk|Export|/api/v1/entities/export|export/pdf|api_keys|StatusUnauthorized' backend/app/apiRepository: sysadminsmedia/homebox
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== export/pdf POST handler and auth chain =="
rg -n -C10 'func \([^)]*\).*\[POST\].*entities.*export|entities/export/pdf|HandleBulk|Bulk|POST.*entities|StatusUnauthorized|ErrUnauthorized|mwAuthToken\(' backend/app/api -g '*.go'
echo
echo "== auth cookie definitions and CSRF/csrf-related middleware =="
rg -n -C5 'SetSecure|SameSite|hb\.auth\.session|hb\.auth\.attachment|csrf|CSRF|X-CSRF|X-XSRF|CSRF|Secure\(' backend frontend -g '*.go' -g '*.ts' -g '*.vue'Repository: sysadminsmedia/homebox
Length of output: 18623
Route bulk PDF requests through the shared API client
UserClient’s Requests instance supplies the bearer token, while exportBulkPDF() uses raw fetch with only same-origin cookies. The endpoint is protected by mwAuthToken, so cookie-authenticated or API-key-authenticated requests are rejected with 401 before export and the caller cannot tell that failure from a server or over-limit error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/lib/api/classes/items.ts` around lines 258 - 286, Update
UserClient.exportBulkPDF to use the shared Requests API client instead of raw
fetch, ensuring its bearer-token authentication and standard error handling are
applied to /entities/export/pdf while preserving the existing request body,
query parameters, and return shape.
1 participant
Summary
Revives #1387 (closed after drifting out of date), rebased onto the new Entities architecture and updated for all review feedback. Implements the feature requested in discussion #735.
Server-side PDF generation for inventory entities, enabling insurance-grade documentation exports directly from Homebox.
codeberg.org/go-pdf/fpdfwith four API endpoints for single-entity, all-entities, bulk, and theme listingtenantquery parameterPDF contents
API endpoints
/api/v1/entities/{id}/export/pdf/api/v1/entities/export/pdf/api/v1/entities/export/pdf/api/v1/entities/export/pdf/themesAll export endpoints support
?theme=,?photos=true|false,?owner=, and?tenant=query parameters.Changes since #1387
Review feedback from @tankerkiller125, all addressed:
HBOX_OPTIONS_PDF_EXPORT_MAX_ITEMS(default 500) instead of a hardcoded constant, so low-power systems can lower it and large homelabs can raise itClose()on the blob bucket and attachment reader are now handled and loggedMigration to current main:
repo.Items/repo.ItemOutported torepo.Entities/repo.EntityOut/items/...to/entities/...IsLocation: false)tenantquery parameter handled by the existing tenant middlewareTest plan
go build ./...),go vetand package tests pass (two pre-existing Windows-only test failures also occur on clean main)