From 476fba33677b64eb73a19df96048bcdb2d548d00 Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 24 Jan 2024 05:40:16 -0800 Subject: [PATCH] [7.1.0] Fix up permissions error in getInputStream, like we already do for getOutputStream. We're likely reporting permission errors as FileNotFoundException in some cases, which makes debugging more difficult and might have correctness implications. Better yet would be to migrate to the standard java.nio.file.FileSystemException types, but that's a big and scary change, and it will have to wait for another day. PiperOrigin-RevId: 601092545 Change-Id: I1b8777850c946c97e7faf2a47a60255f4e591a3a --- .../devtools/build/lib/vfs/AbstractFileSystem.java | 12 +++++++++--- .../devtools/build/lib/vfs/FileSystemTest.java | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java index c96b8e88d37f4f..986b19e7051622 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/AbstractFileSystem.java @@ -61,6 +61,12 @@ protected InputStream getInputStream(PathFragment path) throws IOException { if (e.getMessage().endsWith("(Interrupted system call)")) { continue; } else { + // FileInputStream throws FileNotFoundException if opening fails for any reason, + // including permissions. Fix it up here. + // TODO(tjgq): Migrate to java.nio. + if (e.getMessage().equals(path + ERR_PERMISSION_DENIED)) { + throw new FileAccessException(e.getMessage()); + } throw e; } } @@ -158,9 +164,9 @@ protected OutputStream getOutputStream(PathFragment path, boolean append, boolea try { return createFileOutputStream(path, append, internal); } catch (FileNotFoundException e) { - // Why does it throw a *FileNotFoundException* if it can't write? - // That does not make any sense! And its in a completely different - // format than in other situations, no less! + // FileOutputStream throws FileNotFoundException if opening fails for any reason, + // including permissions. Fix it up here. + // TODO(tjgq): Migrate to java.nio. if (e.getMessage().equals(path + ERR_PERMISSION_DENIED)) { throw new FileAccessException(e.getMessage()); } diff --git a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java index b7a358d1776662..6316a49ea89bce 100644 --- a/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java +++ b/src/test/java/com/google/devtools/build/lib/vfs/FileSystemTest.java @@ -1304,6 +1304,20 @@ public void testInputAndOutputStream() throws Exception { } } + @Test + public void testInputStreamPermissionError() throws Exception { + assertThat(xFile.exists()).isTrue(); + xFile.setReadable(false); + assertThrows(FileAccessException.class, () -> xFile.getInputStream()); + } + + @Test + public void testOutputStreamPermissionError() throws Exception { + assertThat(xFile.exists()).isTrue(); + xFile.setWritable(false); + assertThrows(FileAccessException.class, () -> xFile.getOutputStream()); + } + @Test public void testFileChannel() throws Exception { byte[] bytes = "abcdefghijklmnoprstuvwxyz".getBytes(StandardCharsets.ISO_8859_1);