chibenwa commented on code in PR #2021:
URL: https://github.com/apache/james-project/pull/2021#discussion_r1494334622
##########
server/blob/blob-aes/src/main/java/org/apache/james/blob/aes/AESBlobStoreDAO.java:
##########
@@ -67,12 +76,53 @@ public FileBackedOutputStream encrypt(InputStream input) {
public InputStream decrypt(InputStream ciphertext) throws IOException {
// We break symmetry and avoid allocating resources like files as we
are not able, in higher level APIs (mailbox) to do resource cleanup.
try {
- return streamingAead.newDecryptingStream(ciphertext,
PBKDF2StreamingAeadFactory.EMPTY_ASSOCIATED_DATA);
+ return ByteStreams.limit(
+ streamingAead.newDecryptingStream(ciphertext,
PBKDF2StreamingAeadFactory.EMPTY_ASSOCIATED_DATA),
+ MAXIMUM_BLOB_SIZE);
} catch (GeneralSecurityException e) {
throw new IOException("Incorrect crypto setup", e);
}
}
+ public Mono<byte[]> decryptReactiveByteSource(ReactiveByteSource
ciphertext, BlobId blobId) {
+ if (ciphertext.getSize() > MAXIMUM_BLOB_SIZE) {
+ throw new RuntimeException(blobId.asString() + " exceeded maximum
blob size");
+ }
+
+ FileBackedOutputStream encryptedContent = new
FileBackedOutputStream(FILE_THRESHOLD_100_KB_READ);
+ WritableByteChannel channel = Channels.newChannel(encryptedContent);
+
+ return Flux.from(ciphertext.getContent())
+ .doOnNext(Throwing.consumer(channel::write))
+ .then(Mono.fromCallable(() -> {
+ try {
+ FileBackedOutputStream decryptedContent = new
FileBackedOutputStream(FILE_THRESHOLD_100_KB_READ);
+ try {
+ CountingOutputStream countingOutputStream = new
CountingOutputStream(decryptedContent);
+ try (InputStream ciphertextStream =
encryptedContent.asByteSource().openStream()) {
+
decrypt(ciphertextStream).transferTo(countingOutputStream);
+ }
+ try (InputStream decryptedStream =
decryptedContent.asByteSource().openStream()) {
+ return IOUtils.toByteArray(decryptedStream,
countingOutputStream.getCount());
+ }
+ } finally {
+ decryptedContent.reset();
+ decryptedContent.close();
+ }
+ } catch (OutOfMemoryError error) {
+ LoggerFactory.getLogger(AESBlobStoreDAO.class)
+ .error("OOM reading {}. Blob size read so far {}
bytes.", blobId.asString(), ciphertext.getSize());
+ // TODO add a JVM option not to do this as this might be
regarded as controversial
Review Comment:
Yes that's it.
Shadowing the OOM is considered a bad practice.
But it crashes all reactor suff... While in practice we were able to recover
most of the time from it.
an option like
`shadow.oom` defaulting to `false`
Give the explanation that on excessive object store read s the JVM *might*
be able to recover if we let it the opportunity.
--
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]
