Skip to content

Commit 40a6769

Browse files
committed
Avoid infinite loop in AbstractResource#contentLength
Due to changes made in commit 2fa87a7 for SPR-9118, AbstractResource#contentLength would fall into an infinite loop unless the method were overridden by a subclass (which it is in the majority of use cases). This commit: - fixes the infinite recursion by refactoring to a while loop - asserts that the value returned from #getInputStream is not null in order to avoid NullPointerException - tests both of the above - adds Javadoc to the Resource interface to clearly document that the contract for any implementation is that #getInputStream must not return null Issue: SPR-9163 Backport-Issue: SPR-9161 Backport-Commit: 7ca5fba
1 parent e30d610 commit 40a6769

File tree

3 files changed

+44
-9
lines changed

3 files changed

+44
-9
lines changed

org.springframework.core/src/main/java/org/springframework/core/io/AbstractResource.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.net.URL;
2626

2727
import org.springframework.core.NestedIOException;
28+
import org.springframework.util.Assert;
2829
import org.springframework.util.ResourceUtils;
2930

3031
/**
@@ -112,13 +113,16 @@ public File getFile() throws IOException {
112113
* content length. Subclasses will almost always be able to provide
113114
* a more optimal version of this, e.g. checking a File length.
114115
* @see #getInputStream()
116+
* @throws IllegalStateException if {@link #getInputStream()} returns null.
115117
*/
116118
public long contentLength() throws IOException {
117-
InputStream is = getInputStream();
119+
InputStream is = this.getInputStream();
120+
Assert.state(is != null, "resource input stream must not be null");
118121
try {
119122
long size = 0;
120123
byte[] buf = new byte[255];
121-
for (int read = is.read(buf); read != -1;) {
124+
int read;
125+
while((read = is.read(buf)) != -1) {
122126
size += read;
123127
}
124128
return size;

org.springframework.core/src/main/java/org/springframework/core/io/Resource.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.io.File;
2020
import java.io.IOException;
21+
import java.io.InputStream;
2122
import java.net.URI;
2223
import java.net.URL;
2324

@@ -132,4 +133,9 @@ public interface Resource extends InputStreamSource {
132133
*/
133134
String getDescription();
134135

136+
/**
137+
* {@inheritDoc}
138+
* @return the input stream for the underlying resource (must not be {@code null}).
139+
*/
140+
public InputStream getInputStream() throws IOException;
135141
}

org.springframework.core/src/test/java/org/springframework/core/io/ResourceTests.java

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2008 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,9 +16,6 @@
1616

1717
package org.springframework.core.io;
1818

19-
import static org.hamcrest.CoreMatchers.nullValue;
20-
import static org.junit.Assert.*;
21-
2219
import java.io.ByteArrayInputStream;
2320
import java.io.FileNotFoundException;
2421
import java.io.IOException;
@@ -30,8 +27,12 @@
3027
import org.junit.Test;
3128
import org.springframework.util.FileCopyUtils;
3229

30+
import static org.hamcrest.CoreMatchers.*;
31+
import static org.junit.Assert.*;
32+
3333
/**
3434
* @author Juergen Hoeller
35+
* @author Chris Beams
3536
* @since 09.09.2004
3637
*/
3738
public class ResourceTests {
@@ -174,13 +175,11 @@ public void testUrlResourceWithRelativePath() throws IOException {
174175
assertEquals(new UrlResource("file:dir/subdir"), relative);
175176
}
176177

177-
/*
178-
* @Test
178+
@Ignore @Test // this test is quite slow. TODO: re-enable with JUnit categories
179179
public void testNonFileResourceExists() throws Exception {
180180
Resource resource = new UrlResource("http://www.springframework.org");
181181
assertTrue(resource.exists());
182182
}
183-
*/
184183

185184
@Test
186185
public void testAbstractResourceExceptions() throws Exception {
@@ -220,4 +219,30 @@ public InputStream getInputStream() {
220219
assertThat(resource.getFilename(), nullValue());
221220
}
222221

222+
@Test
223+
public void testContentLength() throws IOException {
224+
AbstractResource resource = new AbstractResource() {
225+
public InputStream getInputStream() throws IOException {
226+
return new ByteArrayInputStream(new byte[] { 'a', 'b', 'c' });
227+
}
228+
public String getDescription() {
229+
return null;
230+
}
231+
};
232+
assertThat(resource.contentLength(), is(3L));
233+
}
234+
235+
@Test(expected=IllegalStateException.class)
236+
public void testContentLength_withNullInputStream() throws IOException {
237+
AbstractResource resource = new AbstractResource() {
238+
public InputStream getInputStream() throws IOException {
239+
return null;
240+
}
241+
public String getDescription() {
242+
return null;
243+
}
244+
};
245+
resource.contentLength();
246+
}
247+
223248
}

0 commit comments

Comments
 (0)