diff options
author | HÃ¥kon Hallingstad <hakon.hallingstad@gmail.com> | 2022-04-02 11:57:45 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-04-02 11:57:45 +0200 |
commit | 7074892f0d55124a9373feae7b18e23855466066 (patch) | |
tree | 785f3eb12447a2ee063682f37f702af92e3aec6e /configserver | |
parent | caa488658d2b5c812e2450f7d91d2a9fb672618f (diff) |
Revert "Move verification down"
Diffstat (limited to 'configserver')
8 files changed, 72 insertions, 55 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/AddFileInterface.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/AddFileInterface.java index d51713cff77..855b27e0f92 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/AddFileInterface.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/AddFileInterface.java @@ -2,8 +2,8 @@ package com.yahoo.vespa.config.server.filedistribution; import com.yahoo.config.FileReference; -import com.yahoo.path.Path; +import java.io.File; import java.io.IOException; import java.nio.ByteBuffer; @@ -11,7 +11,8 @@ import java.nio.ByteBuffer; * @author baldersheim */ public interface AddFileInterface { - FileReference addUri(String uri, Path path); - FileReference addFile(Path path) throws IOException; - FileReference addBlob(ByteBuffer blob, Path path); + FileReference addUri(String uri, String relativePath); + FileReference addFile(String relativePath) throws IOException; + FileReference addFile(File file) throws IOException; + FileReference addBlob(ByteBuffer blob, String relativePath); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/ApplicationFileManager.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/ApplicationFileManager.java index 79fa919fabe..ad47f2b9e95 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/ApplicationFileManager.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/ApplicationFileManager.java @@ -3,10 +3,8 @@ package com.yahoo.vespa.config.server.filedistribution; import com.yahoo.config.FileReference; import com.yahoo.io.IOUtils; -import com.yahoo.path.Path; import net.jpountz.lz4.LZ4FrameOutputStream; -import java.io.Closeable; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; @@ -16,6 +14,8 @@ import java.nio.ByteBuffer; import java.nio.channels.Channels; import java.nio.channels.ReadableByteChannel; import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; /** * @author baldersheim @@ -31,43 +31,53 @@ public class ApplicationFileManager implements AddFileInterface { } @Override - public FileReference addFile(Path path) throws IOException { - File file = new File(applicationDir, path.getRelative()); - return addFile(file); + public FileReference addFile(String relativePath) throws IOException { + Path path = Path.of(relativePath).normalize(); + if (path.isAbsolute()) + throw new IllegalArgumentException(relativePath + " is not relative"); + File file = new File(applicationDir, relativePath); + Path relative = applicationDir.toPath().relativize(file.toPath()).normalize(); + if (relative.isAbsolute() || relative.startsWith("..")) + throw new IllegalArgumentException(file + " is not a descendant of " + applicationDir); + + return fileDirectory.addFile(file); } - private FileReference addFile(File file) throws IOException { + @Override + public FileReference addFile(File file) throws IOException { return fileDirectory.addFile(file); } @Override - public FileReference addUri(String uri, Path path) { - throw new UnsupportedOperationException("URI type is not supported"); - /* TODO: this needs to be super-restricted if the config server should ever do this. - try (TmpDir tmp = new TmpDir()) { - return addFile(download(uri, tmp.dir, path.getRelative())); - } - catch (IOException e) { + public FileReference addUri(String uri, String relativePath) { + File file = download(uri, relativePath); + try { + return addFile(file); + } catch (IOException e) { throw new IllegalArgumentException(e); + } finally { + cleanup(file, relativePath); } - */ } @Override - public FileReference addBlob(ByteBuffer blob, Path path) { - try (TmpDir tmp = new TmpDir()) { - return addFile(writeBlob(blob, tmp.dir, path.getRelative())); - } - catch (IOException e) { + public FileReference addBlob(ByteBuffer blob, String relativePath) { + File file = writeBlob(blob, relativePath); + try { + return addFile(file); + } catch (IOException e) { throw new IllegalArgumentException(e); + } finally { + cleanup(file, relativePath); } } - private File writeBlob(ByteBuffer blob, File tmpDir, String relativePath) { + private File writeBlob(ByteBuffer blob, String relativePath) { FileOutputStream fos = null; File file = null; try { - file = new File(tmpDir, relativePath); + Path path = Files.createTempDirectory(""); + file = new File(path.toFile(), relativePath); Files.createDirectories(file.getParentFile().toPath()); fos = new FileOutputStream(file); if (relativePath.endsWith(".lz4")) { @@ -91,12 +101,13 @@ public class ApplicationFileManager implements AddFileInterface { } } - private File download(String uri, File tmpDir, String relativePath) { + private File download(String uri, String relativePath) { File file = null; FileOutputStream fos = null; ReadableByteChannel rbc = null; try { - file = new File(tmpDir, relativePath); + Path path = Files.createTempDirectory(""); + file = new File(path.toFile(), relativePath); Files.createDirectories(file.getParentFile().toPath()); URL website = new URL(uri); rbc = Channels.newChannel(website.openStream()); @@ -121,10 +132,13 @@ public class ApplicationFileManager implements AddFileInterface { } } - private static class TmpDir implements Closeable { - final File dir = Files.createTempDirectory("").toFile(); - private TmpDir() throws IOException { } - @Override public void close() { IOUtils.recursiveDeleteDir(dir); } + private void cleanup(File file, String relativePath) { + Path pathToDelete = file.toPath(); + // Remove as many components as there are in relative path to find temp path to delete + for (int i = 0; i < Paths.get(relativePath).getNameCount(); i++) + pathToDelete = pathToDelete.resolveSibling(""); + IOUtils.recursiveDeleteDir(pathToDelete.toFile()); } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistry.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistry.java index c80a6d1d864..3eeaa6598d3 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistry.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistry.java @@ -5,7 +5,6 @@ import com.google.common.collect.ImmutableMap; import com.yahoo.config.FileReference; import com.yahoo.config.application.api.FileRegistry; import com.yahoo.net.HostName; -import com.yahoo.path.Path; import com.yahoo.text.Utf8; import net.jpountz.xxhash.XXHashFactory; @@ -71,13 +70,10 @@ public class FileDBRegistry implements FileRegistry { @Override public synchronized FileReference addFile(String relativePath) { - if (relativePath.startsWith("/")) - throw new IllegalArgumentException(relativePath + " is not relative"); - Optional<FileReference> cachedReference = Optional.ofNullable(fileReferenceCache.get(relativePath)); return cachedReference.orElseGet(() -> { try { - FileReference newRef = manager.addFile(Path.fromString(relativePath)); + FileReference newRef = manager.addFile(relativePath); fileReferenceCache.put(relativePath, newRef); return newRef; } catch (FileNotFoundException e) { @@ -98,7 +94,7 @@ public class FileDBRegistry implements FileRegistry { String relativePath = uriToRelativeFile(uri); Optional<FileReference> cachedReference = Optional.ofNullable(fileReferenceCache.get(uri)); return cachedReference.orElseGet(() -> { - FileReference newRef = manager.addUri(uri, Path.fromString(relativePath)); + FileReference newRef = manager.addUri(uri, relativePath); fileReferenceCache.put(uri, newRef); return newRef; }); @@ -110,7 +106,7 @@ public class FileDBRegistry implements FileRegistry { synchronized (this) { Optional<FileReference> cachedReference = Optional.ofNullable(fileReferenceCache.get(blobName)); return cachedReference.orElseGet(() -> { - FileReference newRef = manager.addBlob(blob, Path.fromString(relativePath)); + FileReference newRef = manager.addBlob(blob, relativePath); fileReferenceCache.put(blobName, newRef); return newRef; }); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java index 46ae2fd15d5..f4cb7bc1fba 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDirectory.java @@ -135,7 +135,7 @@ public class FileDirectory { return new FileReference(Long.toHexString(hash)); } - private FileReference addFile(File source, FileReference reference) { + FileReference addFile(File source, FileReference reference) { ensureRootExist(); try { logfileInfo(source); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileManager.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileManager.java index 6f551e40716..ce0a9866919 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileManager.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/MockFileManager.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.config.server.filedistribution; import com.yahoo.config.FileReference; -import com.yahoo.path.Path; import java.io.File; import java.nio.ByteBuffer; @@ -13,17 +12,22 @@ import java.nio.ByteBuffer; public class MockFileManager implements AddFileInterface { @Override - public FileReference addUri(String uri, Path path) { + public FileReference addUri(String uri, String relativePath) { return null; } @Override - public FileReference addFile(Path path) { + public FileReference addFile(String relativePath) { return null; } @Override - public FileReference addBlob(ByteBuffer blob, Path path) { + public FileReference addFile(File file) { + return null; + } + + @Override + public FileReference addBlob(ByteBuffer blob, String relativePath) { return null; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistryTestCase.java b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistryTestCase.java index 6d7074aef3c..cdb01f2013b 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistryTestCase.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistryTestCase.java @@ -27,7 +27,7 @@ public class FileDBRegistryTestCase { private static final String NO_FOO_FILE = "files/no_foo.json"; private static final String BOO_FILE = "/files/no_foo.json"; private static final String BAR_FILE = "../files/no_foo.json"; - private static final String BLOB_NAME = "././myblob.name"; + private static final String BLOB_NAME = "myblob.name"; private static final FileReference BLOB_REF = new FileReference("12f292a25163dd9"); private static final FileReference FOO_REF = new FileReference("b5ce94ca1feae86c"); @@ -54,7 +54,7 @@ public class FileDBRegistryTestCase { fileRegistry.addFile(BAR_FILE); fail(); } catch (IllegalArgumentException e) { - assertEquals("'..' is not allowed in path", e.getMessage()); + assertEquals("src/test/apps/zkapp/../files/no_foo.json is not a descendant of src/test/apps/zkapp", e.getMessage()); } assertEquals(BLOB_REF, fileRegistry.addBlob(BLOB_NAME, ByteBuffer.wrap(BLOB.getBytes(StandardCharsets.UTF_8)))); String serializedRegistry = FileDBRegistry.exportRegistry(fileRegistry); @@ -78,5 +78,4 @@ public class FileDBRegistryTestCase { void checkConsistentEntry(FileRegistry.Entry entry, FileRegistry registry) { assertEquals(entry.reference, registry.addFile(entry.relativePath)); } - } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileDistributionFactory.java b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileDistributionFactory.java index 6c65a2dee81..3ac621f7ac4 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileDistributionFactory.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileDistributionFactory.java @@ -17,7 +17,7 @@ public class MockFileDistributionFactory extends FileDistributionFactory { @Override public FileRegistry createFileRegistry(File applicationPackage) { - return new MockFileRegistry(applicationPackage, getFileReferencesDir()); + return new MockFileRegistry(applicationPackage, getFileReferencesDir().toPath()); } @Override diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileRegistry.java b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileRegistry.java index 9a9c224627a..be2d087478d 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileRegistry.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/MockFileRegistry.java @@ -3,11 +3,11 @@ package com.yahoo.vespa.config.server.filedistribution; import com.yahoo.config.FileReference; import com.yahoo.config.application.api.FileRegistry; -import com.yahoo.path.Path; import java.io.File; import java.io.IOException; import java.nio.ByteBuffer; +import java.nio.file.Path; import java.util.ArrayList; import java.util.List; @@ -21,15 +21,18 @@ public class MockFileRegistry implements FileRegistry { private final List<Entry> entries = new ArrayList<>(); private final AddFileInterface addFileInterface; - public MockFileRegistry(File applicationDir, File rootPath) { - FileDirectory fileDirectory = new FileDirectory(rootPath); + public MockFileRegistry(File applicationDir, Path rootPath) { + FileDirectory fileDirectory = new FileDirectory(rootPath.toFile()); this.addFileInterface = new ApplicationFileManager(applicationDir, fileDirectory); } public FileReference addFile(String relativePath) { - if (relativePath.isEmpty()) relativePath = "./"; + if (relativePath.isEmpty()) + relativePath = "./"; + try { - addFileInterface.addFile(Path.fromString(relativePath)); + addFileInterface.addFile(relativePath); + FileReference fileReference = new FileReference(relativePath); entries.add(new Entry(relativePath, fileReference)); return fileReference; @@ -47,10 +50,10 @@ public class MockFileRegistry implements FileRegistry { @Override public FileReference addBlob(String name, ByteBuffer blob) { - name = "./" + name; - FileReference fileReference = addFileInterface.addBlob(blob, Path.fromString(name)); + String relativePath = "./" + name; + FileReference fileReference = addFileInterface.addBlob(blob, relativePath); - entries.add(new Entry(name, fileReference)); + entries.add(new Entry(relativePath, fileReference)); return fileReference; } |