From 1a154c3e4c2296940e2c3920c664b4999d8dc368 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Sun, 7 Jan 2018 23:18:28 +0100 Subject: [PATCH] Consistently throw FileNotFoundException even for NIO access Issue: SPR-16334 --- .../io/AbstractFileResolvingResource.java | 137 +++++++++--------- .../core/io/FileSystemResource.java | 20 ++- .../core/io/FileUrlResource.java | 29 +++- .../springframework/core/io/PathResource.java | 12 +- .../core/io/PathResourceTests.java | 80 +++++----- .../core/io/ResourceTests.java | 38 +++-- 6 files changed, 186 insertions(+), 130 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 29aec145309..7e03d3f57b1 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 @@ -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. @@ -26,6 +26,7 @@ import java.net.URL; import java.net.URLConnection; import java.nio.channels.FileChannel; import java.nio.channels.ReadableByteChannel; +import java.nio.file.NoSuchFileException; import java.nio.file.StandardOpenOption; import org.springframework.util.ResourceUtils; @@ -42,6 +43,68 @@ import org.springframework.util.ResourceUtils; */ public abstract class AbstractFileResolvingResource extends AbstractResource { + @Override + public boolean exists() { + try { + URL url = getURL(); + if (ResourceUtils.isFileURL(url)) { + // Proceed with file system resolution + return getFile().exists(); + } + else { + // Try a URL connection content-length header + URLConnection con = url.openConnection(); + customizeConnection(con); + HttpURLConnection httpCon = + (con instanceof HttpURLConnection ? (HttpURLConnection) con : null); + if (httpCon != null) { + int code = httpCon.getResponseCode(); + if (code == HttpURLConnection.HTTP_OK) { + return true; + } + else if (code == HttpURLConnection.HTTP_NOT_FOUND) { + return false; + } + } + if (con.getContentLength() >= 0) { + return true; + } + if (httpCon != null) { + // 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(); + return true; + } + } + } + catch (IOException ex) { + return false; + } + } + + @Override + public boolean isReadable() { + try { + URL url = getURL(); + if (ResourceUtils.isFileURL(url)) { + // Proceed with file system resolution + File file = getFile(); + return (file.canRead() && !file.isDirectory()); + } + else { + return true; + } + } + catch (IOException ex) { + return false; + } + } + @Override public boolean isFile() { try { @@ -123,81 +186,20 @@ public abstract class AbstractFileResolvingResource extends AbstractResource { * This implementation returns a FileChannel for the given URI-identified * resource, provided that it refers to a file in the file system. * @since 5.0 - * @see #getFile(URI) + * @see #getFile() */ @Override public ReadableByteChannel readableChannel() throws IOException { - if (isFile()) { + try { + // Try file system channel return FileChannel.open(getFile().toPath(), StandardOpenOption.READ); } - else { + catch (FileNotFoundException | NoSuchFileException ex) { + // Fall back to InputStream adaptation in superclass return super.readableChannel(); } } - - @Override - public boolean exists() { - try { - URL url = getURL(); - if (ResourceUtils.isFileURL(url)) { - // Proceed with file system resolution - return getFile().exists(); - } - else { - // Try a URL connection content-length header - URLConnection con = url.openConnection(); - customizeConnection(con); - HttpURLConnection httpCon = - (con instanceof HttpURLConnection ? (HttpURLConnection) con : null); - if (httpCon != null) { - int code = httpCon.getResponseCode(); - if (code == HttpURLConnection.HTTP_OK) { - return true; - } - else if (code == HttpURLConnection.HTTP_NOT_FOUND) { - return false; - } - } - if (con.getContentLength() >= 0) { - return true; - } - if (httpCon != null) { - // 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(); - return true; - } - } - } - catch (IOException ex) { - return false; - } - } - - @Override - public boolean isReadable() { - try { - URL url = getURL(); - if (ResourceUtils.isFileURL(url)) { - // Proceed with file system resolution - File file = getFile(); - return (file.canRead() && !file.isDirectory()); - } - else { - return true; - } - } - catch (IOException ex) { - return false; - } - } - @Override public long contentLength() throws IOException { URL url = getURL(); @@ -231,7 +233,6 @@ public abstract class AbstractFileResolvingResource extends AbstractResource { return con.getLastModified(); } - /** * Customize the given {@link URLConnection}, obtained in the course of an * {@link #exists()}, {@link #contentLength()} or {@link #lastModified()} call. diff --git a/spring-core/src/main/java/org/springframework/core/io/FileSystemResource.java b/spring-core/src/main/java/org/springframework/core/io/FileSystemResource.java index c7e1276f300..4f17630776b 100644 --- a/spring-core/src/main/java/org/springframework/core/io/FileSystemResource.java +++ b/spring-core/src/main/java/org/springframework/core/io/FileSystemResource.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. @@ -17,6 +17,7 @@ package org.springframework.core.io; import java.io.File; +import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -26,6 +27,7 @@ import java.nio.channels.FileChannel; import java.nio.channels.ReadableByteChannel; import java.nio.channels.WritableByteChannel; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.StandardOpenOption; import org.springframework.util.Assert; @@ -115,12 +117,17 @@ public class FileSystemResource extends AbstractResource implements WritableReso } /** - * This implementation opens a FileInputStream for the underlying file. + * This implementation opens a NIO file stream for the underlying file. * @see java.io.FileInputStream */ @Override public InputStream getInputStream() throws IOException { - return Files.newInputStream(this.file.toPath()); + try { + return Files.newInputStream(this.file.toPath()); + } + catch (NoSuchFileException ex) { + throw new FileNotFoundException(ex.getMessage()); + } } /** @@ -183,7 +190,12 @@ public class FileSystemResource extends AbstractResource implements WritableReso */ @Override public ReadableByteChannel readableChannel() throws IOException { - return FileChannel.open(this.file.toPath(), StandardOpenOption.READ); + try { + return FileChannel.open(this.file.toPath(), StandardOpenOption.READ); + } + catch (NoSuchFileException ex) { + throw new FileNotFoundException(ex.getMessage()); + } } /** diff --git a/spring-core/src/main/java/org/springframework/core/io/FileUrlResource.java b/spring-core/src/main/java/org/springframework/core/io/FileUrlResource.java index f560bd2aef2..7c725f9fe3d 100644 --- a/spring-core/src/main/java/org/springframework/core/io/FileUrlResource.java +++ b/spring-core/src/main/java/org/springframework/core/io/FileUrlResource.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. @@ -26,11 +26,13 @@ import java.nio.channels.WritableByteChannel; import java.nio.file.Files; import java.nio.file.StandardOpenOption; +import org.springframework.lang.Nullable; import org.springframework.util.ResourceUtils; /** * Subclass of {@link UrlResource} which assumes file resolution, to the degree - * of implementing the {@link WritableResource} interface for it. + * of implementing the {@link WritableResource} interface for it. This resource + * variant also caches resolved {@link File} handles from {@link #getFile()}. * *

This is the class resolved by {@link DefaultResourceLoader} for a "file:..." * URL location, allowing a downcast to {@link WritableResource} for it. @@ -44,6 +46,10 @@ import org.springframework.util.ResourceUtils; */ public class FileUrlResource extends UrlResource implements WritableResource { + @Nullable + private volatile File file; + + /** * Create a new {@code FileUrlResource} based on the given URL object. *

Note that this does not enforce "file" as URL protocol. If a protocol @@ -71,11 +77,14 @@ public class FileUrlResource extends UrlResource implements WritableResource { @Override - public Resource createRelative(String relativePath) throws MalformedURLException { - if (relativePath.startsWith("/")) { - relativePath = relativePath.substring(1); + public File getFile() throws IOException { + File file = this.file; + if (file != null) { + return file; } - return new FileUrlResource(new URL(getURL(), relativePath)); + file = super.getFile(); + this.file = file; + return file; } @Override @@ -106,4 +115,12 @@ public class FileUrlResource extends UrlResource implements WritableResource { return FileChannel.open(getFile().toPath(), StandardOpenOption.WRITE); } + @Override + public Resource createRelative(String relativePath) throws MalformedURLException { + if (relativePath.startsWith("/")) { + relativePath = relativePath.substring(1); + } + return new FileUrlResource(new URL(getURL(), relativePath)); + } + } diff --git a/spring-core/src/main/java/org/springframework/core/io/PathResource.java b/spring-core/src/main/java/org/springframework/core/io/PathResource.java index a63c22fb97c..23a947c06d8 100644 --- a/spring-core/src/main/java/org/springframework/core/io/PathResource.java +++ b/spring-core/src/main/java/org/springframework/core/io/PathResource.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. @@ -26,6 +26,7 @@ import java.net.URL; import java.nio.channels.ReadableByteChannel; import java.nio.channels.WritableByteChannel; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.OpenOption; import java.nio.file.Path; import java.nio.file.Paths; @@ -202,7 +203,12 @@ public class PathResource extends AbstractResource implements WritableResource { */ @Override public ReadableByteChannel readableChannel() throws IOException { - return Files.newByteChannel(this.path, StandardOpenOption.READ); + try { + return Files.newByteChannel(this.path, StandardOpenOption.READ); + } + catch (NoSuchFileException ex) { + throw new FileNotFoundException(ex.getMessage()); + } } /** @@ -215,7 +221,7 @@ public class PathResource extends AbstractResource implements WritableResource { } /** - * This implementation returns the underlying File's length. + * This implementation returns the underlying file's length. */ @Override public long contentLength() throws IOException { diff --git a/spring-core/src/test/java/org/springframework/core/io/PathResourceTests.java b/spring-core/src/test/java/org/springframework/core/io/PathResourceTests.java index 12717200fa7..aa3d95b7dc7 100644 --- a/spring-core/src/test/java/org/springframework/core/io/PathResourceTests.java +++ b/spring-core/src/test/java/org/springframework/core/io/PathResourceTests.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. @@ -18,13 +18,13 @@ package org.springframework.core.io; import java.io.File; import java.io.FileNotFoundException; +import java.io.IOException; import java.net.URI; import java.nio.ByteBuffer; import java.nio.channels.ReadableByteChannel; import java.nio.channels.WritableByteChannel; import java.nio.charset.StandardCharsets; import java.nio.file.AccessDeniedException; -import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.Paths; @@ -75,136 +75,136 @@ public class PathResourceTests { @Test - public void nullPath() throws Exception { + public void nullPath() { thrown.expect(IllegalArgumentException.class); thrown.expectMessage("Path must not be null"); new PathResource((Path) null); } @Test - public void nullPathString() throws Exception { + public void nullPathString() { thrown.expect(IllegalArgumentException.class); thrown.expectMessage("Path must not be null"); new PathResource((String) null); } @Test - public void nullUri() throws Exception { + public void nullUri() { thrown.expect(IllegalArgumentException.class); thrown.expectMessage("URI must not be null"); new PathResource((URI) null); } @Test - public void createFromPath() throws Exception { + public void createFromPath() { Path path = Paths.get(TEST_FILE); PathResource resource = new PathResource(path); assertThat(resource.getPath(), equalTo(TEST_FILE)); } @Test - public void createFromString() throws Exception { + public void createFromString() { PathResource resource = new PathResource(TEST_FILE); assertThat(resource.getPath(), equalTo(TEST_FILE)); } @Test - public void createFromUri() throws Exception { + public void createFromUri() { File file = new File(TEST_FILE); PathResource resource = new PathResource(file.toURI()); assertThat(resource.getPath(), equalTo(file.getAbsoluteFile().toString())); } @Test - public void getPathForFile() throws Exception { + public void getPathForFile() { PathResource resource = new PathResource(TEST_FILE); assertThat(resource.getPath(), equalTo(TEST_FILE)); } @Test - public void getPathForDir() throws Exception { + public void getPathForDir() { PathResource resource = new PathResource(TEST_DIR); assertThat(resource.getPath(), equalTo(TEST_DIR)); } @Test - public void fileExists() throws Exception { + public void fileExists() { PathResource resource = new PathResource(TEST_FILE); assertThat(resource.exists(), equalTo(true)); } @Test - public void dirExists() throws Exception { + public void dirExists() { PathResource resource = new PathResource(TEST_DIR); assertThat(resource.exists(), equalTo(true)); } @Test - public void fileDoesNotExist() throws Exception { + public void fileDoesNotExist() { PathResource resource = new PathResource(NON_EXISTING_FILE); assertThat(resource.exists(), equalTo(false)); } @Test - public void fileIsReadable() throws Exception { + public void fileIsReadable() { PathResource resource = new PathResource(TEST_FILE); assertThat(resource.isReadable(), equalTo(true)); } @Test - public void doesNotExistIsNotReadable() throws Exception { + public void doesNotExistIsNotReadable() { PathResource resource = new PathResource(NON_EXISTING_FILE); assertThat(resource.isReadable(), equalTo(false)); } @Test - public void directoryIsNotReadable() throws Exception { + public void directoryIsNotReadable() { PathResource resource = new PathResource(TEST_DIR); assertThat(resource.isReadable(), equalTo(false)); } @Test - public void getInputStream() throws Exception { + public void getInputStream() throws IOException { PathResource resource = new PathResource(TEST_FILE); byte[] bytes = FileCopyUtils.copyToByteArray(resource.getInputStream()); assertThat(bytes.length, greaterThan(0)); } @Test - public void getInputStreamForDir() throws Exception { + public void getInputStreamForDir() throws IOException { PathResource resource = new PathResource(TEST_DIR); thrown.expect(FileNotFoundException.class); resource.getInputStream(); } @Test - public void getInputStreamDoesNotExist() throws Exception { + public void getInputStreamDoesNotExist() throws IOException { PathResource resource = new PathResource(NON_EXISTING_FILE); thrown.expect(FileNotFoundException.class); resource.getInputStream(); } @Test - public void getUrl() throws Exception { + public void getUrl() throws IOException { PathResource resource = new PathResource(TEST_FILE); assertThat(resource.getURL().toString(), Matchers.endsWith("core/io/example.properties")); } @Test - public void getUri() throws Exception { + public void getUri() throws IOException { PathResource resource = new PathResource(TEST_FILE); assertThat(resource.getURI().toString(), Matchers.endsWith("core/io/example.properties")); } @Test - public void getFile() throws Exception { + public void getFile() throws IOException { PathResource resource = new PathResource(TEST_FILE); File file = new File(TEST_FILE); assertThat(resource.getFile().getAbsoluteFile(), equalTo(file.getAbsoluteFile())); } @Test - public void getFileUnsupported() throws Exception { + public void getFileUnsupported() throws IOException { Path path = mock(Path.class); given(path.normalize()).willReturn(path); given(path.toFile()).willThrow(new UnsupportedOperationException()); @@ -214,72 +214,72 @@ public class PathResourceTests { } @Test - public void contentLength() throws Exception { + public void contentLength() throws IOException { PathResource resource = new PathResource(TEST_FILE); File file = new File(TEST_FILE); assertThat(resource.contentLength(), equalTo(file.length())); } @Test - public void contentLengthForDirectory() throws Exception { + public void contentLengthForDirectory() throws IOException { PathResource resource = new PathResource(TEST_DIR); File file = new File(TEST_DIR); assertThat(resource.contentLength(), equalTo(file.length())); } @Test - public void lastModified() throws Exception { + public void lastModified() throws IOException { PathResource resource = new PathResource(TEST_FILE); File file = new File(TEST_FILE); assertThat(resource.lastModified() / 1000, equalTo(file.lastModified() / 1000)); } @Test - public void createRelativeFromDir() throws Exception { + public void createRelativeFromDir() throws IOException { Resource resource = new PathResource(TEST_DIR).createRelative("example.properties"); assertThat(resource, equalTo((Resource) new PathResource(TEST_FILE))); } @Test - public void createRelativeFromFile() throws Exception { + public void createRelativeFromFile() throws IOException { Resource resource = new PathResource(TEST_FILE).createRelative("../example.properties"); assertThat(resource, equalTo((Resource) new PathResource(TEST_FILE))); } @Test - public void filename() throws Exception { + public void filename() { Resource resource = new PathResource(TEST_FILE); assertThat(resource.getFilename(), equalTo("example.properties")); } @Test - public void description() throws Exception { + public void description() { Resource resource = new PathResource(TEST_FILE); assertThat(resource.getDescription(), containsString("path [")); assertThat(resource.getDescription(), containsString(TEST_FILE)); } @Test - public void fileIsWritable() throws Exception { + public void fileIsWritable() { PathResource resource = new PathResource(TEST_FILE); assertThat(resource.isWritable(), equalTo(true)); } @Test - public void directoryIsNotWritable() throws Exception { + public void directoryIsNotWritable() { PathResource resource = new PathResource(TEST_DIR); assertThat(resource.isWritable(), equalTo(false)); } @Test - public void outputStream() throws Exception { + public void outputStream() throws IOException { PathResource resource = new PathResource(temporaryFolder.newFile("test").toPath()); FileCopyUtils.copy("test".getBytes(StandardCharsets.UTF_8), resource.getOutputStream()); assertThat(resource.contentLength(), equalTo(4L)); } @Test - public void doesNotExistOutputStream() throws Exception { + public void doesNotExistOutputStream() throws IOException { File file = temporaryFolder.newFile("test"); file.delete(); PathResource resource = new PathResource(file.toPath()); @@ -288,14 +288,14 @@ public class PathResourceTests { } @Test - public void directoryOutputStream() throws Exception { + public void directoryOutputStream() throws IOException { PathResource resource = new PathResource(TEST_DIR); thrown.expect(FileNotFoundException.class); resource.getOutputStream(); } @Test - public void getReadableByteChannel() throws Exception { + public void getReadableByteChannel() throws IOException { PathResource resource = new PathResource(TEST_FILE); ReadableByteChannel channel = null; try { @@ -313,7 +313,7 @@ public class PathResourceTests { } @Test - public void getReadableByteChannelForDir() throws Exception { + public void getReadableByteChannelForDir() throws IOException { PathResource resource = new PathResource(TEST_DIR); try { resource.readableChannel(); @@ -324,14 +324,14 @@ public class PathResourceTests { } @Test - public void getReadableByteChannelDoesNotExist() throws Exception { + public void getReadableByteChannelDoesNotExist() throws IOException { PathResource resource = new PathResource(NON_EXISTING_FILE); - thrown.expect(NoSuchFileException.class); + thrown.expect(FileNotFoundException.class); resource.readableChannel(); } @Test - public void getWritableChannel() throws Exception { + public void getWritableChannel() throws IOException { PathResource resource = new PathResource(temporaryFolder.newFile("test").toPath()); ByteBuffer buffer = ByteBuffer.wrap("test".getBytes(StandardCharsets.UTF_8)); WritableByteChannel channel = null; 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 c5664cb5727..23fb8201b72 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-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. @@ -201,8 +201,8 @@ public class ResourceTests { return name; } @Override - public InputStream getInputStream() { - return null; + public InputStream getInputStream() throws IOException { + throw new FileNotFoundException(); } }; @@ -211,21 +211,21 @@ public class ResourceTests { fail("FileNotFoundException should have been thrown"); } catch (FileNotFoundException ex) { - assertTrue(ex.getMessage().indexOf(name) != -1); + assertTrue(ex.getMessage().contains(name)); } try { resource.getFile(); fail("FileNotFoundException should have been thrown"); } catch (FileNotFoundException ex) { - assertTrue(ex.getMessage().indexOf(name) != -1); + assertTrue(ex.getMessage().contains(name)); } try { resource.createRelative("/testing"); fail("FileNotFoundException should have been thrown"); } catch (FileNotFoundException ex) { - assertTrue(ex.getMessage().indexOf(name) != -1); + assertTrue(ex.getMessage().contains(name)); } assertThat(resource.getFilename(), nullValue()); @@ -235,19 +235,19 @@ public class ResourceTests { public void testContentLength() throws IOException { AbstractResource resource = new AbstractResource() { @Override - public InputStream getInputStream() throws IOException { + public InputStream getInputStream() { return new ByteArrayInputStream(new byte[] { 'a', 'b', 'c' }); } @Override public String getDescription() { - return null; + return ""; } }; assertThat(resource.contentLength(), is(3L)); } @Test - public void testGetReadableByteChannel() throws IOException { + public void testReadableChannel() throws IOException { Resource resource = new FileSystemResource(getClass().getResource("Resource.class").getFile()); ReadableByteChannel channel = null; try { @@ -264,4 +264,24 @@ public class ResourceTests { } } + @Test(expected = FileNotFoundException.class) + public void testInputStreamNotFoundOnFileSystemResource() throws IOException { + new FileSystemResource(getClass().getResource("Resource.class").getFile()).createRelative("X").getInputStream(); + } + + @Test(expected = FileNotFoundException.class) + public void testReadableChannelNotFoundOnFileSystemResource() throws IOException { + new FileSystemResource(getClass().getResource("Resource.class").getFile()).createRelative("X").readableChannel(); + } + + @Test(expected = FileNotFoundException.class) + public void testInputStreamNotFoundOnClassPathResource() throws IOException { + new ClassPathResource("Resource.class", getClass()).createRelative("X").getInputStream(); + } + + @Test(expected = FileNotFoundException.class) + public void testReadableChannelNotFoundOnClassPathResource() throws IOException { + new ClassPathResource("Resource.class", getClass()).createRelative("X").readableChannel(); + } + }