ClassPathResource.isReadable() checks InputStream (for jar directories)

Resource.isReadable() is defined to semantically imply exists() now.

Issue: SPR-16832

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

@@ -19,7 +19,6 @@ package org.springframework.core.io;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
-import java.io.InputStream;
 import java.net.HttpURLConnection;
 import java.net.URI;
 import java.net.URL;
@@ -70,19 +69,18 @@ public abstract class AbstractFileResolvingResource extends AbstractResource {
 					return true;
 				}
 				if (httpCon != null) {
-					// no HTTP OK status, and no content-length header: give up
+					// No HTTP OK status, and no content-length header: give up
 					httpCon.disconnect();
 					return false;
 				}
 				else {
 					// Fall back to stream existence: can we open the stream?
-					InputStream is = getInputStream();
+					getInputStream().close();
-					is.close();
 					return true;
 				}
 			}
 		}
-		catch (IOException ex) {
+		catch (Exception ex) {
 			return false;
 		}
 	}
@@ -97,10 +95,33 @@ public abstract class AbstractFileResolvingResource extends AbstractResource {
 				return (file.canRead() && !file.isDirectory());
 			}
 			else {
+				// Try InputStream resolution for jar resources
+				URLConnection con = url.openConnection();
+				customizeConnection(con);
+				if (con instanceof HttpURLConnection) {
+					HttpURLConnection httpCon = (HttpURLConnection) con;
+					int code = httpCon.getResponseCode();
+					if (code != HttpURLConnection.HTTP_OK) {
+						httpCon.disconnect();
+						return false;
+					}
+				}
+				int contentLength = con.getContentLength();
+				if (contentLength > 0) {
+					return true;
+				}
+				else if (contentLength < 0) {
+					return false;
+				}
+				// 0 length: either an empty file or a directory...
+				// On current JDKs, this will trigger an NPE from within the close() call
+				// for directories, only returning true for actual files with 0 length.
+				getInputStream().close();
 				return true;
 			}
 		}
-		catch (IOException ex) {
+		catch (Exception ex) {
+			// Usually an IOException but potentially a NullPointerException (see above)
 			return false;
 		}
 	}
@@ -114,7 +135,7 @@ public abstract class AbstractFileResolvingResource extends AbstractResource {
 			}
 			return ResourceUtils.URL_PROTOCOL_FILE.equals(url.getProtocol());
 		}
-		catch (IOException ex) {
+		catch (Exception ex) {
 			return false;
 		}
 	}
@@ -165,7 +186,7 @@ public abstract class AbstractFileResolvingResource extends AbstractResource {
 			}
 			return ResourceUtils.URL_PROTOCOL_FILE.equals(uri.getScheme());
 		}
-		catch (IOException ex) {
+		catch (Exception ex) {
 			return false;
 		}
 	}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -57,8 +57,7 @@ public abstract class AbstractResource implements Resource {
 		catch (IOException ex) {
 			// Fall back to stream existence: can we open the stream?
 			try {
-				InputStream is = getInputStream();
+				getInputStream().close();
-				is.close();
 				return true;
 			}
 			catch (Throwable isEx) {
@@ -68,11 +67,12 @@ public abstract class AbstractResource implements Resource {
 	}
 
 	/**
-	 * This implementation always returns {@code true}.
+	 * This implementation always returns {@code true} for a resource
+	 * that {@link #exists() exists} (revised as of 5.1).
 	 */
 	@Override
 	public boolean isReadable() {
-		return true;
+		return exists();
 	}
 
 	/**

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -62,14 +62,16 @@ public interface Resource extends InputStreamSource {
 	/**
 	 * Indicate whether the contents of this resource can be read via
 	 * {@link #getInputStream()}.
-	 * <p>Will be {@code true} for typical resource descriptors;
+	 * <p>Will be {@code true} for typical resource descriptors that exist
-	 * note that actual content reading may still fail when attempted.
+	 * since it strictly implies {@link #exists()} semantics as of 5.1.
+	 * Note that actual content reading may still fail when attempted.
 	 * However, a value of {@code false} is a definitive indication
 	 * that the resource content cannot be read.
 	 * @see #getInputStream()
+	 * @see #exists()
 	 */
 	default boolean isReadable() {
-		return true;
+		return exists();
 	}
 
 	/**

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -109,6 +109,17 @@ public class ClassPathResourceTests {
 		assertEquals("/test.html", ((ClassPathResource) new ClassPathResource("", getClass()).createRelative("/test.html")).getPath());
 	}
 
+	@Test
+	public void directoryNotReadable() {
+		Resource fileDir = new ClassPathResource("org/springframework/core");
+		assertTrue(fileDir.exists());
+		assertFalse(fileDir.isReadable());
+
+		Resource jarDir = new ClassPathResource("reactor/core");
+		assertTrue(jarDir.exists());
+		assertFalse(jarDir.isReadable());
+	}
+
 
 	private void assertDescriptionContainsExpectedPath(ClassPathResource resource, String expectedPath) {
 		Matcher matcher = DESCRIPTION_PATTERN.matcher(resource.getDescription());

spring-webflux/src/main/java/org/springframework/web/reactive/resource/PathResourceResolver.java

@@ -109,7 +109,7 @@ public class PathResourceResolver extends AbstractResourceResolver {
 	protected Mono<Resource> getResource(String resourcePath, Resource location) {
 		try {
 			Resource resource = location.createRelative(resourcePath);
-			if (resource.exists() && resource.isReadable()) {
+			if (resource.isReadable()) {
 				if (checkResource(resource, location)) {
 					if (logger.isTraceEnabled()) {
 						logger.trace("Found match: " + resource);

spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java

@@ -183,7 +183,7 @@ public class PathResourceResolver extends AbstractResourceResolver {
 	@Nullable
 	protected Resource getResource(String resourcePath, Resource location) throws IOException {
 		Resource resource = location.createRelative(resourcePath);
-		if (resource.exists() && resource.isReadable()) {
+		if (resource.isReadable()) {
 			if (checkResource(resource, location)) {
 				return resource;
 			}