Skip to content

feat(inbound): AttachmentDownloader for provider-hosted attachments#32

Open
mpge wants to merge 1 commit intofeat/inbound-email-orchestrationfrom
feat/attachment-downloader
Open

feat(inbound): AttachmentDownloader for provider-hosted attachments#32
mpge wants to merge 1 commit intofeat/inbound-email-orchestrationfrom
feat/attachment-downloader

Conversation

@mpge
Copy link
Copy Markdown
Member

@mpge mpge commented Apr 24, 2026

Summary

Ports the Go (escalated-go#35) and .NET (escalated-dotnet#29) reference to Spring Boot. Host apps now have a ready-to-wire worker for the Mailgun-style provider-hosted attachments surfaced in `InboundEmailService.ProcessResult.pendingAttachmentDownloads`.

Design

`AttachmentDownloader`

  • `download(pending, ticketId, replyId?)` — HTTP GET via JDK `HttpClient` + persist via `AttachmentStorage` + new row on `AttachmentRepository`.
  • `downloadAll` continues past per-attachment failures so a bad URL doesn't block the rest; returns a `Result` per input (`persisted` or `error` set).
  • `Options.maxBytes` size cap (throws `AttachmentTooLargeException`); `Options.basicAuth` for providers that gate URLs by API key (Mailgun).
  • `safeFilename` uses `Path.getFileName` so `../../etc/passwd` becomes `passwd` — crafted names can't escape the storage root.
  • Response Content-Type fallback when the `PendingAttachment`'s own `contentType` is empty.

`AttachmentStorage` contract + reference `LocalFileAttachmentStorage` that writes to local FS with timestamp+nanos prefixing so concurrent uploads with the same original name don't collide. Host apps with S3 / GCS / Azure can implement `AttachmentStorage` themselves.

Tests

13 Mockito / AssertJ cases:

  • `download`: happy path (ticket + reply attachable targets), 404 throw + no-persist, oversize → `AttachmentTooLargeException`, basic auth header encoding, missing URL guard, response Content-Type fallback.
  • `safeFilename`: `../../etc/passwd`, absolute paths, empty, null, `..`, `.` — all neutralized.
  • `downloadAll`: 3-attachment batch where the middle one fails — other two still persist.
  • `LocalFileAttachmentStorage`: write, null-root rejection, unique path per call.

Uses a `StubResponse` implementing `HttpResponse<byte[]>` directly to avoid the ceremony of standing up MockWebServer for a unit test.

Stacked PR

Based on `feat/inbound-email-orchestration` (#29) so `PendingAttachment` + `ProcessResult` are available. Merge order: #26#27#28#29 → this PR.

Ports the Go (escalated-go#35) and .NET (escalated-dotnet#29) reference
to Spring Boot. Closes the last gap in the Phase-5 inbound pipeline:
host apps now have a ready-to-wire worker for the Mailgun-style
provider-hosted attachments surfaced in
InboundEmailService.ProcessResult.pendingAttachmentDownloads.

AttachmentDownloader
  - download(pending, ticketId, replyId?) — HTTP GET via JDK
    HttpClient + persist via AttachmentStorage + new row on
    AttachmentRepository.
  - downloadAll continues past per-attachment failures so a bad URL
    doesn't block the rest; returns Result per input (persisted or
    error set).
  - Options.maxBytes size cap (throws AttachmentTooLargeException);
    Options.basicAuth for providers that gate URLs by API key.
  - safeFilename uses Path.getFileName so "../../etc/passwd" becomes
    "passwd" — crafted names can't escape the storage root.
  - Response Content-Type fallback when the PendingAttachment's own
    contentType is empty.

AttachmentStorage contract (put) + reference LocalFileAttachmentStorage
that writes to local FS with timestamp+nanos prefixing so concurrent
uploads with the same original name don't collide. Host apps with S3 /
GCS / Azure can implement AttachmentStorage themselves.

13 Mockito/AssertJ tests cover: happy path (ticket + reply attachable
targets), 404 throw + no-persist, oversize → AttachmentTooLargeException,
basic auth header encoding, missing URL guard, response Content-Type
fallback, safeFilename path-traversal + edge cases, downloadAll
partial-failure handling, LocalFileStorage write / null-root rejection /
unique path per call.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant