You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by ma...@apache.org on 2023/02/02 22:08:12 UTC
[james-project] 02/02: Ensure InputStream is closed after text extract processing is done to avoid leaks
This is an automated email from the ASF dual-hosted git repository.
matthieu pushed a commit to branch refactorings-1
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit b769e07009bb726cefe29c2299389526633a014e
Author: Matthieu Baechler <ma...@baechler-craftsmanship.fr>
AuthorDate: Thu Feb 2 21:07:18 2023 +0100
Ensure InputStream is closed after text extract processing is done to avoid leaks
---
.../james/mailbox/extractor/TextExtractor.java | 12 ++++-
.../mailbox/extractor/TextExtractorContract.java | 59 ++++++++++++++++++++--
.../store/extractor/DefaultTextExtractor.java | 19 ++++---
.../store/extractor/DefaultTextExtractorTest.java | 19 ++++++-
4 files changed, 96 insertions(+), 13 deletions(-)
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/extractor/TextExtractor.java b/mailbox/api/src/main/java/org/apache/james/mailbox/extractor/TextExtractor.java
index 7628839bd0..7747b8c0f7 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/extractor/TextExtractor.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/extractor/TextExtractor.java
@@ -21,6 +21,7 @@ package org.apache.james.mailbox.extractor;
import java.io.InputStream;
+import com.github.fge.lambdas.Throwing;
import org.apache.james.mailbox.model.ContentType;
import reactor.core.publisher.Mono;
@@ -31,11 +32,18 @@ public interface TextExtractor {
return true;
}
+ /**
+ * This method will close the InputStream argument.
+ */
ParsedContent extractContent(InputStream inputStream, ContentType contentType) throws Exception;
+ /**
+ * This method will close the InputStream argument.
+ */
default Mono<ParsedContent> extractContentReactive(InputStream inputStream, ContentType contentType) {
- return Mono.fromCallable(() -> extractContent(inputStream, contentType))
- .subscribeOn(Schedulers.boundedElastic());
+ return Mono.using(() -> inputStream,
+ stream -> Mono.fromCallable(() -> extractContent(stream, contentType)).subscribeOn(Schedulers.boundedElastic()),
+ Throwing.consumer(InputStream::close).orDoNothing());
}
}
diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/extractor/TextExtractorContract.java b/mailbox/api/src/test/java/org/apache/james/mailbox/extractor/TextExtractorContract.java
index 7fbce4131d..2b88072362 100644
--- a/mailbox/api/src/test/java/org/apache/james/mailbox/extractor/TextExtractorContract.java
+++ b/mailbox/api/src/test/java/org/apache/james/mailbox/extractor/TextExtractorContract.java
@@ -1,6 +1,59 @@
package org.apache.james.mailbox.extractor;
-import static org.junit.jupiter.api.Assertions.*;
-class TextExtractorContract {
-
+import org.apache.james.mailbox.model.ContentType;
+import org.junit.jupiter.api.Test;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+import static org.assertj.core.api.Assertions.catchException;
+import static org.mockito.Mockito.*;
+
+public interface TextExtractorContract {
+
+ TextExtractor testee();
+ ContentType supportedContentType();
+
+ byte[] supportedContent();
+
+ @Test
+ default void extractContentShouldCloseInputStreamOnSuccess() throws Exception {
+ InputStream stream = spy(new ByteArrayInputStream(supportedContent()));
+
+ testee().extractContent(stream, supportedContentType());
+
+ verify(stream).close();
+ }
+
+ @Test
+ default void extractContentShouldCloseInputStreamOnException() throws Exception {
+ InputStream stream = mock(InputStream.class);
+
+ when(stream.read(any(), anyInt(), anyInt())).thenThrow(new IOException(""));
+
+ catchException(() -> testee().extractContent(stream, supportedContentType()));
+
+ verify(stream).close();
+ }
+
+ @Test
+ default void extractContentReactiveShouldCloseInputStreamOnSuccess() throws Exception {
+ InputStream stream = spy(new ByteArrayInputStream(supportedContent()));
+
+ testee().extractContentReactive(stream, supportedContentType()).block();
+
+ verify(stream).close();
+ }
+
+ @Test
+ default void extractContentReactiveShouldCloseInputStreamOnException() throws Exception {
+ InputStream stream = mock(InputStream.class);
+
+ when(stream.read(any(), anyInt(), anyInt())).thenThrow(new IOException(""));
+
+ catchException(() -> testee().extractContentReactive(stream, supportedContentType()).block());
+
+ verify(stream).close();
+ }
}
\ No newline at end of file
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractor.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractor.java
index b2af165023..133eab9653 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractor.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractor.java
@@ -25,6 +25,7 @@ import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.Optional;
+import com.github.fge.lambdas.Throwing;
import org.apache.commons.io.IOUtils;
import org.apache.james.mailbox.extractor.ParsedContent;
import org.apache.james.mailbox.extractor.TextExtractor;
@@ -46,11 +47,13 @@ public class DefaultTextExtractor implements TextExtractor {
@Override
public ParsedContent extractContent(InputStream inputStream, ContentType contentType) throws Exception {
- if (applicable(contentType)) {
- Charset charset = contentType.charset().orElse(StandardCharsets.UTF_8);
- return new ParsedContent(Optional.ofNullable(IOUtils.toString(inputStream, charset)), new HashMap<>());
- } else {
- return new ParsedContent(Optional.empty(), new HashMap<>());
+ try (var input = inputStream) {
+ if (applicable(contentType)) {
+ Charset charset = contentType.charset().orElse(StandardCharsets.UTF_8);
+ return new ParsedContent(Optional.ofNullable(IOUtils.toString(input, charset)), new HashMap<>());
+ } else {
+ return new ParsedContent(Optional.empty(), new HashMap<>());
+ }
}
}
@@ -58,8 +61,10 @@ public class DefaultTextExtractor implements TextExtractor {
public Mono<ParsedContent> extractContentReactive(InputStream inputStream, ContentType contentType) {
if (applicable(contentType)) {
Charset charset = contentType.charset().orElse(StandardCharsets.UTF_8);
- return Mono.fromCallable(() -> new ParsedContent(Optional.ofNullable(IOUtils.toString(inputStream, charset)), new HashMap<>()))
- .subscribeOn(Schedulers.boundedElastic());
+ return Mono.using(() -> inputStream,
+ stream -> Mono.fromCallable(() -> new ParsedContent(Optional.ofNullable(IOUtils.toString(stream, charset)), new HashMap<>()))
+ .subscribeOn(Schedulers.boundedElastic()),
+ Throwing.consumer(InputStream::close).orDoNothing());
} else {
return Mono.just(new ParsedContent(Optional.empty(), new HashMap<>()));
}
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractorTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractorTest.java
index aaac5b987d..954cb86283 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractorTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractorTest.java
@@ -22,15 +22,32 @@ package org.apache.james.mailbox.store.extractor;
import static org.assertj.core.api.Assertions.assertThat;
import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
import org.apache.james.mailbox.extractor.TextExtractor;
+import org.apache.james.mailbox.extractor.TextExtractorContract;
import org.apache.james.mailbox.model.ContentType;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
-class DefaultTextExtractorTest {
+class DefaultTextExtractorTest implements TextExtractorContract {
TextExtractor textExtractor;
+ @Override
+ public TextExtractor testee() {
+ return textExtractor;
+ }
+
+ @Override
+ public ContentType supportedContentType() {
+ return ContentType.of("text/plain");
+ }
+
+ @Override
+ public byte[] supportedContent() {
+ return "foo".getBytes(StandardCharsets.UTF_8);
+ }
+
@BeforeEach
void setUp() {
textExtractor = new DefaultTextExtractor();
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org