From 846b72dccd38322960db6d65e642b14fba7ac19b Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 21 Feb 2025 06:51:17 -0800 Subject: [PATCH] Rethrow permissions error when opening source file --- .../dotty/tools/dotc/util/SourceFile.scala | 8 +---- .../src/dotty/tools/io/AbstractFile.scala | 24 +++++++++----- .../dotty/tools/io/AbstractFileTest.scala | 33 ++++++++++++++++--- 3 files changed, 44 insertions(+), 21 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/util/SourceFile.scala b/compiler/src/dotty/tools/dotc/util/SourceFile.scala index 1c264b395689..da5d20538b93 100644 --- a/compiler/src/dotty/tools/dotc/util/SourceFile.scala +++ b/compiler/src/dotty/tools/dotc/util/SourceFile.scala @@ -275,13 +275,7 @@ object SourceFile { ScriptSourceFile.hasScriptHeader(content) def apply(file: AbstractFile | Null, codec: Codec): SourceFile = - // Files.exists is slow on Java 8 (https://rules.sonarsource.com/java/tag/performance/RSPEC-3725), - // so cope with failure. - val chars = - try new String(file.toByteArray, codec.charSet).toCharArray - catch - case _: FileSystemException => Array.empty[Char] - + val chars = file.toCharArray(using codec.charSet) if isScript(file, chars) then ScriptSourceFile(file, chars) else diff --git a/compiler/src/dotty/tools/io/AbstractFile.scala b/compiler/src/dotty/tools/io/AbstractFile.scala index ee72297c2a4f..95046abc8b39 100644 --- a/compiler/src/dotty/tools/io/AbstractFile.scala +++ b/compiler/src/dotty/tools/io/AbstractFile.scala @@ -6,13 +6,14 @@ package dotty.tools.io import scala.language.unsafeNulls +import scala.io.Codec import java.io.{ IOException, InputStream, OutputStream, BufferedOutputStream, ByteArrayOutputStream } import java.net.URL -import java.nio.file.{FileAlreadyExistsException, Files, Paths} +import java.nio.file.{AccessDeniedException, FileAlreadyExistsException, FileSystemException, Files, Paths} /** * An abstraction over files for use in the reflection/compiler libraries. @@ -112,7 +113,7 @@ abstract class AbstractFile extends Iterable[AbstractFile] { def absolute: AbstractFile /** Returns the containing directory of this abstract file */ - def container : AbstractFile + def container: AbstractFile /** Returns the underlying File if any and null otherwise. */ def file: JFile = try { @@ -159,18 +160,23 @@ abstract class AbstractFile extends Iterable[AbstractFile] { def toURL: URL = if (jpath == null) null else jpath.toUri.toURL - /** Returns contents of file (if applicable) in a Char array. - * warning: use `Global.getSourceFile()` to use the proper - * encoding when converting to the char array. + /** Returns contents of `input` in a Char array. */ @throws(classOf[IOException]) - def toCharArray: Array[Char] = new String(toByteArray).toCharArray + def toCharArray(using codec: Codec): Array[Char] = new String(toByteArray, codec.charSet).toCharArray - /** Returns contents of file (if applicable) in a byte array. + /** Returns contents of `input` in a Byte array. + * + * Files.exists is slow on Java 8 (https://rules.sonarsource.com/java/tag/performance/RSPEC-3725), + * so cope with failure by defaulting to empty array. Rethrow `AccessDeniedException` as helpful. + * See #14664 for limits on probing for other conditions such as path prefix is existing regular file. */ @throws(classOf[IOException]) def toByteArray: Array[Byte] = { - val in = input + val in = + try input catch + case e: AccessDeniedException => throw e + case _: FileSystemException => return Array.empty[Byte] sizeOption match { case Some(size) => var rest = size @@ -186,7 +192,7 @@ abstract class AbstractFile extends Iterable[AbstractFile] { case None => val out = new ByteArrayOutputStream() var c = in.read() - while(c != -1) { + while (c != -1) { out.write(c) c = in.read() } diff --git a/compiler/test/dotty/tools/io/AbstractFileTest.scala b/compiler/test/dotty/tools/io/AbstractFileTest.scala index 1814370d41fe..3d338565ece5 100644 --- a/compiler/test/dotty/tools/io/AbstractFileTest.scala +++ b/compiler/test/dotty/tools/io/AbstractFileTest.scala @@ -1,14 +1,22 @@ -package dotty.tools.io +package dotty.tools +package io import scala.language.unsafeNulls import org.junit.Test +import org.junit.Assert.assertTrue -import dotty.tools.io.AbstractFile -import java.nio.file.Files._ +import java.nio.file.AccessDeniedException +import java.nio.file.Files.* import java.nio.file.attribute.PosixFilePermissions class AbstractFileTest { + import Directory.inTempDirectory + + def permitted(rwx: String = "rwxrwxrwx") = + val permissions = PosixFilePermissions.fromString(rwx) + PosixFilePermissions.asFileAttribute(permissions) + // // Cope with symbolic links. Exercised by -d output. // @@ -43,8 +51,23 @@ class AbstractFileTest { assert(dir.subdirectoryNamed("link").exists) } @Test def t6450(): Unit = - try Directory.inTempDirectory(exerciseSymbolicLinks) - catch { case _: UnsupportedOperationException => () } + try inTempDirectory(exerciseSymbolicLinks) + catch case _: UnsupportedOperationException => () + + @Test def i14664 = inTempDirectory: d => + val f = createTempFile(d.jpath, "i14664", "test", permitted("-w--w--w-")) + val p = PlainFile(d / Path(f)) + assertThrows[AccessDeniedException](_ => true): + p.toCharArray + + @Test def `i14664 ENOENT` = inTempDirectory: d => + val p = PlainFile(d / "random") + assertTrue(p.toCharArray.isEmpty) // would be NoSuchFileException + + @Test def `i14664 ENOTDIR` = inTempDirectory: d => + val f = createTempFile(d.jpath, "i14664", "test", permitted()) + val p = PlainFile(d / Path(f) / "random") + assertTrue(p.toCharArray.isEmpty) // would be FileSystemException } /* Was: