Skip to content

Commit

Permalink
TemporaryFolder tweaks
Browse files Browse the repository at this point in the history 
- fixes a security issue reported by Sonar;
- don't expose java.io.File as public interface (almost all tests are
using just java.nio.file.Path;
- cleanups in tests.
  • Loading branch information
@dfa1
dfa1 committed Dec 22, 2023
1 parent ba10437 commit 72932fb
Show file tree
Hide file tree
Showing 8 changed files with 174 additions and 189 deletions.
4 changes: 2 additions & 2 deletions main/src/test/java/hosh/integration/HoshIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -553,14 +553,14 @@ void tooManyScripts() throws Exception {

// simple test infrastructure
private Path givenScript(String... lines) throws IOException {
Path scriptPath = temporaryFolder.newFile("test.hosh").toPath();
Path scriptPath = temporaryFolder.newFile("test.hosh");
Files.write(scriptPath, List.of(lines));
return scriptPath;
}

@SuppressWarnings("SameParameterValue")
private Path givenFolder(String... filenames) throws IOException {
Path folder = temporaryFolder.newFolder("folder").toPath();
Path folder = temporaryFolder.newFolder("folder");
for (String filename : filenames) {
Files.write(folder.resolve(filename), List.of("some content"));
}
Expand Down
188 changes: 91 additions & 97 deletions modules/filesystem/src/test/java/hosh/modules/filesystem/FileSystemModuleTest.java
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion modules/system/src/test/java/hosh/modules/system/SystemModuleTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1438,7 +1438,7 @@ void twoLines() throws IOException {
@SuppressWarnings("unchecked")
@Test
void appendExisting() throws IOException {
Path file = temporaryFolder.newFile("filename").toPath();
Path file = temporaryFolder.newFile("filename");
Files.write(file, List.of("existing line"), StandardCharsets.UTF_8);
given(state.getCwd()).willReturn(temporaryFolder.toPath());
given(in.recv()).willReturn(
Expand Down
31 changes: 17 additions & 14 deletions runtime/src/test/java/hosh/runtime/CommandCompleterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,21 @@
import org.jline.reader.ParsedLine;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.condition.OS;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.BDDMockito.given;

@ExtendWith(MockitoExtension.class)
Expand Down Expand Up @@ -97,16 +100,16 @@ void builtin() {
void builtinOverridesExternal() throws IOException {
given(state.getPath()).willReturn(List.of(temporaryFolder.toPath()));
given(parsedLine.wordIndex()).willReturn(0);
File file = temporaryFolder.newFile("cmd");
assertThat(file.setExecutable(true, true)).isTrue();
Path file = temporaryFolder.newFile(temporaryFolder.toPath(), "cmd");
assertThat(file.toFile().setExecutable(true, true)).isTrue();
given(state.getCommands()).willReturn(Map.of("cmd", () -> command));
List<Candidate> candidates = new ArrayList<>();
sut.complete(lineReader, parsedLine, candidates);
assertThat(candidates)
.hasSize(1)
.allSatisfy(candidate -> {
assertThat(candidate.value()).isEqualTo("cmd");
assertThat(candidate.descr()).isEqualTo("built-in, overrides " + file.getAbsolutePath());
assertThat(candidate.descr()).isEqualTo("built-in, overrides " + file.toAbsolutePath());
});
}

Expand All @@ -123,8 +126,8 @@ void pathWithEmptyDir() {
void pathWithExecutable() throws IOException {
given(state.getPath()).willReturn(List.of(temporaryFolder.toPath()));
given(parsedLine.wordIndex()).willReturn(0);
File file = temporaryFolder.newFile("cmd");
assertThat(file.setExecutable(true, true)).isTrue();
Path file = temporaryFolder.newFile(temporaryFolder.toPath(), "cmd");
assertThat(file.toFile().setExecutable(true, true)).isTrue();
List<Candidate> candidates = new ArrayList<>();
sut.complete(lineReader, parsedLine, candidates);
assertThat(candidates)
Expand All @@ -138,29 +141,29 @@ void pathWithExecutable() throws IOException {
@Test
void pathWithExecutableAndNonEmptyLine() throws IOException {
given(parsedLine.wordIndex()).willReturn(1); // skipping autocomplete
File file = temporaryFolder.newFile("cmd");
assertThat(file.setExecutable(true, true)).isTrue();
Path file = temporaryFolder.newFile(temporaryFolder.toPath(), "cmd");
assertThat(file.toFile().setExecutable(true, true)).isTrue();
List<Candidate> candidates = new ArrayList<>();
sut.complete(lineReader, parsedLine, candidates);
assertThat(candidates).isEmpty();
}

@Test
void skipNonInPathDirectory() throws IOException {
File file = temporaryFolder.newFile();
given(state.getPath()).willReturn(List.of(file.toPath()));
Path file = temporaryFolder.newFile(temporaryFolder.toPath(), "aaa");
given(state.getPath()).willReturn(List.of(file));
given(parsedLine.wordIndex()).willReturn(0);
List<Candidate> candidates = new ArrayList<>();
sut.complete(lineReader, parsedLine, candidates);
assertThat(candidates).isEmpty();
}

@Test
@DisabledOnOs(OS.WINDOWS)
void ioErrorsAreIgnored() throws IOException {
File bin = temporaryFolder.newFolder("bin");
bin.setExecutable(false);
bin.setReadable(false); // throws java.nio.file.AccessDeniedException
given(state.getPath()).willReturn(List.of(bin.toPath().toAbsolutePath()));
Path bin = temporaryFolder.newFolder("bin");
assertTrue(bin.toFile().setReadable(false)); // throws java.nio.file.AccessDeniedException, does not work on windows
given(state.getPath()).willReturn(List.of(bin.toAbsolutePath()));
given(parsedLine.wordIndex()).willReturn(0);
List<Candidate> candidates = new ArrayList<>();
sut.complete(lineReader, parsedLine, candidates);
Expand Down
52 changes: 26 additions & 26 deletions runtime/src/test/java/hosh/runtime/CommandResolversTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collections;
import java.util.List;
Expand All @@ -57,7 +57,7 @@ class CommandResolversTest {
class BuiltinsThenSystemTest {

@RegisterExtension
final TemporaryFolder folder = new TemporaryFolder();
final TemporaryFolder temporaryFolder = new TemporaryFolder();

@Mock(stubOnly = true)
Command command;
Expand Down Expand Up @@ -90,25 +90,24 @@ void builtin() {

@Test
void validAbsolutePath() throws IOException {
File file = folder.newFile("test");
assertThat(file.setExecutable(true)).isTrue();
Path file = temporaryFolder.newExecutableFile("test");
given(state.getCommands()).willReturn(Collections.emptyMap());
Optional<Command> result = sut.tryResolve(file.getAbsolutePath());
Optional<Command> result = sut.tryResolve(file.toAbsolutePath().toString());
assertThat(result).isPresent();
}

@Test
void invalidAbsolutePath() throws IOException {
File file = folder.newFile("test");
Path file = temporaryFolder.newExecutableFile("test");
given(state.getCommands()).willReturn(Collections.emptyMap());
Optional<Command> result = sut.tryResolve(file.getAbsolutePath() + "_invalid");
Optional<Command> result = sut.tryResolve(file.toAbsolutePath() + "_invalid");
assertThat(result).isEmpty();
}

@Test
void invalidDirectory() throws IOException {
folder.newFolder("test");
given(state.getPath()).willReturn(List.of(folder.toPath().toAbsolutePath()));
temporaryFolder.newFolder("test");
given(state.getPath()).willReturn(List.of(temporaryFolder.toPath().toAbsolutePath()));
given(state.getCwd()).willReturn(Paths.get("."));
given(state.getCommands()).willReturn(Collections.emptyMap());
Optional<Command> result = sut.tryResolve("test");
Expand All @@ -118,29 +117,30 @@ void invalidDirectory() throws IOException {
@DisabledOnOs(OS.WINDOWS)
@Test
void foundNonExecutableInPath() throws IOException {
assertThat(folder.newFile("test").setExecutable(false)).isTrue();
temporaryFolder.newFile("test");
given(state.getCommands()).willReturn(Collections.emptyMap());
given(state.getPath()).willReturn(List.of(folder.toPath().toAbsolutePath()));
given(state.getPath()).willReturn(List.of(temporaryFolder.toPath().toAbsolutePath()));
given(state.getCwd()).willReturn(Paths.get("."));
Optional<Command> result = sut.tryResolve("test");
assertThat(result).isNotPresent();
}

@Test
void foundExecutableInPath() throws IOException {
assertThat(folder.newFile("test").setExecutable(true)).isTrue();
temporaryFolder.newExecutableFile("test");
given(state.getCommands()).willReturn(Collections.emptyMap());
given(state.getPath()).willReturn(List.of(folder.toPath().toAbsolutePath()));
given(state.getPath()).willReturn(List.of(temporaryFolder.toPath().toAbsolutePath()));
given(state.getCwd()).willReturn(Paths.get("."));
Optional<Command> result = sut.tryResolve("test");
assertThat(result).isPresent();
}

@Test
@EnabledOnOs(OS.WINDOWS)
void notFoundInPathAsSpecifiedByPathExt() throws IOException {
assertThat(folder.newFile("test.vbs").setExecutable(true)).isTrue(); // VBS in not PATHEXT
temporaryFolder.newExecutableFile("test.vbs");// VBS in not PATHEXT
given(state.getCommands()).willReturn(Collections.emptyMap());
given(state.getPath()).willReturn(List.of(folder.toPath().toAbsolutePath()));
given(state.getPath()).willReturn(List.of(temporaryFolder.toPath().toAbsolutePath()));
given(state.getCwd()).willReturn(Paths.get("."));
given(state.getVariables()).willReturn(Map.of(VariableName.constant("PATHEXT"), ".COM;.EXE;.BAT;.CMD"));
Optional<Command> result = sut.tryResolve("test");
Expand All @@ -150,9 +150,9 @@ void notFoundInPathAsSpecifiedByPathExt() throws IOException {
@Test
@EnabledOnOs(OS.WINDOWS)
void foundInPathAsSpecifiedByPathExt() throws IOException {
assertThat(folder.newFile("test.exe").setExecutable(true)).isTrue();
temporaryFolder.newExecutableFile("test.exe");// VBS in not PATHEXT
given(state.getCommands()).willReturn(Collections.emptyMap());
given(state.getPath()).willReturn(List.of(folder.toPath().toAbsolutePath()));
given(state.getPath()).willReturn(List.of(temporaryFolder.toPath().toAbsolutePath()));
given(state.getCwd()).willReturn(Paths.get("."));
given(state.getVariables()).willReturn(Map.of(VariableName.constant("PATHEXT"), ".COM;.EXE;.BAT;.CMD"));
Optional<Command> result = sut.tryResolve("test");
Expand All @@ -161,9 +161,9 @@ void foundInPathAsSpecifiedByPathExt() throws IOException {

@Test
void foundInCwd() throws IOException {
assertThat(folder.newFile("test").setExecutable(true)).isTrue();
temporaryFolder.newExecutableFile("test");
given(state.getCommands()).willReturn(Collections.emptyMap());
given(state.getPath()).willReturn(List.of(folder.toPath().toAbsolutePath()));
given(state.getPath()).willReturn(List.of(temporaryFolder.toPath().toAbsolutePath()));
given(state.getCwd()).willReturn(Paths.get("."));
Optional<Command> result = sut.tryResolve("./test");
assertThat(result).isPresent();
Expand All @@ -172,7 +172,7 @@ void foundInCwd() throws IOException {
@Test
void notFoundInPath() {
given(state.getCommands()).willReturn(Collections.emptyMap());
given(state.getPath()).willReturn(List.of(folder.toPath().toAbsolutePath()));
given(state.getPath()).willReturn(List.of(temporaryFolder.toPath().toAbsolutePath()));
given(state.getCwd()).willReturn(Paths.get("."));
Optional<Command> result = sut.tryResolve("test");
assertThat(result).isEmpty();
Expand All @@ -184,7 +184,7 @@ void notFoundInPath() {
class WindowsCommandResolverTest {

@RegisterExtension
final TemporaryFolder folder = new TemporaryFolder();
final TemporaryFolder temporaryFolder = new TemporaryFolder();

@Mock(stubOnly = true)
State state;
Expand All @@ -198,27 +198,27 @@ void setup() {

@Test
void pathExtNotDefined() {
given(state.getPath()).willReturn(List.of(folder.toPath().toAbsolutePath()));
given(state.getPath()).willReturn(List.of(temporaryFolder.toPath().toAbsolutePath()));
given(state.getCwd()).willReturn(Paths.get("."));
Optional<Command> result = sut.tryResolve("test");
assertThat(result).isEmpty();
}

@Test
void findExecutableInPathext() throws IOException {
assertThat(folder.newFile("TEST.EXE").setExecutable(true)).isTrue();
temporaryFolder.newExecutableFile("TEST.EXE");
given(state.getVariables()).willReturn(Map.of(VariableName.constant("PATHEXT"), ".COM;.EXE"));
given(state.getPath()).willReturn(List.of(folder.toPath().toAbsolutePath()));
given(state.getPath()).willReturn(List.of(temporaryFolder.toPath().toAbsolutePath()));
given(state.getCwd()).willReturn(Paths.get("."));
Optional<Command> result = sut.tryResolve("TEST");
assertThat(result).isNotEmpty();
}

@Test
void findExecutableNotInPathext() throws IOException {
assertThat(folder.newFile("TEST.CMD").setExecutable(true)).isTrue();
temporaryFolder.newExecutableFile("test.cmd");
given(state.getVariables()).willReturn(Map.of(VariableName.constant("PATHEXT"), ".COM;.EXE"));
given(state.getPath()).willReturn(List.of(folder.toPath().toAbsolutePath()));
given(state.getPath()).willReturn(List.of(temporaryFolder.toPath().toAbsolutePath()));
given(state.getCwd()).willReturn(Paths.get("."));
Optional<Command> result = sut.tryResolve("TEST");
assertThat(result).isEmpty();
Expand Down
4 changes: 2 additions & 2 deletions runtime/src/test/java/hosh/runtime/ExternalCommandTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
class ExternalCommandTest {

@RegisterExtension
final TemporaryFolder folder = new TemporaryFolder();
final TemporaryFolder temporaryFolder = new TemporaryFolder();

@Mock(stubOnly = true)
State state;
Expand All @@ -89,7 +89,7 @@ class ExternalCommandTest {

@BeforeEach
void setup() throws IOException {
executable = folder.newFile().toPath();
executable = temporaryFolder.newFile(temporaryFolder.toPath(), "executable");
sut = new ExternalCommand(executable);
sut.setProcessFactory(processFactory);
sut.setState(state);
Expand Down
19 changes: 10 additions & 9 deletions runtime/src/test/java/hosh/runtime/FileSystemCompleterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@

import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;

Expand Down Expand Up @@ -88,7 +89,7 @@ void nonEmptyWordInEmptyDir() {

@Test
void emptyWordInNonEmptyDir() throws IOException {
temporaryFolder.newFile("a");
temporaryFolder.newFile(temporaryFolder.toPath(), "a");
given(state.getCwd()).willReturn(temporaryFolder.toPath());
given(line.word()).willReturn("");
List<Candidate> candidates = new ArrayList<>();
Expand Down Expand Up @@ -127,37 +128,37 @@ void rootDirWindows() {

@Test
void absoluteDirWithoutEndingSeparator() throws IOException {
File dir = temporaryFolder.newFolder("dir");
File newFile = temporaryFolder.newFile(dir, "aaa");
given(line.word()).willReturn(newFile.getParent());
Path dir = temporaryFolder.newFolder("dir");
Path newFile = temporaryFolder.newFile(dir, "aaa");
given(line.word()).willReturn(newFile.getParent().toString());
List<Candidate> candidates = new ArrayList<>();
sut.complete(lineReader, line, candidates);
assertThat(candidates)
.hasSize(1)
.allSatisfy(candidate -> {
assertThat(candidate.value()).isEqualTo(dir.getAbsolutePath() + File.separator);
assertThat(candidate.value()).isEqualTo(dir.toAbsolutePath() + File.separator);
assertThat(candidate.complete()).isFalse();
});
}

@Test
void absoluteDirWithEndingSeparator() throws IOException {
File dir = temporaryFolder.newFolder("dir");
File newFile = temporaryFolder.newFile(dir, "aaa");
Path dir = temporaryFolder.newFolder("dir");
Path newFile = temporaryFolder.newFile(dir, "aaa");
given(line.word()).willReturn(newFile.getParent() + File.separator);
List<Candidate> candidates = new ArrayList<>();
sut.complete(lineReader, line, candidates);
assertThat(candidates)
.hasSize(1)
.allSatisfy(candidate -> {
assertThat(candidate.value()).isEqualTo(newFile.getAbsolutePath());
assertThat(candidate.value()).isEqualTo(newFile.toAbsolutePath().toString());
assertThat(candidate.complete()).isTrue();
});
}

@Test
void partialMatchDirectory() throws IOException {
temporaryFolder.newFolder(temporaryFolder.newFolder("aaa"), "bbb");
temporaryFolder.newFolder(temporaryFolder.newFolder(temporaryFolder.toPath(), "aaa"), "bbb");
given(state.getCwd()).willReturn(temporaryFolder.toPath());
given(line.word()).willReturn("aaa" + File.separator + "b");
List<Candidate> candidates = new ArrayList<>();
Expand Down

0 comments on commit 72932fb

Please sign in to comment.