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 <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
