From 33827ab07c995616fdf216e88a23bce308688040 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Wed, 21 Apr 2021 14:36:58 +0200 Subject: [server] More checks and messages when renaming/deleting upload --- .../bwlp/sat/fileserv/IncomingDataTransfer.java | 54 +++++++++++++++------- .../java/org/openslx/bwlp/sat/util/Formatter.java | 5 +- 2 files changed, 41 insertions(+), 18 deletions(-) diff --git a/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/fileserv/IncomingDataTransfer.java b/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/fileserv/IncomingDataTransfer.java index 3c663bd0..e5a9cd09 100644 --- a/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/fileserv/IncomingDataTransfer.java +++ b/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/fileserv/IncomingDataTransfer.java @@ -4,6 +4,9 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.sql.SQLException; import java.util.List; import java.util.concurrent.ExecutorService; @@ -81,6 +84,11 @@ public class IncomingDataTransfer extends IncomingTransferBase { * connection */ private String errorMessage = null; + + /** + * Timestamp when this transfer was created + */ + private final long initTimestamp = System.currentTimeMillis(); public IncomingDataTransfer(String uploadId, UserInfo owner, ImageDetailsRead image, File destinationFile, long fileSize, List sha1Sums, byte[] machineDescription, @@ -210,8 +218,10 @@ public class IncomingDataTransfer extends IncomingTransferBase { */ @Override protected synchronized boolean finishIncomingTransfer() { - if (getState() != TransferState.FINISHED) + if (getState() != TransferState.FINISHED) { + LOGGER.warn("finishIncomingTransfer called in bad state " + getState()); return false; + } potentialFinishTime.set(System.currentTimeMillis()); // If owner is not set, this was a repair-transfer, which downloads directly to the existing target file. // Nothing more to do in that case. @@ -229,16 +239,16 @@ public class IncomingDataTransfer extends IncomingTransferBase { LOGGER.info("Finalizing uploaded image " + image.imageName); // Ready to go. First step: Rename temp file to something usable String ext = "img"; - try { - ext = DiskImage.newInstance(getTmpFileName()).getFormat().getExtension(); + try (DiskImage inst = DiskImage.newInstance(getTmpFileName())) { + ext = inst.getFormat().getExtension(); } catch (IOException | DiskImageException e1) { } - File destination = new File(getTmpFileName().getParent(), Formatter.vmName(owner, image.imageName, - ext)); + File destination = new File(getTmpFileName().getParent(), Formatter.vmName(initTimestamp, owner, image.imageName, + ext)).getAbsoluteFile(); // Sanity check: destination should be a sub directory of the vmStorePath String relPath = FileSystem.getRelativePath(destination, Configuration.getVmStoreBasePath()); if (relPath == null) { - LOGGER.warn(destination.getAbsolutePath() + " is not a subdir of " + LOGGER.error(destination.getAbsolutePath() + " is not a subdir of " + Configuration.getVmStoreBasePath().getAbsolutePath()); cancel(); return false; @@ -248,22 +258,32 @@ public class IncomingDataTransfer extends IncomingTransferBase { } // Execute rename - boolean ret = false; - Exception renameException = null; try { - ret = getTmpFileName().renameTo(destination); - } catch (Exception e) { - ret = false; - renameException = e; + Path d = Files.move(getTmpFileName().toPath(), destination.toPath(), StandardCopyOption.ATOMIC_MOVE); + if (d != null && d.toFile().exists()) { + destination = d.toFile(); + } + } catch (IOException e1) { + LOGGER.warn("Cannot rename", e1); + } + if (!destination.exists()) { + try { + getTmpFileName().renameTo(destination); + } catch (Exception e) { + LOGGER.warn("Cannot rename", e); + } } - if (!ret) { + if (!destination.exists()) { // Rename failed :-( - LOGGER.warn( - "Could not rename '" + getTmpFileName().getAbsolutePath() + "' to '" - + destination.getAbsolutePath() + "'", renameException); + LOGGER.warn("Could not rename '" + getTmpFileName().getAbsolutePath() + "' to '" + + destination.getAbsolutePath()); cancel(); return false; } + + if (destination.length() != getFileSize()) { + LOGGER.warn("Destination file size mismatch. Is: " + destination.length() + ", should be: " + getFileSize()); + } // Now insert meta data into DB try { @@ -278,6 +298,7 @@ public class IncomingDataTransfer extends IncomingTransferBase { } catch (SQLException e) { LOGGER.error("Error finishing upload: Inserting version to DB failed", e); // Also delete uploaded file, as there is no reference to it + LOGGER.info("Deleting file " + destination, e); FileSystem.deleteAsync(destination); cancel(); return false; @@ -314,6 +335,7 @@ public class IncomingDataTransfer extends IncomingTransferBase { public synchronized void cancel() { if (!isRepairUpload() && getTmpFileName().exists()) { super.cancel(); + LOGGER.debug("Deleting file " + getTmpFileName(), new RuntimeException()); FileSystem.deleteAsync(getTmpFileName()); } } diff --git a/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/util/Formatter.java b/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/util/Formatter.java index f13df0eb..268ceda1 100644 --- a/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/util/Formatter.java +++ b/dozentenmodulserver/src/main/java/org/openslx/bwlp/sat/util/Formatter.java @@ -28,13 +28,14 @@ public class Formatter { /** * Generate a file name for the given VM based on owner and display name. * + * @param ts Timestamp of upload * @param user The user associated with the VM, e.g. the owner * @param imageName Name of the VM * @param ext * @return File name for the VM derived from the function's input */ - public static String vmName(UserInfo user, String imageName, String ext) { - return cleanFileName(vmNameDateFormat.print(System.currentTimeMillis()) + "_" + user.lastName + "_" + public static String vmName(long ts, UserInfo user, String imageName, String ext) { + return cleanFileName(vmNameDateFormat.print(ts) + "_" + user.lastName + "_" + imageName + "." + ext).toLowerCase(); } -- cgit v1.2.3-55-g7522