Web UI: Upload to tmp file name then rename if successful (#1272)

* Upload to tmp file name then rename if successful

* Move the dropzone.js operations back into web.py

* Move list_images() from file commands into piscsi commands (it was the only class method in that package that calls the protobuf interface)

* Remove now-redundant helptext

python/common/src/piscsi/file_cmds.py

@@ -5,7 +5,6 @@ Module for methods reading from and writing to the file system
 import logging
 import asyncio
 from os import walk, path
-from functools import lru_cache
 from pathlib import PurePath, Path
 from zipfile import ZipFile, is_zipfile
 from subprocess import run, Popen, PIPE, CalledProcessError, TimeoutExpired
@@ -17,18 +16,15 @@ from re import search
 
 import requests
 
-import piscsi_interface_pb2 as proto
 from piscsi.common_settings import (
     CFG_DIR,
     CONFIG_FILE_SUFFIX,
     PROPERTIES_SUFFIX,
-    ARCHIVE_FILE_SUFFIXES,
     RESERVATIONS,
     SHELL_ERROR,
 )
 from piscsi.piscsi_cmds import PiscsiCmds
 from piscsi.return_codes import ReturnCodes
-from piscsi.socket_cmds import SocketCmds
 from util import unarchiver
 
 FILE_READ_ERROR = "Unhandled exception when reading file: %s"
@@ -41,18 +37,8 @@ class FileCmds:
     class for methods reading from and writing to the file system
     """
 
-    def __init__(self, sock_cmd: SocketCmds, piscsi: PiscsiCmds, token=None, locale=None):
+    def __init__(self, piscsi: PiscsiCmds):
-        self.sock_cmd = sock_cmd
         self.piscsi = piscsi
-        self.token = token
-        self.locale = locale
-
-    def send_pb_command(self, command):
-        if logging.getLogger().isEnabledFor(logging.DEBUG):
-            # TODO: Uncouple/move to common dependency
-            logging.debug(self.piscsi.format_pb_command(command))
-
-        return self.sock_cmd.send_pb_command(command.SerializeToString())
 
     # noinspection PyMethodMayBeStatic
     def list_config_files(self):
@@ -87,76 +73,6 @@ class FileCmds:
         subdir_list.sort()
         return subdir_list
 
-    def list_images(self):
-        """
-        Sends a IMAGE_FILES_INFO command to the server
-        Returns a (dict) with (bool) status, (str) msg, and (list) of (dict)s files
-
-        """
-        command = proto.PbCommand()
-        command.operation = proto.PbOperation.DEFAULT_IMAGE_FILES_INFO
-        command.params["token"] = self.token
-        command.params["locale"] = self.locale
-
-        data = self.send_pb_command(command)
-        result = proto.PbResult()
-        result.ParseFromString(data)
-
-        server_info = self.piscsi.get_server_info()
-        files = []
-        for file in result.image_files_info.image_files:
-            prop_file_path = Path(CFG_DIR) / f"{file.name}.{PROPERTIES_SUFFIX}"
-            # Add properties meta data for the image, if matching prop file is found
-            if prop_file_path.exists():
-                process = self.read_drive_properties(prop_file_path)
-                prop = process["conf"]
-            else:
-                prop = False
-
-            archive_contents = []
-            if PurePath(file.name).suffix.lower()[1:] in ARCHIVE_FILE_SUFFIXES:
-                try:
-                    archive_info = self._get_archive_info(
-                        f"{server_info['image_dir']}/{file.name}",
-                        _cache_extra_key=file.size,
-                    )
-
-                    properties_files = [
-                        x["path"]
-                        for x in archive_info["members"]
-                        if x["path"].endswith(PROPERTIES_SUFFIX)
-                    ]
-
-                    for member in archive_info["members"]:
-                        if member["is_dir"] or member["is_resource_fork"]:
-                            continue
-
-                        if PurePath(member["path"]).suffix.lower()[1:] == PROPERTIES_SUFFIX:
-                            member["is_properties_file"] = True
-                        elif f"{member['path']}.{PROPERTIES_SUFFIX}" in properties_files:
-                            member[
-                                "related_properties_file"
-                            ] = f"{member['path']}.{PROPERTIES_SUFFIX}"
-
-                        archive_contents.append(member)
-                except (unarchiver.LsarCommandError, unarchiver.LsarOutputError):
-                    pass
-
-            size_mb = "{:,.1f}".format(file.size / 1024 / 1024)
-            dtype = proto.PbDeviceType.Name(file.type)
-            files.append(
-                {
-                    "name": file.name,
-                    "size": file.size,
-                    "size_mb": size_mb,
-                    "detected_type": dtype,
-                    "prop": prop,
-                    "archive_contents": archive_contents,
-                }
-            )
-
-        return {"status": result.status, "msg": result.msg, "files": files}
-
     # noinspection PyMethodMayBeStatic
     def delete_file(self, file_path):
         """
@@ -892,15 +808,3 @@ class FileCmds:
             logging.info("stderr: %s", stderr)
 
         return {"returncode": proc.returncode, "stdout": stdout, "stderr": stderr}
-
-    # noinspection PyMethodMayBeStatic
-    @lru_cache(maxsize=32)
-    def _get_archive_info(self, file_path, **kwargs):
-        """
-        Cached wrapper method to improve performance, e.g. on index screen
-        """
-        try:
-            return unarchiver.inspect_archive(file_path)
-        except (unarchiver.LsarCommandError, unarchiver.LsarOutputError) as error:
-            logging.error(str(error))
-            raise

python/common/src/piscsi/piscsi_cmds.py

@@ -2,10 +2,21 @@
 Module for commands sent to the PiSCSI backend service.
 """
 
+import logging
+from pathlib import PurePath, Path
+from functools import lru_cache
+
 import piscsi_interface_pb2 as proto
 from piscsi.return_codes import ReturnCodes
 from piscsi.socket_cmds import SocketCmds
-import logging
+
+from piscsi.common_settings import (
+    CFG_DIR,
+    PROPERTIES_SUFFIX,
+    ARCHIVE_FILE_SUFFIXES,
+)
+
+from util import unarchiver
 
 
 class PiscsiCmds:
@@ -24,6 +35,79 @@ class PiscsiCmds:
 
         return self.sock_cmd.send_pb_command(command.SerializeToString())
 
+    def list_images(self):
+        """
+        Sends a IMAGE_FILES_INFO command to the server
+        Returns a (dict) with (bool) status, (str) msg, and (list) of (dict)s files
+        """
+        from piscsi.file_cmds import FileCmds
+
+        self.file_cmd = FileCmds(piscsi=self)
+
+        command = proto.PbCommand()
+        command.operation = proto.PbOperation.DEFAULT_IMAGE_FILES_INFO
+        command.params["token"] = self.token
+        command.params["locale"] = self.locale
+
+        data = self.send_pb_command(command)
+        result = proto.PbResult()
+        result.ParseFromString(data)
+
+        server_info = self.get_server_info()
+        files = []
+        for file in result.image_files_info.image_files:
+            prop_file_path = Path(CFG_DIR) / f"{file.name}.{PROPERTIES_SUFFIX}"
+            # Add properties meta data for the image, if matching prop file is found
+            if prop_file_path.exists():
+                process = self.file_cmd.read_drive_properties(prop_file_path)
+                prop = process["conf"]
+            else:
+                prop = False
+
+            archive_contents = []
+            if PurePath(file.name).suffix.lower()[1:] in ARCHIVE_FILE_SUFFIXES:
+                try:
+                    archive_info = self._get_archive_info(
+                        f"{server_info['image_dir']}/{file.name}",
+                        _cache_extra_key=file.size,
+                    )
+
+                    properties_files = [
+                        x["path"]
+                        for x in archive_info["members"]
+                        if x["path"].endswith(PROPERTIES_SUFFIX)
+                    ]
+
+                    for member in archive_info["members"]:
+                        if member["is_dir"] or member["is_resource_fork"]:
+                            continue
+
+                        if PurePath(member["path"]).suffix.lower()[1:] == PROPERTIES_SUFFIX:
+                            member["is_properties_file"] = True
+                        elif f"{member['path']}.{PROPERTIES_SUFFIX}" in properties_files:
+                            member[
+                                "related_properties_file"
+                            ] = f"{member['path']}.{PROPERTIES_SUFFIX}"
+
+                        archive_contents.append(member)
+                except (unarchiver.LsarCommandError, unarchiver.LsarOutputError):
+                    pass
+
+            size_mb = "{:,.1f}".format(file.size / 1024 / 1024)
+            dtype = proto.PbDeviceType.Name(file.type)
+            files.append(
+                {
+                    "name": file.name,
+                    "size": file.size,
+                    "size_mb": size_mb,
+                    "detected_type": dtype,
+                    "prop": prop,
+                    "archive_contents": archive_contents,
+                }
+            )
+
+        return {"status": result.status, "msg": result.msg, "files": files}
+
     def get_server_info(self):
         """
         Sends a SERVER_INFO command to the server.
@@ -521,3 +605,15 @@ class PiscsiCmds:
             message += f", device: {formatted_device}"
 
         return message
+
+    # noinspection PyMethodMayBeStatic
+    @lru_cache(maxsize=32)
+    def _get_archive_info(self, file_path, **kwargs):
+        """
+        Cached wrapper method to improve performance, e.g. on index screen
+        """
+        try:
+            return unarchiver.inspect_archive(file_path)
+        except (unarchiver.LsarCommandError, unarchiver.LsarOutputError) as error:
+            logging.error(str(error))
+            raise

python/ctrlboard/src/ctrlboard_menu_builder.py

@@ -142,7 +142,7 @@ class CtrlBoardMenuBuilder(MenuBuilder):
     def create_images_menu(self, context_object=None):
         """Creates a sub menu showing all the available images"""
         menu = Menu(CtrlBoardMenuBuilder.IMAGES_MENU)
-        images_info = self.file_cmd.list_images()
+        images_info = self.piscsi_cmd.list_images()
         menu.add_entry("Return", {"context": self.IMAGES_MENU, "action": self.ACTION_RETURN})
         images = images_info["files"]
         sorted_images = sorted(images, key=lambda d: d["name"])

python/web/src/templates/upload.html

@@ -4,7 +4,6 @@
 <h2>{{ _("Upload File from Local Computer") }}</h2>
 <ul>
     <li>{{ _("The largest file size accepted in this form is %(max_file_size)s MiB. Use other file transfer means for larger files.", max_file_size=max_file_size) }}</li>
-    <li>{{ _("You have to manually clean up partially uploaded files, as a result of cancelling the upload or closing this page.") }}</li>
     <li>{{ _("Disk Images") }} = {{ env["image_dir"] }}</li>
     {% if file_server_dir_exists %}
     <li>{{ _("Shared Files") }} = {{ FILE_SERVER_DIR }}</li>

python/web/src/web.py

@@ -8,11 +8,13 @@ import argparse
 from pathlib import Path, PurePath
 from functools import wraps
 from grp import getgrall
-
+from os import path
 import bjoern
+
 from piscsi.return_codes import ReturnCodes
 from simplepam import authenticate
 from flask_babel import Babel, Locale, refresh, _
+from werkzeug.utils import secure_filename
 
 from flask import (
     Flask,
@@ -55,7 +57,6 @@ from web_utils import (
     auth_active,
     is_bridge_configured,
     is_safe_path,
-    upload_with_dropzonejs,
     browser_supports_modern_themes,
 )
 from settings import (
@@ -225,9 +226,8 @@ def index():
 
     devices = piscsi_cmd.list_devices()
     device_types = map_device_types_and_names(piscsi_cmd.get_device_types()["device_types"])
-    image_files = file_cmd.list_images()
+    image_files = piscsi_cmd.list_images()
     config_files = file_cmd.list_config_files()
-    ip_addr, host = sys_cmd.get_ip_and_host()
     formatted_image_files = format_image_list(
         image_files["files"], Path(server_info["image_dir"]).name, device_types
     )
@@ -315,7 +315,7 @@ def drive_list():
     return response(
         template="drives.html",
         page_title=_("PiSCSI Create Drive"),
-        files=file_cmd.list_images()["files"],
+        files=piscsi_cmd.list_images()["files"],
         drive_properties=format_drive_properties(APP.config["PISCSI_DRIVE_PROPERTIES"]),
     )
 
@@ -1035,7 +1035,41 @@ def upload_file():
     else:
         return make_response(_("Unknown destination"), 403)
 
-    return upload_with_dropzonejs(destination_dir)
+    log = logging.getLogger("pydrop")
+    file_object = request.files["file"]
+    file_name = secure_filename(file_object.filename)
+    tmp_file_name = "__tmp_" + file_name
+
+    save_path = path.join(destination_dir, file_name)
+    tmp_save_path = path.join(destination_dir, tmp_file_name)
+    current_chunk = int(request.form["dzchunkindex"])
+
+    # Makes sure not to overwrite an existing file,
+    # but continues writing to a file transfer in progress
+    if path.exists(save_path) and current_chunk == 0:
+        return make_response(_("The file already exists!"), 400)
+
+    try:
+        with open(tmp_save_path, "ab") as save:
+            save.seek(int(request.form["dzchunkbyteoffset"]))
+            save.write(file_object.stream.read())
+    except OSError:
+        log.exception("Could not write to file")
+        return make_response(_("Unable to write the file to disk!"), 500)
+
+    total_chunks = int(request.form["dztotalchunkcount"])
+
+    if current_chunk + 1 == total_chunks:
+        # Validate the resulting file size after writing the last chunk
+        if path.getsize(tmp_save_path) != int(request.form["dztotalfilesize"]):
+            log.error("File size mismatch between the original file and transferred file.")
+            return make_response(_("Transferred file corrupted!"), 500)
+
+        process = file_cmd.rename_file(Path(tmp_save_path), Path(save_path))
+        if not process["status"]:
+            return make_response(_("Unable to rename temporary file!"), 500)
+
+    return make_response(_("File upload successful!"), 200)
 
 
 @APP.route("/files/create", methods=["POST"])
@@ -1487,7 +1521,7 @@ if __name__ == "__main__":
 
     sock_cmd = SocketCmdsFlask(host=arguments.backend_host, port=arguments.backend_port)
     piscsi_cmd = PiscsiCmds(sock_cmd=sock_cmd, token=APP.config["PISCSI_TOKEN"])
-    file_cmd = FileCmds(sock_cmd=sock_cmd, piscsi=piscsi_cmd, token=APP.config["PISCSI_TOKEN"])
+    file_cmd = FileCmds(piscsi=piscsi_cmd)
     sys_cmd = SysCmds()
 
     if not piscsi_cmd.is_token_auth()["status"] and not APP.config["PISCSI_TOKEN"]:

python/web/src/web_utils.py

@@ -4,12 +4,11 @@ Module for PiSCSI Web Interface utility methods
 
 import logging
 from grp import getgrall
-from os import path
 from pathlib import Path
 from ua_parser import user_agent_parser
 from re import findall
 
-from flask import request, make_response, abort
+from flask import request, abort
 from flask_babel import _
 from werkzeug.utils import secure_filename
 
@@ -325,42 +324,6 @@ def is_safe_path(file_name):
     return {"status": True, "msg": ""}
 
 
-def upload_with_dropzonejs(image_dir):
-    """
-    Takes (str) image_dir which is the path to the image dir to store files.
-    Opens a stream to transfer a file via the embedded dropzonejs library.
-    """
-    log = logging.getLogger("pydrop")
-    file_object = request.files["file"]
-    file_name = secure_filename(file_object.filename)
-
-    save_path = path.join(image_dir, file_name)
-    current_chunk = int(request.form["dzchunkindex"])
-
-    # Makes sure not to overwrite an existing file,
-    # but continues writing to a file transfer in progress
-    if path.exists(save_path) and current_chunk == 0:
-        return make_response(_("The file already exists!"), 400)
-
-    try:
-        with open(save_path, "ab") as save:
-            save.seek(int(request.form["dzchunkbyteoffset"]))
-            save.write(file_object.stream.read())
-    except OSError:
-        log.exception("Could not write to file")
-        return make_response(_("Unable to write the file to disk!"), 500)
-
-    total_chunks = int(request.form["dztotalchunkcount"])
-
-    if current_chunk + 1 == total_chunks:
-        # Validate the resulting file size after writing the last chunk
-        if path.getsize(save_path) != int(request.form["dztotalfilesize"]):
-            log.error("File size mismatch between the original file and transferred file.")
-            return make_response(_("Transferred file corrupted!"), 500)
-
-    return make_response(_("File upload successful!"), 200)
-
-
 def browser_supports_modern_themes():
     """
     Determines if the browser supports the HTML/CSS/JS features used in non-legacy themes.