From 7ca5fba05ff0ef4dcf076303e1cba19c4b771d94 Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Fri, 24 Feb 2012 14:29:28 +0100 Subject: [PATCH] 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 --- .../core/io/AbstractResource.java | 8 +++- .../org/springframework/core/io/Resource.java | 6 +++ .../core/io/ResourceTests.java | 39 +++++++++++++++---- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/io/AbstractResource.java b/spring-core/src/main/java/org/springframework/core/io/AbstractResource.java index 1d48f7fb33..20b44df103 100644 --- a/spring-core/src/main/java/org/springframework/core/io/AbstractResource.java +++ b/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; diff --git a/spring-core/src/main/java/org/springframework/core/io/Resource.java b/spring-core/src/main/java/org/springframework/core/io/Resource.java index f90b13c7ed..0d088b9202 100644 --- a/spring-core/src/main/java/org/springframework/core/io/Resource.java +++ b/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; } diff --git a/spring-core/src/test/java/org/springframework/core/io/ResourceTests.java b/spring-core/src/test/java/org/springframework/core/io/ResourceTests.java index 86c0043d40..469795437c 100644 --- a/spring-core/src/test/java/org/springframework/core/io/ResourceTests.java +++ b/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(); + } + }