Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@

package org.springframework.http.converter;

import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;

import org.jspecify.annotations.Nullable;

Expand Down Expand Up @@ -147,31 +145,14 @@ protected boolean supportsRepeatableWrites(Resource resource) {


protected void writeContent(Resource resource, HttpOutputMessage outputMessage)
throws IOException, HttpMessageNotWritableException {

// We cannot use try-with-resources here for the InputStream, since we have
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its seems to be just a try catch around the resource operation.

BUILD SUCCESSFUL

image

// custom handling of the close() method in a finally-block.
try {
InputStream in = resource.getInputStream();
try {
OutputStream out = outputMessage.getBody();
in.transferTo(out);
out.flush();
}
catch (NullPointerException ex) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NPE is ignored by try-with-resource.

// ignore, see SPR-13620
}
finally {
try {
in.close();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will be called anyways by try-with-resource.

}
catch (Throwable ex) {
// ignore, see SPR-12999
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes we still ignore but in 1 catch not in 3.

first 2 are now handled by try-with-resource.

}
}
throws HttpMessageNotWritableException {
try (InputStream in = resource.getInputStream()) {
var body = outputMessage.getBody();
in.transferTo(body);
body.flush();
}
catch (FileNotFoundException ex) {
// ignore, see SPR-12999
catch (Throwable ignored) {
Copy link
Author

@Pankraz76 Pankraz76 Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
catch (Throwable ignored) {
catch (Throwable | NullPointerException | FileNotFoundException ignored) {

thats whats going on, right? @bclozel

// see SPR-12999, SPR-13620
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,5 +156,123 @@ public void writeContentInputStreamThrowingNullPointerException() throws Excepti

assertThat(outputMessage.getHeaders().getContentLength()).isEqualTo(0);
}
@Test
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@Test
@Test

checkstyle solution seems broken and costly to obtain such a custom tool not working. better use spot.

void canReadWithNullMediaType() {
assertThat(converter.canRead(Resource.class, null)).isTrue();
}

@Test
void canWriteWithNullMediaType() {
assertThat(converter.canWrite(Resource.class, null)).isTrue();
}

@Test
void shouldReadWithNullContentDisposition() throws IOException {
byte[] body = FileCopyUtils.copyToByteArray(getClass().getResourceAsStream("logo.jpg"));
MockHttpInputMessage inputMessage = new MockHttpInputMessage(body);
inputMessage.getHeaders().setContentType(MediaType.IMAGE_JPEG);
Resource actualResource = converter.read(Resource.class, inputMessage);
assertThat(FileCopyUtils.copyToByteArray(actualResource.getInputStream())).isEqualTo(body);
assertThat(actualResource.getFilename()).isNull();
}

@Test
void getContentLengthWithInputStreamResource() throws IOException {
try (InputStream body = getClass().getResourceAsStream("logo.jpg")) {
InputStreamResource resource = new InputStreamResource(body);
Long contentLength = converter.getContentLength(resource, MediaType.IMAGE_JPEG);
assertThat(contentLength).isNull();
}
}

@Test
void getContentLengthWithByteArrayResource() throws IOException {
byte[] bytes = new byte[100];
ByteArrayResource resource = new ByteArrayResource(bytes);
Long contentLength = converter.getContentLength(resource, MediaType.APPLICATION_OCTET_STREAM);
assertThat(contentLength).isEqualTo(100);
}

@Test
void supportsRepeatableWritesWithInputStreamResource() {
InputStreamResource resource = new InputStreamResource(mock(InputStream.class));
assertThat(converter.supportsRepeatableWrites(resource)).isFalse();
}

@Test
void supportsRepeatableWritesWithByteArrayResource() {
ByteArrayResource resource = new ByteArrayResource(new byte[0]);
assertThat(converter.supportsRepeatableWrites(resource)).isTrue();
}

@Test
void getDefaultContentTypeWithKnownExtension() throws IOException {
Resource resource = new ClassPathResource("logo.jpg", getClass());
MediaType mediaType = converter.getDefaultContentType(resource);
assertThat(mediaType).isEqualTo(MediaType.IMAGE_JPEG);
}

@Test
void getDefaultContentTypeWithUnknownExtension() throws IOException {
Resource resource = new ByteArrayResource(new byte[0]) {
@Override
public String getFilename() {
return "test.unknown";
}
};
MediaType mediaType = converter.getDefaultContentType(resource);
assertThat(mediaType).isEqualTo(MediaType.APPLICATION_OCTET_STREAM);
}

@Test
void getContentLengthWithUnknownLengthShouldReturnNull() throws IOException {
Resource resource = new ByteArrayResource(new byte[0]) {
@Override
public long contentLength() {
return -1;
}
};
assertThat(converter.getContentLength(resource, MediaType.APPLICATION_OCTET_STREAM)).isNull();
}

@Test
void getDefaultContentTypeWithNoFilenameShouldFallbackToOctetStream() {
Resource resource = new ByteArrayResource(new byte[0]) {
@Override
public String getFilename() {
return null;
}
};
assertThat(converter.getDefaultContentType(resource)).isEqualTo(MediaType.APPLICATION_OCTET_STREAM);
}

@Test
void supportsWithNonResourceClassShouldReturnFalse() {
assertThat(converter.canRead(String.class, MediaType.APPLICATION_OCTET_STREAM)).isFalse();
assertThat(converter.canWrite(String.class, MediaType.APPLICATION_OCTET_STREAM)).isFalse();
}

@Test
void shouldWriteWithCustomContentDisposition() throws IOException {
MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
Resource body = new ClassPathResource("logo.jpg", getClass());
ContentDisposition contentDisposition = ContentDisposition.inline()
.filename("custom_logo.jpg")
.build();
outputMessage.getHeaders().setContentDisposition(contentDisposition);

converter.write(body, null, outputMessage);

assertThat(outputMessage.getHeaders().getContentDisposition().getFilename())
.isEqualTo("custom_logo.jpg");
}

@Test
void writeContentWhenInputStreamIsEmpty() throws IOException {
MockHttpOutputMessage outputMessage = new MockHttpOutputMessage();
Resource resource = new ByteArrayResource(new byte[0]);
converter.write(resource, MediaType.APPLICATION_OCTET_STREAM, outputMessage);
assertThat(outputMessage.getBodyAsBytes()).isEmpty();
}

}