Skip to content

Commit cdc569a

Browse files
code-ashercode-asher
committed
Catch trying to create a directory at/under non-directory
1 parent 36500b5 commit cdc569a

File tree

3 files changed

+47
-29
lines changed

3 files changed

+47
-29
lines changed

‎src/main/kotlin/com/coder/gateway/CoderSettingsConfigurable.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package com.coder.gateway
22

33
import com.coder.gateway.sdk.CoderCLIManager
4-
import com.coder.gateway.sdk.canWrite
4+
import com.coder.gateway.sdk.canCreateDirectory
55
import com.coder.gateway.services.CoderSettingsState
66
import com.intellij.openapi.components.service
77
import com.intellij.openapi.options.BoundConfigurable
@@ -45,8 +45,8 @@ class CoderSettingsConfigurable : BoundConfigurable("Coder") {
4545
}
4646

4747
private fun validateBinaryDestination(): ValidationInfoBuilder.(JBTextField) -> ValidationInfo? = {
48-
if (it.text.isNotBlank() && !Path.of(it.text).canWrite()) {
49-
error("Cannot write to this path")
48+
if (it.text.isNotBlank() && !Path.of(it.text).canCreateDirectory()) {
49+
error("Cannot create this directory")
5050
} else {
5151
null
5252
}

‎src/main/kotlin/com/coder/gateway/sdk/PathExtensions.kt

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,23 @@ import java.nio.file.Files
44
import java.nio.file.Path
55

66
/**
7-
* Return true if the path can be created.
7+
* Return true if a directory can be created at the specified path or if one
8+
* already exists and we can write into it.
89
*
9-
* Unlike File.canWrite() or Files.isWritable() the file does not need to exist;
10-
* it only needs a writable parent.
10+
* Unlike File.canWrite() or Files.isWritable() the directory does not need to
11+
* exist; it only needs a writable parent and the target needs to be
12+
* non-existent or a directory (not a regular file or nested under one).
13+
*
14+
* This check is deficient on Windows since directories that have write
15+
* permissions but are read-only will still return true.
1116
*/
12-
fun Path.canWrite(): Boolean {
17+
fun Path.canCreateDirectory(): Boolean {
1318
var current: Path? = this.toAbsolutePath()
1419
while (current != null && !Files.exists(current)) {
1520
current = current.parent
1621
}
1722
// On Windows File.canWrite() only checks read-only while Files.isWritable()
18-
// actually checks permissions.
19-
return current != null && Files.isWritable(current)
23+
// also checks permissions. For directories neither of them seem to care if
24+
// it is read-only.
25+
return current != null && Files.isWritable(current) && Files.isDirectory(current)
2026
}

‎src/test/groovy/PathExtensionsTest.groovy

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,35 +12,47 @@ class PathExtensionsTest extends Specification {
1212
@Shared
1313
private Path tmpdir = Path.of(System.getProperty("java.io.tmpdir"))
1414
@Shared
15-
private Path unwritable = tmpdir.resolve("coder-gateway-test/path-extensions/unwritable")
15+
private Path unwritableFile = tmpdir.resolve("coder-gateway-test/path-extensions/unwritable/file")
16+
@Shared
17+
private Path writableFile = tmpdir.resolve("coder-gateway-test/path-extensions/writable-file")
1618

1719
void setupSpec() {
18-
if (unwritable.parent.toFile().exists()) {
19-
unwritable.parent.toFile().setWritable(true)
20-
unwritable.parent.toFile().deleteDir()
20+
// TODO: On Windows setWritable() only sets read-only; how do we set
21+
// actual permissions? Initially I tried an existing dir like WINDIR
22+
// which worked locally but in CI that is writable for some reason.
23+
if (unwritableFile.parent.toFile().exists()) {
24+
unwritableFile.parent.toFile().setWritable(true)
25+
unwritableFile.parent.toFile().deleteDir()
2126
}
22-
Files.createDirectories(unwritable.parent)
23-
unwritable.toFile().write("text")
24-
unwritable.toFile().setWritable(false)
25-
unwritable.parent.toFile().setWritable(false)
27+
Files.createDirectories(unwritableFile.parent)
28+
unwritableFile.toFile().write("text")
29+
writableFile.toFile().write("text")
30+
unwritableFile.toFile().setWritable(false)
31+
unwritableFile.parent.toFile().setWritable(false)
2632
}
2733

28-
def "canWrite"() {
34+
def "canCreateDirectory"() {
2935
expect:
3036
use(PathExtensionsKt) {
31-
path.canWrite() == expected
37+
path.canCreateDirectory() == expected
3238
}
3339

3440
where:
35-
path | expected
36-
unwritable | false
37-
unwritable.resolve("probably/nonexistent") | false
38-
Path.of("relative to project") | true
39-
tmpdir.resolve("./foo/bar/../..") | true
40-
tmpdir | true
41-
tmpdir.resolve("some/nested/non-existent/path") | true
42-
tmpdir.resolve("with space") | true
43-
CoderCLIManager.getConfigDir() | true
44-
CoderCLIManager.getDataDir() | true
41+
path | expected
42+
unwritableFile | false
43+
unwritableFile.resolve("probably/nonexistent") | false
44+
// TODO: Java reports read-only directories on Windows as writable.
45+
unwritableFile.parent.resolve("probably/nonexistent") | System.getProperty("os.name").toLowerCase().contains("windows")
46+
writableFile | false
47+
writableFile.parent | true
48+
writableFile.resolve("nested/under/file") | false
49+
writableFile.parent.resolve("nested/under/dir") | true
50+
Path.of("relative to project") | true
51+
tmpdir.resolve("./foo/bar/../../coder-gateway-test/path-extensions") | true
52+
tmpdir | true
53+
tmpdir.resolve("some/nested/non-existent/path") | true
54+
tmpdir.resolve("with space") | true
55+
CoderCLIManager.getConfigDir() | true
56+
CoderCLIManager.getDataDir() | true
4557
}
4658
}

0 commit comments

Comments
 (0)