chibenwa commented on code in PR #2021:
URL: https://github.com/apache/james-project/pull/2021#discussion_r1494392510
##########
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:
I said it would be controversial :-P
Here it was a bit specific: we were asking for chunks of 50MB memory at once
which was denied, and eventually all context unreferenced and gced (most of the
time).
> I read your email about the difficult deployment, but I'm afraid this is
not the right fix for it
the right fix is of course not getting the OOM in the first place, which we
eventually achieved painfully.
I agree dropping the OOM shadowing.
> 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)
You mean instead here something as easy as calling `System.exit`?
Then K8s would reboot the process and life would go on....
--
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]
