Avoid infinite loop in AbstractResource#contentLength

Due to changes made in commit 2fa87a71 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-9161

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

@@ -25,6 +25,7 @@ import java.net.URISyntaxException;
 import java.net.URL;
 
 import org.springframework.core.NestedIOException;
+import org.springframework.util.Assert;
 import org.springframework.util.ResourceUtils;
 
 /**
@@ -112,13 +113,16 @@ public abstract class AbstractResource implements Resource {
 	 * content length. Subclasses will almost always be able to provide
 	 * a more optimal version of this, e.g. checking a File length.
 	 * @see #getInputStream()
+	 * @throws IllegalStateException if {@link #getInputStream()} returns null.
 	 */
 	public long contentLength() throws IOException {
-		InputStream is = getInputStream();
+		InputStream is = this.getInputStream();
+		Assert.state(is != null, "resource input stream must not be null");
 		try {
 			long size = 0;
 			byte[] buf = new byte[255];
-			for (int read = is.read(buf); read != -1;) {
+			int read;
+			while((read = is.read(buf)) != -1) {
 				size += read;
 			}
 			return size;

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

@@ -18,6 +18,7 @@ package org.springframework.core.io;
 
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.URI;
 import java.net.URL;
 
@@ -132,4 +133,9 @@ public interface Resource extends InputStreamSource {
 	 */
 	String getDescription();
 
+	/**
+	 * {@inheritDoc}
+	 * @return the input stream for the underlying resource (must not be {@code null}).
+	 */
+	public InputStream getInputStream() throws IOException;
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2008 the original author or authors.
+ * Copyright 2002-2012 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.
@@ -16,9 +16,6 @@
 
 package org.springframework.core.io;
 
-import static org.hamcrest.CoreMatchers.nullValue;
-import static org.junit.Assert.*;
-
 import java.io.ByteArrayInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -30,8 +27,12 @@ import org.junit.Ignore;
 import org.junit.Test;
 import org.springframework.util.FileCopyUtils;
 
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
+
 /**
  * @author Juergen Hoeller
+ * @author Chris Beams
  * @since 09.09.2004
  */
 public class ResourceTests {
@@ -174,13 +175,11 @@ public class ResourceTests {
 		assertEquals(new UrlResource("file:dir/subdir"), relative);
 	}
 
-	/*
-	 * @Test
+	@Ignore @Test // this test is quite slow. TODO: re-enable with JUnit categories
 	public void testNonFileResourceExists() throws Exception {
 		Resource resource = new UrlResource("http://www.springframework.org");
 		assertTrue(resource.exists());
 	}
-	*/
 
 	@Test
 	public void testAbstractResourceExceptions() throws Exception {
@@ -220,4 +219,30 @@ public class ResourceTests {
 		assertThat(resource.getFilename(), nullValue());
 	}
 
+	@Test
+	public void testContentLength() throws IOException {
+		AbstractResource resource = new AbstractResource() {
+			public InputStream getInputStream() throws IOException {
+				return new ByteArrayInputStream(new byte[] { 'a', 'b', 'c' });
+			}
+			public String getDescription() {
+				return null;
+			}
+		};
+		assertThat(resource.contentLength(), is(3L));
+	}
+
+	@Test(expected=IllegalStateException.class)
+	public void testContentLength_withNullInputStream() throws IOException {
+		AbstractResource resource = new AbstractResource() {
+			public InputStream getInputStream() throws IOException {
+				return null;
+			}
+			public String getDescription() {
+				return null;
+			}
+		};
+		resource.contentLength();
+	}
+
 }