use try-with-resources statement in ResourceRegionHttpMessageConverter

Signed-off-by: Vincent Potucek <[email protected]>

Author: Vincent Potucek

spring-web/src/main/java/org/springframework/http/converter/ResourceRegionHttpMessageConverter.java

@@ -173,19 +173,10 @@ protected void writeResourceRegion(ResourceRegion region, HttpOutputMessage outp
 		responseHeaders.add("Content-Range", "bytes " + start + '-' + end + '/' + resourceLength);
 		responseHeaders.setContentLength(rangeLength);
 
-		InputStream in = region.getResource().getInputStream();
-		// We cannot use try-with-resources here for the InputStream, since we have
-		// custom handling of the close() method in a finally-block.
-		try {
+		try (InputStream in = region.getResource().getInputStream()) {
 			StreamUtils.copyRange(in, outputMessage.getBody(), start, end);
-		}
-		finally {
-			try {
-				in.close();
-			}
-			catch (IOException ex) {
-				// ignore
-			}
+		} catch (IOException ex) {
+			// ignore
 		}
 	}
 

spring-web/src/test/java/org/springframework/http/converter/ResourceRegionHttpMessageConverterTests.java

@@ -17,6 +17,7 @@
 package org.springframework.http.converter;
 
 import java.io.ByteArrayInputStream;
+import java.io.IOException;
 import java.lang.reflect.Type;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
@@ -27,6 +28,7 @@
 
 import org.springframework.core.ParameterizedTypeReference;
 import org.springframework.core.io.ClassPathResource;
+import org.springframework.core.io.InputStreamResource;
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.support.ResourceRegion;
 import org.springframework.http.HttpHeaders;
@@ -36,6 +38,7 @@
 import org.springframework.web.testfixture.http.MockHttpOutputMessage;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
 
@@ -198,4 +201,174 @@ public void applicationOctetStreamDefaultContentType() throws Exception {
 		assertThat(outputMessage.getBodyAsString(StandardCharsets.UTF_8)).isEqualTo("Spring");
 	}
 
+	@Test
+	void shouldNotWriteForUnsupportedType() {
+		MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
+		Object unsupportedBody = new Object();
+
+		assertThatThrownBy(() -> converter.write(unsupportedBody, null, outputMessage))
+				.isInstanceOfAny(ClassCastException.class, HttpMessageNotWritableException.class);
+	}
+
+	@Test
+	void shouldGetDefaultContentTypeForResourceRegion() {
+		Resource resource = new ClassPathResource("byterangeresource.txt", getClass());
+		ResourceRegion region = new ResourceRegion(resource, 0, 10);
+
+		MediaType contentType = converter.getDefaultContentType(region);
+		assertThat(contentType).isEqualTo(MediaType.TEXT_PLAIN);
+	}
+
+	@Test
+	void shouldGetDefaultOctetStreamContentTypeForUnknownResource() {
+		Resource resource = mock(Resource.class);
+		given(resource.getFilename()).willReturn("unknown.dat");
+		ResourceRegion region = new ResourceRegion(resource, 0, 10);
+
+		MediaType contentType = converter.getDefaultContentType(region);
+		assertThat(contentType).isEqualTo(MediaType.APPLICATION_OCTET_STREAM);
+	}
+
+	@Test
+	void shouldSupportRepeatableWritesForNonInputStreamResource() {
+		Resource resource = new ClassPathResource("byterangeresource.txt", getClass());
+		ResourceRegion region = new ResourceRegion(resource, 0, 10);
+
+		assertThat(converter.supportsRepeatableWrites(region)).isTrue();
+	}
+
+	@Test
+	void shouldNotSupportRepeatableWritesForInputStreamResource() {
+		Resource resource = mock(InputStreamResource.class);
+		ResourceRegion region = new ResourceRegion(resource, 0, 10);
+
+		assertThat(converter.supportsRepeatableWrites(region)).isFalse();
+	}
+
+	@Test
+	void shouldHandleIOExceptionWhenWritingRegion() throws Exception {
+		MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
+		Resource resource = mock(Resource.class);
+		given(resource.contentLength()).willReturn(10L);
+		given(resource.getInputStream()).willThrow(new IOException("Simulated error"));
+		ResourceRegion region = new ResourceRegion(resource, 0, 5);
+
+		// Should not throw exception
+		converter.write(region, MediaType.TEXT_PLAIN, outputMessage);
+
+		// Verify Content-Range header is set correctly
+		assertThat(outputMessage.getHeaders().getFirst(HttpHeaders.CONTENT_RANGE))
+				.isEqualTo("bytes 0-4/10");
+
+		// Verify no content was written due to the IOException
+		assertThat(outputMessage.getBodyAsString(StandardCharsets.UTF_8)).isEmpty();
+	}
+	@Test
+	void shouldHandleIOExceptionWhenWritingRegionCollection() throws Exception {
+		MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
+		Resource resource = mock(Resource.class);
+		given(resource.contentLength()).willReturn(10L);
+		given(resource.getInputStream()).willThrow(new IOException("Simulated error"));
+		ResourceRegion region = new ResourceRegion(resource, 0, 5);
+		List<ResourceRegion> regions = Collections.singletonList(region);
+
+		// Should not throw exception
+		converter.write(regions, MediaType.TEXT_PLAIN, outputMessage);
+
+		assertThat(outputMessage.getHeaders().getContentType().toString())
+				.isEqualTo("text/plain");
+	}
+
+	@Test
+	void shouldHandleNullResourceRegion() {
+		assertThatThrownBy(() -> converter.write(null, null, new MockHttpOutputMessage()))
+				.isInstanceOf(NullPointerException.class);
+	}
+
+	@Test
+	void shouldHandleInvalidRangeBeyondResourceLength() throws Exception {
+		MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
+		Resource resource = new ClassPathResource("byterangeresource.txt", getClass());
+		ResourceRegion region = new ResourceRegion(resource, 35, 10); // Goes beyond resource length
+
+		converter.write(region, MediaType.TEXT_PLAIN, outputMessage);
+
+		assertThat(outputMessage.getHeaders().getFirst(HttpHeaders.CONTENT_RANGE))
+				.isEqualTo("bytes 35-38/39");
+		assertThat(outputMessage.getBodyAsString(StandardCharsets.UTF_8)).hasSize(4);
+	}
+
+	@Test
+	void shouldHandleZeroLengthResourceRegion() throws Exception {
+		MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
+		Resource resource = new ClassPathResource("byterangeresource.txt", getClass());
+		ResourceRegion region = new ResourceRegion(resource, 5, 0);
+
+		converter.write(region, MediaType.TEXT_PLAIN, outputMessage);
+
+		assertThat(outputMessage.getHeaders().getFirst(HttpHeaders.CONTENT_RANGE))
+				.isEqualTo("bytes 5-4/39");
+		assertThat(outputMessage.getBodyAsString(StandardCharsets.UTF_8)).isEmpty();
+	}
+
+	@Test
+	void shouldHandleMultipleResourcesInCollection() throws Exception {
+		MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
+		Resource resource1 = new ClassPathResource("byterangeresource.txt", getClass());
+		Resource resource2 = new ClassPathResource("byterangeresource.txt", getClass());
+		List<ResourceRegion> regions = List.of(
+				new ResourceRegion(resource1, 0, 5),  // "Spring" is 6 bytes (0-5)
+				new ResourceRegion(resource2, 7, 8)  // "Framework" is 8 bytes (7-14)
+		);
+
+		converter.write(regions, MediaType.TEXT_PLAIN, outputMessage);
+
+		String content = outputMessage.getBodyAsString(StandardCharsets.UTF_8);
+
+		// Verify multipart structure
+		assertThat(content).contains("Content-Type: text/plain");
+		assertThat(content).contains("Content-Range: bytes 7-14/39");
+
+		// Verify partial content (note the ranges only include parts of the words)
+		assertThat(content).contains("Sprin");  // First 5 bytes of "Spring" (0-4)
+		assertThat(content).contains("Framewor"); // First 7 bytes of "Framework" (7-13)
+	}
+	@Test
+	void shouldHandleNullContentType() throws Exception {
+		MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
+		Resource resource = new ClassPathResource("byterangeresource.txt", getClass());
+		ResourceRegion region = new ResourceRegion(resource, 0, 5);
+
+		converter.write(region, null, outputMessage);
+
+		assertThat(outputMessage.getHeaders().getContentType()).isEqualTo(MediaType.TEXT_PLAIN);
+	}
+
+	@Test
+	void shouldHandleUnreadableResource() throws Exception {
+		MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
+		Resource resource = mock(Resource.class);
+		given(resource.contentLength()).willReturn(10L);
+		given(resource.getInputStream()).willThrow(new IOException("Cannot read resource"));
+		ResourceRegion region = new ResourceRegion(resource, 0, 5);
+
+		converter.write(region, MediaType.TEXT_PLAIN, outputMessage);
+
+		assertThat(outputMessage.getHeaders().getFirst(HttpHeaders.CONTENT_RANGE))
+				.isEqualTo("bytes 0-4/10");
+		assertThat(outputMessage.getBodyAsString(StandardCharsets.UTF_8)).isEmpty();
+	}
+
+	@Test
+	void shouldHandleCanWriteWithNullType() {
+		assertThat(converter.canWrite(null, null, null)).isFalse();
+	}
+
+	@Test
+	void shouldHandleCanWriteWithNonParameterizedType() {
+		assertThat(converter.canWrite(ResourceRegion.class, null, null)).isTrue();
+		assertThat(converter.canWrite(String.class, null, null)).isFalse();
+	}
+
+
 }