Skip to content
Prev Previous commit
Next Next commit
Catch trying to create a directory at/under non-directory
  • Loading branch information
code-asher committed Apr 21, 2023
commit cdc569a5789444a522f085a451cccc9fe62143f5
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.coder.gateway

import com.coder.gateway.sdk.CoderCLIManager
import com.coder.gateway.sdk.canWrite
import com.coder.gateway.sdk.canCreateDirectory
import com.coder.gateway.services.CoderSettingsState
import com.intellij.openapi.components.service
import com.intellij.openapi.options.BoundConfigurable
Expand Down Expand Up @@ -45,8 +45,8 @@ class CoderSettingsConfigurable : BoundConfigurable("Coder") {
}

private fun validateBinaryDestination(): ValidationInfoBuilder.(JBTextField) -> ValidationInfo? = {
if (it.text.isNotBlank() && !Path.of(it.text).canWrite()) {
error("Cannot write to this path")
if (it.text.isNotBlank() && !Path.of(it.text).canCreateDirectory()) {
error("Cannot create this directory")
} else {
null
}
Expand Down
18 changes: 12 additions & 6 deletions src/main/kotlin/com/coder/gateway/sdk/PathExtensions.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,23 @@ import java.nio.file.Files
import java.nio.file.Path

/**
* Return true if the path can be created.
* Return true if a directory can be created at the specified path or if one
* already exists and we can write into it.
*
* Unlike File.canWrite() or Files.isWritable() the file does not need to exist;
* it only needs a writable parent.
* Unlike File.canWrite() or Files.isWritable() the directory does not need to
* exist; it only needs a writable parent and the target needs to be
* non-existent or a directory (not a regular file or nested under one).
*
* This check is deficient on Windows since directories that have write
* permissions but are read-only will still return true.
*/
fun Path.canWrite(): Boolean {
fun Path.canCreateDirectory(): Boolean {
var current: Path? = this.toAbsolutePath()
while (current != null && !Files.exists(current)) {
current = current.parent
}
// On Windows File.canWrite() only checks read-only while Files.isWritable()
// actually checks permissions.
return current != null && Files.isWritable(current)
// also checks permissions. For directories neither of them seem to care if
// it is read-only.
return current != null && Files.isWritable(current) && Files.isDirectory(current)
}
52 changes: 32 additions & 20 deletions src/test/groovy/PathExtensionsTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -12,35 +12,47 @@ class PathExtensionsTest extends Specification {
@Shared
private Path tmpdir = Path.of(System.getProperty("java.io.tmpdir"))
@Shared
private Path unwritable = tmpdir.resolve("coder-gateway-test/path-extensions/unwritable")
private Path unwritableFile = tmpdir.resolve("coder-gateway-test/path-extensions/unwritable/file")
@Shared
private Path writableFile = tmpdir.resolve("coder-gateway-test/path-extensions/writable-file")

void setupSpec() {
if (unwritable.parent.toFile().exists()) {
unwritable.parent.toFile().setWritable(true)
unwritable.parent.toFile().deleteDir()
// TODO: On Windows setWritable() only sets read-only; how do we set
// actual permissions? Initially I tried an existing dir like WINDIR
// which worked locally but in CI that is writable for some reason.
if (unwritableFile.parent.toFile().exists()) {
unwritableFile.parent.toFile().setWritable(true)
unwritableFile.parent.toFile().deleteDir()
}
Files.createDirectories(unwritable.parent)
unwritable.toFile().write("text")
unwritable.toFile().setWritable(false)
unwritable.parent.toFile().setWritable(false)
Files.createDirectories(unwritableFile.parent)
unwritableFile.toFile().write("text")
writableFile.toFile().write("text")
unwritableFile.toFile().setWritable(false)
unwritableFile.parent.toFile().setWritable(false)
}

def "canWrite"() {
def "canCreateDirectory"() {
expect:
use(PathExtensionsKt) {
path.canWrite() == expected
path.canCreateDirectory() == expected
}

where:
path | expected
unwritable | false
unwritable.resolve("probably/nonexistent") | false
Path.of("relative to project") | true
tmpdir.resolve("./foo/bar/../..") | true
tmpdir | true
tmpdir.resolve("some/nested/non-existent/path") | true
tmpdir.resolve("with space") | true
CoderCLIManager.getConfigDir() | true
CoderCLIManager.getDataDir() | true
path | expected
unwritableFile | false
unwritableFile.resolve("probably/nonexistent") | false
// TODO: Java reports read-only directories on Windows as writable.
unwritableFile.parent.resolve("probably/nonexistent") | System.getProperty("os.name").toLowerCase().contains("windows")
writableFile | false
writableFile.parent | true
writableFile.resolve("nested/under/file") | false
writableFile.parent.resolve("nested/under/dir") | true
Path.of("relative to project") | true
tmpdir.resolve("./foo/bar/../../coder-gateway-test/path-extensions") | true
tmpdir | true
tmpdir.resolve("some/nested/non-existent/path") | true
tmpdir.resolve("with space") | true
CoderCLIManager.getConfigDir() | true
CoderCLIManager.getDataDir() | true
}
}