jeantil commented on code in PR #2021:
URL: https://github.com/apache/james-project/pull/2021#discussion_r1494364392
##########
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:
The problem with OOM is that they have non local impacts:L an OOM will
affect all of the VM and all of the threads.
While this specific error may be handled in a safe way, the OOM could have
triggered state corruption in other parts of the system that cannot be safely
recovered from.
That's why the recommended behaviour on OOM is to crash the process and
force a restart and I'm not very confident james is safe wrt to this. Akka
actor system with default settings for instance will auto shutdown in OOM
(killing the james pulsar stack in the process)
I read your email about the difficult deployment, but I'm afraid this is not
the right fix for it
--
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]
