diff --git a/README.md b/README.md index 47727864..c6bc2eee 100644 --- a/README.md +++ b/README.md @@ -61,6 +61,11 @@ history. without asking. A difference, or anything it cannot establish, asks. - Conversions made before this existed carry no such record, so they are never assumed to match. They ask, once, and re-converting produces one that no longer will. +- A zip that holds a pack in the device's own layout — `ni`, `ri`, `si` and the asset folders, + usually copied straight off an SD card — can be added to the library. It is unpacked into a folder + on upload, named after the zip's top-level folder (the pack's UUID) when there is one, and the + library reads it as it reads any pack folder. Such zips used to be dropped from the listing for + want of a `story.json`. **Testing** diff --git a/TESTING.md b/TESTING.md index a15d0b7f..defd5985 100644 --- a/TESTING.md +++ b/TESTING.md @@ -75,7 +75,7 @@ Nothing here touches a device. Fixtures are synthesised in code; no device data | Suite | Tests | | --- | --- | -| Java, standard | **309**, 14 skipped — the opt-in FAT32 classes, and two link cases each of which only one platform can set up. A third conditional case, the non-loopback binding check, skips only on a machine with no routable address | +| Java, standard | **315**, 14 skipped — the opt-in FAT32 classes, and two link cases each of which only one platform can set up. A third conditional case, the non-loopback binding check, skips only on a machine with no routable address | | Java, with `-Dstudio.test.fat32.root=` | last measured at **172** before the C6d-5 additions; not re-measured since, because it needs the volume mounted | | JavaScript | **57** | @@ -124,6 +124,7 @@ counts. | What each of the six conversions records about itself, and every reason it records nothing | `ConversionProvenanceTest` | `web-ui` module; **specifications** | | When a conversion is proven to match its source, and every reason it is not | `ConversionVerificationTest` | `web-ui` module; **specifications** — only MATCH removes a confirmation; a path outside the library is refused rather than answered | | Where a library operation may reach: direct children only, links refused, nominal cases intact | `LibraryPathConfinementTest` | `web-ui` module; **specifications** — converted from characterization once the confinement existed. The symbolic-link case is Linux-only and the junction case Windows-only | +| Uploading a zip that holds a pack folder: unpacked under the folder's or the upload's name, archives and unknown zips stored as they were, an entry climbing out refused before anything is written | `LibraryFsPackImportTest` | `web-ui` module; **specifications** | | A conversion releases its source and its temporary even when the reader or writer throws | `ConversionStreamLifecycleTest` | `web-ui` module; **specifications** — asserts the consequence by deleting the work folder, so the three failure cases are Windows-only | | Where the HTTP server can be reached from: the loopback answers, another address of this machine does not | `HttpServerBindingTest` | `web-ui` module; **specifications** — opens real sockets against a real server rather than reading the constant back. The refusal case needs a non-loopback address to aim at and skips where there is none | | Same, but on the deployed application rather than a bare server | `ServerBindingTest` | `web-ui` module; **specifications** — deploys the real `MainVerticle`, so it proves `start()` uses the address rather than that the address works. It binds the fixed `LISTEN_PORT`, so both cases skip when that port is already taken; `HttpServerBindingTest` is the one that runs unconditionally | diff --git a/web-ui/src/main/java/studio/webui/service/FsPackZipImporter.java b/web-ui/src/main/java/studio/webui/service/FsPackZipImporter.java new file mode 100644 index 00000000..7d119a74 --- /dev/null +++ b/web-ui/src/main/java/studio/webui/service/FsPackZipImporter.java @@ -0,0 +1,116 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +package studio.webui.service; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.util.Enumeration; +import java.util.HashSet; +import java.util.Optional; +import java.util.Set; +import java.util.zip.ZipEntry; +import java.util.zip.ZipException; +import java.util.zip.ZipFile; + +import static studio.core.v1.reader.fs.FsStoryPackReader.IMAGE_INDEX_FILENAME; +import static studio.core.v1.reader.fs.FsStoryPackReader.NODE_INDEX_FILENAME; +import static studio.core.v1.reader.fs.FsStoryPackReader.SOUND_INDEX_FILENAME; + +/** + * Recognises a zip that holds a pack in the device's own layout, and unpacks it. + * + *

The layout is the one {@code FsStoryPackReader} reads from a folder: {@code ni}, {@code ri}, + * {@code si} and the asset folders beside them. In a zip those files sit either at the root or + * under a single top-level folder — the device names its pack folders by UUID, and a zip made from + * one keeps that folder. + */ +final class FsPackZipImporter { + + private FsPackZipImporter() { + } + + /** + * Where the pack files sit in the zip: {@code ""} for the root, {@code "/"} for a single + * top-level folder, or empty when the zip is not a pack folder at all — including when it is not + * a zip, or holds a {@code story.json}, which is an archive and someone else's business. + */ + static Optional packPrefix(Path zip) throws IOException { + try (ZipFile file = new ZipFile(zip.toFile())) { + if (file.getEntry("story.json") != null) { + return Optional.empty(); + } + Set topLevelFolders = new HashSet<>(); + Enumeration entries = file.entries(); + while (entries.hasMoreElements()) { + String name = entries.nextElement().getName(); + int slash = name.indexOf('/'); + if (slash >= 0) { + topLevelFolders.add(name.substring(0, slash + 1)); + } + } + if (isPackFolder(file, "")) { + return Optional.of(""); + } + if (topLevelFolders.size() == 1) { + String prefix = topLevelFolders.iterator().next(); + if (isPackFolder(file, prefix)) { + return Optional.of(prefix); + } + } + return Optional.empty(); + } catch (ZipException notAZip) { + return Optional.empty(); + } + } + + private static boolean isPackFolder(ZipFile file, String prefix) { + return file.getEntry(prefix + NODE_INDEX_FILENAME) != null + && file.getEntry(prefix + IMAGE_INDEX_FILENAME) != null + && file.getEntry(prefix + SOUND_INDEX_FILENAME) != null; + } + + /** + * Unpacks the entries under {@code prefix} into {@code target}, which must already exist. + * + *

Every entry is checked to land inside {@code target} before anything is written for it — + * but an entry that climbs out is found part-way through the zip, after earlier entries have + * been written, so the caller unpacks into a folder it can throw away. + * + * @throws IOException when an entry would land outside {@code target}, or on any I/O failure + */ + static void unpack(Path zip, String prefix, Path target) throws IOException { + Path root = target.toAbsolutePath().normalize(); + try (ZipFile file = new ZipFile(zip.toFile())) { + Enumeration entries = file.entries(); + while (entries.hasMoreElements()) { + ZipEntry entry = entries.nextElement(); + if (!entry.getName().startsWith(prefix)) { + continue; + } + String relative = entry.getName().substring(prefix.length()); + if (relative.isEmpty()) { + continue; + } + Path destination = root.resolve(relative).normalize(); + if (!destination.startsWith(root)) { + throw new IOException("Zip entry would land outside the pack folder: " + entry.getName()); + } + if (entry.isDirectory()) { + Files.createDirectories(destination); + continue; + } + Files.createDirectories(destination.getParent()); + try (InputStream in = file.getInputStream(entry)) { + Files.copy(in, destination, StandardCopyOption.REPLACE_EXISTING); + } + } + } + } +} diff --git a/web-ui/src/main/java/studio/webui/service/LibraryService.java b/web-ui/src/main/java/studio/webui/service/LibraryService.java index 83edeb75..f411bdf4 100644 --- a/web-ui/src/main/java/studio/webui/service/LibraryService.java +++ b/web-ui/src/main/java/studio/webui/service/LibraryService.java @@ -626,6 +626,12 @@ public boolean addPackFile(String destPath, String uploadedFilePath) { try { // Copy temporary file to local library File src = new File(uploadedFilePath); + if (destPath != null && destPath.endsWith(".zip")) { + Optional packPrefix = FsPackZipImporter.packPrefix(src.toPath()); + if (packPrefix.isPresent()) { + return addZippedPackFolder(destPath, src.toPath(), packPrefix.get()); + } + } Optional destination = libraryEntry(destPath); if (!destination.isPresent()) { // Refused before anything is deleted, moved or created. @@ -647,6 +653,41 @@ public boolean addPackFile(String destPath, String uploadedFilePath) { } } + /** + * An uploaded zip that holds a pack folder — the device's own layout, see + * {@link FsPackZipImporter} — becomes a folder of the library, and the zip is not kept. The + * library reads that layout from a folder already; a zip of it was handed to the archive reader + * and dropped from the listing for want of a {@code story.json}. + * + *

The folder is named after the zip's top-level folder when there is one, since that is where + * the reader takes the pack's UUID from, and after the upload otherwise. Unpacking happens in + * the work folder first, so that a zip refused part-way through leaves nothing in the library. + */ + private boolean addZippedPackFolder(String destPath, Path uploaded, String packPrefix) throws IOException { + String folderName = packPrefix.isEmpty() + ? destPath.substring(0, destPath.length() - ".zip".length()) + : packPrefix.substring(0, packPrefix.length() - 1); + Optional destination = libraryEntry(folderName); + if (!destination.isPresent()) { + return false; + } + Path unpacked = Files.createTempDirectory(Paths.get(tmpDirPath()), "import-"); + try { + FsPackZipImporter.unpack(uploaded, packPrefix, unpacked); + } catch (IOException e) { + LOGGER.warn("Refused a zipped pack folder: " + e.getMessage()); + FileUtils.deleteQuietly(unpacked.toFile()); + return false; + } + File dest = destination.get().toFile(); + if (dest.exists()) { + FileUtils.deleteDirectory(dest); + } + FileUtils.moveDirectory(unpacked.toFile(), dest); + Files.delete(uploaded); + return true; + } + public boolean deletePack(String packPath) { File libraryFolder = new File(libraryPath()); if (!libraryFolder.exists() || !libraryFolder.isDirectory()) { diff --git a/web-ui/src/test/java/studio/webui/service/LibraryFsPackImportTest.java b/web-ui/src/test/java/studio/webui/service/LibraryFsPackImportTest.java new file mode 100644 index 00000000..3a1c1645 --- /dev/null +++ b/web-ui/src/test/java/studio/webui/service/LibraryFsPackImportTest.java @@ -0,0 +1,207 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +package studio.webui.service; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import studio.metadata.DatabaseMetadataService; + +import java.io.File; +import java.io.IOException; +import java.io.OutputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Uploading a zip that holds a pack in the device's own layout — {@code ni}, {@code li}, {@code ri}, + * {@code si} and the {@code rf/} and {@code sf/} asset folders — rather than a {@code story.json}. + * + *

Such zips are how packs copied straight off an SD card get passed around. The library reads + * that layout already, but only as a folder: a zip was handed to the archive reader, which found no + * {@code story.json} and the pack was silently dropped from the listing. The upload now unpacks it + * into a folder instead, and the zip itself is not kept. + * + *

The folder takes its name from the zip's single top-level folder when there is one — the + * device's packs are named by UUID and the reader takes the UUID from the folder name — and from + * the upload's own name otherwise. A zip that holds a {@code story.json}, or anything the library + * does not recognise, is stored as it was: saving a pack from the editor goes through the same + * upload, and that flow must not change. + */ +class LibraryFsPackImportTest { + + private static final String UUID = "EF1C88E3F1D34413B75F96BAF2A20C6E"; + private static final String SENTINEL = "outside-sentinel.txt"; + + @TempDir + Path root; + + private Path library; + private Path sentinel; + private LibraryService service; + private final Map previousProperties = new HashMap<>(); + + @BeforeEach + void setUp() throws IOException { + library = root.resolve("library"); + Files.createDirectories(library); + Files.createDirectories(root.resolve("db")); + Files.createDirectories(root.resolve("tmp")); + sentinel = root.resolve(SENTINEL); + Files.write(sentinel, "untouched".getBytes(StandardCharsets.UTF_8)); + + setProperty(LibraryService.LOCAL_LIBRARY_PROP, library + File.separator); + setProperty(LibraryService.TMP_DIR_PROP, root.resolve("tmp") + File.separator); + setProperty(ConversionProvenanceStore.CONVERSIONS_DB_PROP, + root.resolve("db").resolve("conversions.json").toString()); + setProperty(DatabaseMetadataService.UNOFFICIAL_DB_PROP, + root.resolve("db").resolve("unofficial.json").toString()); + + service = new LibraryService(new DatabaseMetadataService(true)); + } + + @AfterEach + void tearDown() { + previousProperties.forEach((key, value) -> { + if (value == null) { + System.clearProperty(key); + } else { + System.setProperty(key, value); + } + }); + } + + @Test + @DisplayName("a zipped pack folder is unpacked under the folder's own name, and the zip is not kept") + void zippedPackFolderIsUnpackedUnderItsOwnName() throws IOException { + Path uploaded = zip("upload.zip", fsPack(UUID + "/")); + + assertTrue(service.addPackFile("Ruben.zip", uploaded.toString())); + + Path pack = library.resolve(UUID); + assertTrue(Files.isDirectory(pack), "the pack must be a folder of the library"); + assertArrayEquals(bytes("ni"), Files.readAllBytes(pack.resolve("ni"))); + assertArrayEquals(bytes("image"), Files.readAllBytes(pack.resolve("rf/000/AABBCCDD"))); + assertArrayEquals(bytes("sound"), Files.readAllBytes(pack.resolve("sf/000/11223344"))); + assertFalse(Files.exists(library.resolve("Ruben.zip")), "the zip is not a library entry"); + assertFalse(Files.exists(uploaded), "the upload has been consumed"); + } + + @Test + @DisplayName("a zip with the pack files at its root is unpacked under the upload's name") + void packFilesAtTheRootAreUnpackedUnderTheUploadName() throws IOException { + Path uploaded = zip("upload.zip", fsPack("")); + + assertTrue(service.addPackFile("Ruben.zip", uploaded.toString())); + + assertArrayEquals(bytes("ni"), Files.readAllBytes(library.resolve("Ruben").resolve("ni"))); + assertFalse(Files.exists(library.resolve("Ruben.zip"))); + } + + @Test + @DisplayName("a story.json archive is stored as it was") + void archiveIsStoredUnchanged() throws IOException { + Map entries = new LinkedHashMap<>(); + entries.put("story.json", bytes("{\"format\":\"v1\"}")); + entries.put("assets/a.png", bytes("png")); + Path uploaded = zip("upload.zip", entries); + byte[] asUploaded = Files.readAllBytes(uploaded); + + assertTrue(service.addPackFile("pack.zip", uploaded.toString())); + + assertArrayEquals(asUploaded, Files.readAllBytes(library.resolve("pack.zip"))); + } + + @Test + @DisplayName("a zip that is neither an archive nor a pack folder is stored as it was") + void unrecognisedZipIsStoredUnchanged() throws IOException { + Map entries = new LinkedHashMap<>(); + entries.put("readme.txt", bytes("not a pack")); + Path uploaded = zip("upload.zip", entries); + byte[] asUploaded = Files.readAllBytes(uploaded); + + assertTrue(service.addPackFile("pack.zip", uploaded.toString())); + + assertArrayEquals(asUploaded, Files.readAllBytes(library.resolve("pack.zip"))); + } + + @Test + @DisplayName("an entry that climbs out of the pack folder is refused, and nothing is written") + void entryLeavingThePackFolderIsRefused() throws IOException { + Map entries = fsPack(UUID + "/"); + entries.put(UUID + "/rf/../../../" + SENTINEL, bytes("overwritten")); + Path uploaded = zip("upload.zip", entries); + + assertFalse(service.addPackFile("Ruben.zip", uploaded.toString())); + + assertEquals("untouched", new String(Files.readAllBytes(sentinel), StandardCharsets.UTF_8)); + assertFalse(Files.exists(library.resolve(UUID)), "a refused import leaves no folder behind"); + assertFalse(Files.exists(library.resolve("Ruben.zip"))); + } + + @Test + @DisplayName("a pack folder already in the library is replaced") + void existingPackFolderIsReplaced() throws IOException { + Path existing = Files.createDirectories(library.resolve(UUID)); + Files.write(existing.resolve("ni"), bytes("old")); + Files.write(existing.resolve("stale"), bytes("should go")); + Path uploaded = zip("upload.zip", fsPack(UUID + "/")); + + assertTrue(service.addPackFile("Ruben.zip", uploaded.toString())); + + assertArrayEquals(bytes("ni"), Files.readAllBytes(existing.resolve("ni"))); + assertFalse(Files.exists(existing.resolve("stale")), "the old folder is not merged into"); + } + + // ---------------------------------------------------------------- fixtures and helpers + + /** The files of a pack folder, under {@code prefix} ({@code ""} for the zip root). */ + private static Map fsPack(String prefix) { + Map entries = new LinkedHashMap<>(); + entries.put(prefix + "ni", bytes("ni")); + entries.put(prefix + "li", bytes("li")); + entries.put(prefix + "ri", bytes("ri")); + entries.put(prefix + "si", bytes("si")); + entries.put(prefix + "rf/000/AABBCCDD", bytes("image")); + entries.put(prefix + "sf/000/11223344", bytes("sound")); + return entries; + } + + private Path zip(String name, Map entries) throws IOException { + Path file = root.resolve(name); + try (OutputStream out = Files.newOutputStream(file); ZipOutputStream zos = new ZipOutputStream(out)) { + for (Map.Entry entry : entries.entrySet()) { + zos.putNextEntry(new ZipEntry(entry.getKey())); + zos.write(entry.getValue()); + zos.closeEntry(); + } + } + return file; + } + + private static byte[] bytes(String s) { + return s.getBytes(StandardCharsets.UTF_8); + } + + private void setProperty(String key, String value) { + previousProperties.putIfAbsent(key, System.getProperty(key)); + System.setProperty(key, value); + } +}