diff --git a/digdag-core/src/main/java/io/digdag/core/log/LocalFileLogServerFactory.java b/digdag-core/src/main/java/io/digdag/core/log/LocalFileLogServerFactory.java index 2a97ca3672..6d7fa78714 100644 --- a/digdag-core/src/main/java/io/digdag/core/log/LocalFileLogServerFactory.java +++ b/digdag-core/src/main/java/io/digdag/core/log/LocalFileLogServerFactory.java @@ -1,5 +1,6 @@ package io.digdag.core.log; +import java.nio.file.NoSuchFileException; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.io.InputStream; import java.io.OutputStream; @@ -122,11 +123,15 @@ protected void listFiles(String dateDir, String attemptDir, boolean enableDirect protected byte[] getFile(String dateDir, String attemptDir, String fileName) throws StorageFileNotFoundException { - Path path = getPrefixDir(dateDir, attemptDir).resolve(fileName); + Path prefixDir = getPrefixDir(dateDir, attemptDir); + Path path = prefixDir.resolve(fileName).normalize(); + if (!path.startsWith(prefixDir)) { + throw new IllegalArgumentException("Invalid file name: " + fileName); + } try (InputStream in = Files.newInputStream(path)) { return ByteStreams.toByteArray(in); } - catch (FileNotFoundException ex) { + catch (FileNotFoundException | NoSuchFileException ex) { throw new StorageFileNotFoundException(ex); } catch (IOException ex) { diff --git a/digdag-core/src/test/java/io/digdag/core/log/LocalFileLogServerFactoryTest.java b/digdag-core/src/test/java/io/digdag/core/log/LocalFileLogServerFactoryTest.java index aef90defde..1486c6b42a 100644 --- a/digdag-core/src/test/java/io/digdag/core/log/LocalFileLogServerFactoryTest.java +++ b/digdag-core/src/test/java/io/digdag/core/log/LocalFileLogServerFactoryTest.java @@ -6,6 +6,7 @@ import io.digdag.core.agent.AgentId; import io.digdag.core.config.PropertyUtils; import io.digdag.spi.LogFilePrefix; +import io.digdag.spi.StorageFileNotFoundException; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -23,6 +24,7 @@ import java.util.Properties; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertThrows; import io.digdag.core.log.LocalFileLogServerFactory.LocalFileLogServer.LocalFileDirectTaskLogger; @@ -142,4 +144,30 @@ private String repeatedString(String v, int num) } return b.toString(); } + + @Test + public void testGetFile() throws StorageFileNotFoundException + { + setUpTaskLogger(Optional.absent()); + String fileName = localServer.putFile(prefix, "+task", Instant.now(), "agent", "foo".getBytes(UTF_8)); + byte[] data = localServer.getFile(prefix, fileName); + assertThat(new String(data, UTF_8), is("foo")); + } + + @Test + public void testGetFileNotFound() + { + setUpTaskLogger(Optional.absent()); + assertThrows(StorageFileNotFoundException.class, () -> localServer.getFile(prefix, "foo")); + } + + @Test + public void testGetFileInvalidFileName() + { + setUpTaskLogger(Optional.absent()); + assertThrows(IllegalArgumentException.class, () -> localServer.getFile(prefix, "..")); + assertThrows(IllegalArgumentException.class, () -> localServer.getFile(prefix, "../foo")); + assertThrows(IllegalArgumentException.class, () -> localServer.getFile(prefix, "/foo")); + assertThrows(IllegalArgumentException.class, () -> localServer.getFile(prefix, "foo/../../bar")); + } } diff --git a/digdag-tests/src/test/java/acceptance/LogIT.java b/digdag-tests/src/test/java/acceptance/LogIT.java index b934b30ba2..cc7d07a327 100644 --- a/digdag-tests/src/test/java/acceptance/LogIT.java +++ b/digdag-tests/src/test/java/acceptance/LogIT.java @@ -10,12 +10,15 @@ import utils.CommandStatus; import utils.TemporaryDigdagServer; +import javax.ws.rs.BadRequestException; +import javax.ws.rs.NotFoundException; import java.nio.file.Files; import java.nio.file.Path; import java.util.regex.Pattern; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static utils.TestUtils.*; @@ -91,5 +94,8 @@ public void verifyLogWithAttemptIdAndSessionId() final String regex = "\\[\\d+:\\w+:\\d+:\\d+]"; assertTrue(Pattern.compile(regex, Pattern.DOTALL).matcher(logs).find()); + + assertThrows(NotFoundException.class, () -> client.getLogFile(attemptId, "foo")); + assertThrows(BadRequestException.class, () -> client.getLogFile(attemptId, "/foo")); } }