From 69f14a20380e8353cdfdea706ca4e4d415011e5e Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 29 May 2018 21:47:33 +0200 Subject: [PATCH] ClassPathResource.isReadable() checks InputStream (for jar directories) Resource.isReadable() is defined to semantically imply exists() now. Issue: SPR-16832 --- .../io/AbstractFileResolvingResource.java | 37 +++++++++++++++---- .../core/io/AbstractResource.java | 10 ++--- .../org/springframework/core/io/Resource.java | 10 +++-- .../core/io/ClassPathResourceTests.java | 13 ++++++- .../resource/PathResourceResolver.java | 2 +- .../resource/PathResourceResolver.java | 2 +- 6 files changed, 54 insertions(+), 20 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/io/AbstractFileResolvingResource.java b/spring-core/src/main/java/org/springframework/core/io/AbstractFileResolvingResource.java index 7e03d3f57b..bef6bbb8d4 100644 --- a/spring-core/src/main/java/org/springframework/core/io/AbstractFileResolvingResource.java +++ b/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(); - is.close(); + getInputStream().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; } } 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 6beef08a30..8bc3a05bbf 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 @@ -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(); - is.close(); + getInputStream().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(); } /** 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 40e7f0c13c..c3fca9e5ec 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 @@ -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()}. - *

Will be {@code true} for typical resource descriptors; - * note that actual content reading may still fail when attempted. + *

Will be {@code true} for typical resource descriptors that exist + * 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(); } /** diff --git a/spring-core/src/test/java/org/springframework/core/io/ClassPathResourceTests.java b/spring-core/src/test/java/org/springframework/core/io/ClassPathResourceTests.java index 6319742edb..9f16f17efe 100644 --- a/spring-core/src/test/java/org/springframework/core/io/ClassPathResourceTests.java +++ b/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()); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/PathResourceResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/PathResourceResolver.java index 1f49cde6cb..5b754d9b65 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/PathResourceResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/PathResourceResolver.java @@ -109,7 +109,7 @@ public class PathResourceResolver extends AbstractResourceResolver { protected Mono 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); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java index 13de12e493..3125ea1926 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java +++ b/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; }