prosgarz35 opened a new pull request, #3213:
URL: https://github.com/apache/james-project/pull/3213
## Description
### Context & Problem Statement
In `BlobMailRepository`, multiple mail repositories may share the same
underlying BlobStore and bucket. Previously, several critical issues existed in
repository isolation, cleanup, and life-cycle management:
1. **Unbounded `removeAll()` Deletions (Cross-Repository Data Loss):**
`removeAll()` called `listBlobs()` across the entire bucket without
scoping to the repository's path prefix. As a result, invoking `removeAll()` on
a single repository wiped out metadata and MIME blobs belonging to all other
mail repositories sharing the same bucket.
2. **Missing Ownership & Cross-Repository MIME Reference Protection:**
Operations blindly trusted metadata and MIME blob references. If a
corrupted, forged, or cross-referenced metadata entry pointed to MIME blobs
belonging to a different repository, operations like `remove()` or
`removeAll()` could delete foreign MIME data, and `retrieve()` could leak
foreign emails.
3. **MIME Blob Leaks on Mail Overwrite:**
Calling `store(mail)` with an existing `MailKey` overwrote the metadata
entry with new MIME blob IDs but left the superseded header and body MIME blobs
orphaned in the blobstore.
4. **Stream Leaks & Corrupted Metadata Resiliency:**
Metadata decoding did not reliably close underlying streams via
`Using.resource`. Furthermore, non-JSON or schema-mismatched blobs encountered
during enumeration or retrieval could crash operations rather than being
gracefully skipped.
### Proposed Changes
#### 1. Path-Scoped Listing & Verification (`ownedMails` and `readOwned`)
- Scoped metadata blob listing to the repository's prefix using
`listBlobs(bucket, url.getPath.asString() + "/")`.
- Implemented `readOwned(id: MailPartsId)` as a centralized gatekeeper for
all operations (`retrieve`, `remove`, `removeAll`, `store`, `list`, `size`):
- Validates that the metadata payload's mail name matches the expected
blob ID (`mailMetadataBlobIdFactory.of(mail.getName) == id.metadataBlobId`).
- Enforces `isOwnedMimePart`: verifies that both `headerBlobId` and
`bodyBlobId` reside strictly under the repository's expected MIME path prefix
(`<repoPath>/mimeMessagedata/`). Any foreign MIME references are rejected.
- Gracefully ignores missing (`ObjectNotFoundException`), malformed
(`JsonParseException`), or invalid schema (`JsResultException`) metadata
entries.
#### 2. Cross-Repository Isolation & Progress Tracking
- Implemented `removeAll(progressCallback: Consumer[MailKey])` and updated
`removeAll()` to iterate exclusively over `ownedMails()`, deleting MIME parts
followed by metadata blobs, and reporting deleted keys via callback.
- Fixed `size` and `list` to count/iterate only valid, owned mails.
- Updated `remove(key)` to delete blobs only after ownership and MIME prefix
validation pass.
#### 3. Cleanup of Superseded MIME Blobs on Overwrite
- In `store(mc: Mail)`, checked for existing owned metadata before
persisting the new mail.
- If overwriting an existing key, the superseded MIME blobs are
asynchronously deleted with a backoff retry (`Retry.backoff(2,
Duration.ofMillis(50))`). If deletion exhausts retries, a warning is logged
with the specific blob IDs, preventing silent data leakage while keeping the
primary store operation reliable.
#### 4. Safe Resource Handling
- Wrapped `Store.CloseableByteSource` stream decoding in `Using.resource`
within `MailMetadataDecoder` to guarantee stream closure.
---
### Verification & Testing
- Added regression tests in `BlobMailRepositoryTest.java`:
- `removeAllShouldNotAffectOtherRepositories()`: Verifies that clearing
one repository leaves other repositories and their MIME blobs intact.
- `overwriteShouldCleanOldMimeBlobs()`: Verifies that updating an existing
mail cleans up superseded header/body blobs.
- Added comprehensive failure and isolation scenarios in
`BlobMailRepositoryFailureTest.java` and
`blob-mailrepository-original-fixture.json`:
- Malformed and corrupt metadata handling.
- Foreign MIME reference rejection.
- Stream leak prevention tests.
- Successfully verified with `mvn test-compile checkstyle:check -pl
server/mailrepository/mailrepository-blob` (0 Checkstyle violations).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
