GH-3238: Fix Unmarshaller to close File resource

Fixes #3238 * Extract an `InputStream` from a `File` payload in the `UnmarshallingTransformer` before parsing an XML. Close this `InputStream` in the `finally` block to release the file resource **Cherry-pick to 5.2.x, 5.1.x & 4.3.x**
spring-projects · Apr 2, 2020 · 24e6be6 · 24e6be6
1 parent 79f6479
commit 24e6be6
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 29 deletions.
diff --git a/...c/main/java/org/springframework/integration/xml/transformer/UnmarshallingTransformer.java b/...c/main/java/org/springframework/integration/xml/transformer/UnmarshallingTransformer.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,7 +18,9 @@
 
 import java.io.ByteArrayInputStream;
 import java.io.File;
+import java.io.FileInputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.UncheckedIOException;
 
 import javax.xml.transform.Source;
@@ -37,11 +39,10 @@
 
 /**
  * An implementation of {@link org.springframework.integration.transformer.Transformer}
- * that delegates to an OXM
- * {@link Unmarshaller}. Expects the payload to be of type {@link Document},
- * {@link String}, {@link File}, {@link Source} or to have an instance of
- * {@link SourceFactory} that can convert to a {@link Source}. If
- * {@link #alwaysUseSourceFactory} is set to true, then the {@link SourceFactory}
+ * that delegates to an OXM {@link Unmarshaller}.
+ * Expects the payload to be of type {@link Document}, {@link String}, {@link File}, {@link Source}
+ * or to have an instance of {@link SourceFactory} that can convert to a {@link Source}.
+ * If {@link #alwaysUseSourceFactory} is set to true, then the {@link SourceFactory}
  * will be used to create the {@link Source} regardless of payload type.
  * <p>
  * The {@link #alwaysUseSourceFactory} is ignored if payload is
@@ -97,6 +98,7 @@ public String getComponentType() {
 	@Override
 	public Object transformPayload(Object payload) {
 		Source source;
+		InputStream inputStream = null;
 		try {
 			if (this.mimeMessageUnmarshallerHelper != null) {
 				Object result = this.mimeMessageUnmarshallerHelper.maybeUnmarshalMimeMessage(payload);
@@ -115,7 +117,9 @@ else if (payload instanceof byte[]) {
 				source = new StreamSource(new ByteArrayInputStream((byte[]) payload));
 			}
 			else if (payload instanceof File) {
-				source = new StreamSource((File) payload);
+				File file = (File) payload;
+				inputStream = new FileInputStream(file);
+				source = new StreamSource(inputStream, file.toURI().toASCIIString());
 			}
 			else if (payload instanceof Document) {
 				source = new DOMSource((Document) payload);
@@ -135,6 +139,16 @@ else if (payload instanceof Source) {
 		catch (IOException e) {
 			throw new UncheckedIOException("failed to unmarshal payload", e);
 		}
+		finally {
+			if (inputStream != null) {
+				try {
+					inputStream.close();
+				}
+				catch (IOException e) {
+					// Ignore
+				}
+			}
+		}
 	}
 
 	private static class MimeMessageUnmarshallerHelper {

diff --git a/...framework/integration/xml/transformer/jaxbmarshaling/JaxbMarshallingIntegrationTests.java b/...framework/integration/xml/transformer/jaxbmarshaling/JaxbMarshallingIntegrationTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,68 +17,90 @@
 package org.springframework.integration.xml.transformer.jaxbmarshaling;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
 
-import javax.xml.transform.Result;
 import javax.xml.transform.Source;
 import javax.xml.transform.dom.DOMResult;
 
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
 import org.w3c.dom.Document;
 
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.integration.transformer.MessageTransformationException;
+import org.springframework.messaging.Message;
 import org.springframework.messaging.MessageChannel;
 import org.springframework.messaging.PollableChannel;
 import org.springframework.messaging.support.GenericMessage;
-import org.springframework.test.context.ContextConfiguration;
-import org.springframework.test.context.junit4.AbstractJUnit4SpringContextTests;
+import org.springframework.oxm.UnmarshallingFailureException;
+import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
 import org.springframework.xml.transform.StringSource;
 
 /**
  * @author Jonas Partner
+ * @author Artem Bilan
  */
-@ContextConfiguration
-public class JaxbMarshallingIntegrationTests extends AbstractJUnit4SpringContextTests {
+@SpringJUnitConfig
+public class JaxbMarshallingIntegrationTests {
 
-	@Autowired @Qualifier("marshallIn")
+	@Autowired
+	@Qualifier("marshallIn")
 	MessageChannel marshallIn;
 
-	@Autowired @Qualifier("marshallOut")
+	@Autowired
+	@Qualifier("marshallOut")
 	PollableChannel marshalledOut;
 
-	@Autowired @Qualifier("unmarshallIn")
+	@Autowired
+	@Qualifier("unmarshallIn")
 	MessageChannel unmarshallIn;
 
-	@Autowired @Qualifier("unmarshallOut")
+	@Autowired
+	@Qualifier("unmarshallOut")
 	PollableChannel unmarshallOut;
 
+	@TempDir
+	Path tempDirectory;
 
-	@SuppressWarnings("unchecked")
 	@Test
-	public void testMarshalling() throws Exception {
+	public void testMarshalling() {
 		JaxbAnnotatedPerson person = new JaxbAnnotatedPerson();
 		person.setFirstName("john");
-		marshallIn.send(new GenericMessage<Object>(person));
-		GenericMessage<Result> res = (GenericMessage<Result>) marshalledOut.receive(2000);
-		assertThat(res).as("No response recevied").isNotNull();
+		this.marshallIn.send(new GenericMessage<>(person));
+		Message<?> res = this.marshalledOut.receive(2000);
+		assertThat(res).as("No response received").isNotNull();
 		assertThat(res.getPayload() instanceof DOMResult).as("payload was not a DOMResult").isTrue();
 		Document doc = (Document) ((DOMResult) res.getPayload()).getNode();
 		assertThat(doc.getDocumentElement().getLocalName()).as("Wrong name for root element ").isEqualTo("person");
 	}
 
 
-	@SuppressWarnings("unchecked")
 	@Test
-	public void testUnmarshalling() throws Exception {
+	public void testUnmarshalling() {
 		StringSource source = new StringSource("<person><firstname>bob</firstname></person>");
-		unmarshallIn.send(new GenericMessage<Source>(source));
-		GenericMessage<Object> res = (GenericMessage<Object>) unmarshallOut.receive(2000);
+		this.unmarshallIn.send(new GenericMessage<Source>(source));
+		Message<?> res = this.unmarshallOut.receive(2000);
 		assertThat(res).as("No response").isNotNull();
 		assertThat(res.getPayload() instanceof JaxbAnnotatedPerson).as("Not a Person ").isTrue();
 		JaxbAnnotatedPerson person = (JaxbAnnotatedPerson) res.getPayload();
-		assertThat(person.getFirstName()).as("Worng firstname").isEqualTo("bob");
-
+		assertThat(person.getFirstName()).as("Wrong firstname").isEqualTo("bob");
 	}
 
+	@Test
+	public void testFileUnlockedAfterUnmarshallingFailure() throws IOException {
+		Path tempFile = Files.createTempFile(this.tempDirectory, null, null);
+		Files.write(tempFile, "junk".getBytes());
+		assertThatExceptionOfType(MessageTransformationException.class)
+				.isThrownBy(() -> this.unmarshallIn.send(new GenericMessage<>(tempFile.toFile())))
+				.withCauseInstanceOf(UnmarshallingFailureException.class)
+				.withStackTraceContaining("Content is not allowed in prolog.");
+
+		Files.delete(tempFile);
+	}
 
 }