From 5f19bc34d3bae5d740dc0d4261353e84814da653 Mon Sep 17 00:00:00 2001 From: thomas-bc Date: Mon, 30 Oct 2023 13:41:44 -0700 Subject: [PATCH 1/8] WIP: remove flask_uploads --- src/fprime_gds/common/files/uplinker.py | 3 +- src/fprime_gds/common/pipeline/files.py | 10 ++-- src/fprime_gds/common/pipeline/standard.py | 7 ++- src/fprime_gds/executables/cli.py | 2 +- .../{flask_uploads.py => _flask_uploads.py} | 0 src/fprime_gds/flask/app.py | 8 +-- src/fprime_gds/flask/default_settings.py | 16 ++--- src/fprime_gds/flask/updown.py | 59 +++++++++++++++++-- .../common/testing_fw/api_unit_test.py | 4 +- 9 files changed, 81 insertions(+), 28 deletions(-) rename src/fprime_gds/flask/{flask_uploads.py => _flask_uploads.py} (100%) diff --git a/src/fprime_gds/common/files/uplinker.py b/src/fprime_gds/common/files/uplinker.py index d1ead4c6..5aad7a2c 100644 --- a/src/fprime_gds/common/files/uplinker.py +++ b/src/fprime_gds/common/files/uplinker.py @@ -147,7 +147,7 @@ class FileUplinker(fprime_gds.common.handlers.DataHandler): CHUNK_SIZE = 256 - def __init__(self, file_encoder, chunk=CHUNK_SIZE, timeout=20): + def __init__(self, file_store, file_encoder, chunk=CHUNK_SIZE, timeout=20): """ Constructor to build the file uplinker. """ @@ -157,6 +157,7 @@ def __init__(self, file_encoder, chunk=CHUNK_SIZE, timeout=20): self.sequence = 0 self.chunk = chunk self.file_encoder = file_encoder + self.file_store = file_store self.__destination_dir = "/" self.__expected = [] self.__timeout = Timeout() diff --git a/src/fprime_gds/common/pipeline/files.py b/src/fprime_gds/common/pipeline/files.py index f88b475c..7c70626e 100644 --- a/src/fprime_gds/common/pipeline/files.py +++ b/src/fprime_gds/common/pipeline/files.py @@ -25,21 +25,23 @@ def __init__(self): self.__downlinker = None def setup_file_handling( - self, down_store, file_encoder, file_decoder, distributor, log_dir + self, file_store, file_encoder, file_decoder, distributor, log_dir ): """ Sets up the file handling (uplink and downlink) from a pair of encoders and decoders. Raises a PermissionError if the down_store is not writable. - :param down_store: downlink storage directory + :param file_store: uplink/downlink storage directory :param file_encoder: file encoder for uplink :param file_decoder: file decoder for downlink :param distributor: data distributor to register handshaking to :param log_dir: log directory to output downlink logs """ - self.__uplinker = fprime_gds.common.files.uplinker.FileUplinker(file_encoder) + self.__uplinker = fprime_gds.common.files.uplinker.FileUplinker( + file_store, file_encoder # maybe not here after all?? + ) self.__downlinker = fprime_gds.common.files.downlinker.FileDownlinker( - down_store, log_dir=log_dir + file_store, log_dir=log_dir ) file_decoder.register(self.__downlinker) distributor.register("FW_PACKET_HAND", self.__uplinker) diff --git a/src/fprime_gds/common/pipeline/standard.py b/src/fprime_gds/common/pipeline/standard.py index 73548d5e..6ab2bea4 100644 --- a/src/fprime_gds/common/pipeline/standard.py +++ b/src/fprime_gds/common/pipeline/standard.py @@ -52,7 +52,7 @@ def __init__(self): self.__transport_type = ThreadedTCPSocketClient def setup( - self, config, dictionary, down_store, logging_prefix=None, packet_spec=None + self, config, dictionary, file_store, logging_prefix=None, packet_spec=None ): """ Setup the standard pipeline for moving data from the middleware layer through the GDS layers using the standard @@ -60,7 +60,7 @@ def setup( :param config: config object used when constructing the pipeline. :param dictionary: dictionary path. Used to setup loading of dictionaries. - :param down_store: downlink storage directory + :param file_store: uplink/downlink storage directory :param logging_prefix: logging prefix. Defaults to not logging at all. :param packet_spec: location of packetized telemetry XML specification. """ @@ -76,7 +76,8 @@ def setup( ) self.histories.setup_histories(self.coders) self.files.setup_file_handling( - down_store, + # here + file_store, self.coders.file_encoder, self.coders.file_decoder, self.distributor, diff --git a/src/fprime_gds/executables/cli.py b/src/fprime_gds/executables/cli.py index 171a2d94..4e3a2288 100644 --- a/src/fprime_gds/executables/cli.py +++ b/src/fprime_gds/executables/cli.py @@ -572,7 +572,7 @@ def pipeline_factory(args_ns, pipeline=None) -> StandardPipeline: pipeline_arguments = { "config": ConfigManager(), "dictionary": args_ns.dictionary, - "down_store": args_ns.files_directory, + "file_store": args_ns.files_directory, "packet_spec": args_ns.packet_spec, "logging_prefix": args_ns.logs, } diff --git a/src/fprime_gds/flask/flask_uploads.py b/src/fprime_gds/flask/_flask_uploads.py similarity index 100% rename from src/fprime_gds/flask/flask_uploads.py rename to src/fprime_gds/flask/_flask_uploads.py diff --git a/src/fprime_gds/flask/app.py b/src/fprime_gds/flask/app.py index f3d28e4a..f71b373b 100644 --- a/src/fprime_gds/flask/app.py +++ b/src/fprime_gds/flask/app.py @@ -30,7 +30,6 @@ import fprime_gds.flask.stats import fprime_gds.flask.updown from fprime_gds.executables.cli import ParserBase, StandardPipelineParser -from fprime_gds.flask import flask_uploads from . import components @@ -49,8 +48,7 @@ def construct_app(): 2. Setup JSON encoding for Flask and flask_restful to handle F prime types natively 3. Setup standard pipeline used throughout the system 4. Create Restful API for registering flask items - 5. Setup flask_uploads settings - 6. Register all restful endpoints + 5. Register all restful endpoints :return: setup app """ @@ -78,8 +76,6 @@ def construct_app(): # Restful API registration api = fprime_gds.flask.errors.setup_error_handling(app) # File upload configuration, 1 set for everything - uplink_set = flask_uploads.UploadSet("uplink", flask_uploads.ALL) - flask_uploads.configure_uploads(app, [uplink_set]) # Application routes api.add_resource( @@ -137,7 +133,7 @@ def construct_app(): api.add_resource( fprime_gds.flask.updown.FileUploads, "/upload/files", - resource_class_args=[pipeline.files.uplinker, uplink_set], + resource_class_args=[pipeline.files.uplinker, pipeline.file_store], ) api.add_resource( fprime_gds.flask.updown.FileDownload, diff --git a/src/fprime_gds/flask/default_settings.py b/src/fprime_gds/flask/default_settings.py index ee978982..5b8f8efd 100644 --- a/src/fprime_gds/flask/default_settings.py +++ b/src/fprime_gds/flask/default_settings.py @@ -12,20 +12,22 @@ import getpass # Select uploads directory and create it -username = getpass.getuser() -uplink_dir = os.environ.get("UP_FILES_DIR", "/tmp/" + username + "/fprime-uplink/") -DOWNLINK_DIR = os.environ.get("DOWN_FILES_DIR", "/tmp/" + username + "/fprime-downlink/") + +# username = getpass.getuser() +# uplink_dir = os.environ.get("UP_FILES_DIR", "/tmp/" + username + "/fprime-uplink/") +# DOWNLINK_DIR = os.environ.get("DOWN_FILES_DIR", "/tmp/" + username + "/fprime-downlink/") + STANDARD_PIPELINE_ARGUMENTS = os.environ.get("STANDARD_PIPELINE_ARGUMENTS").split("|") SERVE_LOGS = os.environ.get("SERVE_LOGS", "YES") == "YES" -UPLOADED_UPLINK_DEST = uplink_dir -UPLOADS_DEFAULT_DEST = uplink_dir +# UPLOADED_UPLINK_DEST = uplink_dir +# UPLOADS_DEFAULT_DEST = uplink_dir REMOTE_SEQ_DIRECTORY = "/seq" MAX_CONTENT_LENGTH = 32 * 1024 * 1024 # Max length of request is 32MiB -for directory in [UPLOADED_UPLINK_DEST, UPLOADS_DEFAULT_DEST, DOWNLINK_DIR]: - os.makedirs(directory, exist_ok=True) +# for directory in [UPLOADED_UPLINK_DEST, UPLOADS_DEFAULT_DEST, DOWNLINK_DIR]: +# os.makedirs(directory, exist_ok=True) # TODO: load real config diff --git a/src/fprime_gds/flask/updown.py b/src/fprime_gds/flask/updown.py index 9aa9d03d..0c005abc 100644 --- a/src/fprime_gds/flask/updown.py +++ b/src/fprime_gds/flask/updown.py @@ -11,6 +11,9 @@ import flask import flask_restful +from werkzeug.datastructures import FileStorage +from werkzeug.utils import secure_filename +from pathlib import Path class Destination(flask_restful.Resource): @@ -51,12 +54,13 @@ class FileUploads(flask_restful.Resource): A data model for the current uplinking file set. """ - def __init__(self, uplinker, uplink_set): + def __init__(self, uplinker, dest_dir): #, uplink_set): """ Constructor: setup the uplinker and argument parsing """ self.uplinker = uplinker - self.uplink_set = uplink_set + self.dest_dir = dest_dir / "fprime-uplink" + # self.uplink_set = uplink_set self.parser = flask_restful.reqparse.RequestParser() self.parser.add_argument( "action", required=True, help="Action to take against files" @@ -99,10 +103,10 @@ def post(self): failed = [] for key, file in flask.request.files.items(): try: - filename = self.uplink_set.save(file) + filename = self.save(file) flask.current_app.logger.info(f"Received file. Saved to: {filename}") self.uplinker.enqueue( - os.path.join(self.uplink_set.config.destination, filename) + os.path.join(self.dest_dir, filename) ) successful.append(key) except Exception as exc: @@ -112,6 +116,53 @@ def post(self): failed.append(key) return {"successful": successful, "failed": failed} + def save(self, file_storage: FileStorage): + """ + This saves a `werkzeug.FileStorage` into this upload set. + + :param file_storage: The uploaded file to save. + """ + if not isinstance(file_storage, FileStorage): + raise TypeError("file_storage must be a werkzeug.FileStorage") + + filename = Path(secure_filename(file_storage.filename)).name + dest_dir = Path(self.dest_dir) + + # not necessary + if not dest_dir.exists(): + dest_dir.mkdir(parents=True) + + # resolve conflict may not be needed + if (dest_dir / filename).exists(): + filename = self.resolve_conflict(dest_dir, filename) + + target = dest_dir / filename + file_storage.save(str(target)) + + return filename + + def resolve_conflict(self, target_folder: Path, filename: str): + """ + If a file with the selected name already exists in the target folder, + this method is called to resolve the conflict. It should return a new + filename for the file. + + The default implementation splits the name and extension and adds a + suffix to the name consisting of an underscore and a number, and tries + that until it finds one that doesn't exist. + + :param target_folder: The absolute path to the target. + :param filename: The file's original filename. + """ + path = Path(filename) + name, ext = path.stem, path.suffix + count = 0 + while True: + count = count + 1 + newname = f"{name}_{count}{ext}" + if not (Path(target_folder) / newname).exists(): + return newname + class FileDownload(flask_restful.Resource): """ """ diff --git a/test/fprime_gds/common/testing_fw/api_unit_test.py b/test/fprime_gds/common/testing_fw/api_unit_test.py index a4b5332a..b5c4fa63 100644 --- a/test/fprime_gds/common/testing_fw/api_unit_test.py +++ b/test/fprime_gds/common/testing_fw/api_unit_test.py @@ -77,8 +77,8 @@ def setUpClass(cls): cls.pipeline = UTPipeline() config = ConfigManager() path = os.path.join(os.path.dirname(__file__), "./UnitTestDictionary.xml") - down_store = os.path.join(os.path.dirname(__file__), "./") - cls.pipeline.setup(config, path, down_store) + file_store = os.path.join(os.path.dirname(__file__), "./") + cls.pipeline.setup(config, path, file_store) log_path = os.path.join(os.path.dirname(__file__), "./logs") cls.api = IntegrationTestAPI(cls.pipeline, log_path) cls.case_list = [] # TODO find a better way to do this. From 4f95e1e676c94040238eae2e93ff8b0d2d97d2ff Mon Sep 17 00:00:00 2001 From: thomas-bc Date: Wed, 22 Nov 2023 12:10:50 -0800 Subject: [PATCH 2/8] Cleanup --- src/fprime_gds/common/files/uplinker.py | 3 +- src/fprime_gds/common/pipeline/files.py | 8 +- src/fprime_gds/common/pipeline/standard.py | 9 +- src/fprime_gds/executables/cli.py | 10 +- src/fprime_gds/flask/_flask_uploads.py | 542 --------------------- src/fprime_gds/flask/app.py | 4 +- src/fprime_gds/flask/default_settings.py | 14 +- src/fprime_gds/flask/updown.py | 5 +- 8 files changed, 22 insertions(+), 573 deletions(-) delete mode 100644 src/fprime_gds/flask/_flask_uploads.py diff --git a/src/fprime_gds/common/files/uplinker.py b/src/fprime_gds/common/files/uplinker.py index 5aad7a2c..d1ead4c6 100644 --- a/src/fprime_gds/common/files/uplinker.py +++ b/src/fprime_gds/common/files/uplinker.py @@ -147,7 +147,7 @@ class FileUplinker(fprime_gds.common.handlers.DataHandler): CHUNK_SIZE = 256 - def __init__(self, file_store, file_encoder, chunk=CHUNK_SIZE, timeout=20): + def __init__(self, file_encoder, chunk=CHUNK_SIZE, timeout=20): """ Constructor to build the file uplinker. """ @@ -157,7 +157,6 @@ def __init__(self, file_store, file_encoder, chunk=CHUNK_SIZE, timeout=20): self.sequence = 0 self.chunk = chunk self.file_encoder = file_encoder - self.file_store = file_store self.__destination_dir = "/" self.__expected = [] self.__timeout = Timeout() diff --git a/src/fprime_gds/common/pipeline/files.py b/src/fprime_gds/common/pipeline/files.py index 7c70626e..5e3e6a19 100644 --- a/src/fprime_gds/common/pipeline/files.py +++ b/src/fprime_gds/common/pipeline/files.py @@ -25,23 +25,23 @@ def __init__(self): self.__downlinker = None def setup_file_handling( - self, file_store, file_encoder, file_decoder, distributor, log_dir + self, down_store, file_encoder, file_decoder, distributor, log_dir ): """ Sets up the file handling (uplink and downlink) from a pair of encoders and decoders. Raises a PermissionError if the down_store is not writable. - :param file_store: uplink/downlink storage directory + :param down_store: downlink storage directory :param file_encoder: file encoder for uplink :param file_decoder: file decoder for downlink :param distributor: data distributor to register handshaking to :param log_dir: log directory to output downlink logs """ self.__uplinker = fprime_gds.common.files.uplinker.FileUplinker( - file_store, file_encoder # maybe not here after all?? + file_encoder ) self.__downlinker = fprime_gds.common.files.downlinker.FileDownlinker( - file_store, log_dir=log_dir + down_store, log_dir=log_dir ) file_decoder.register(self.__downlinker) distributor.register("FW_PACKET_HAND", self.__uplinker) diff --git a/src/fprime_gds/common/pipeline/standard.py b/src/fprime_gds/common/pipeline/standard.py index 6ab2bea4..fbe9ad7b 100644 --- a/src/fprime_gds/common/pipeline/standard.py +++ b/src/fprime_gds/common/pipeline/standard.py @@ -44,6 +44,9 @@ def __init__(self): self.client_socket = None self.logger = None self.dictionary_path = None + self.up_store = None + self.down_store = None + self.__dictionaries = dictionaries.Dictionaries() self.__coders = encoding.EncodingDecoding() @@ -65,6 +68,9 @@ def setup( :param packet_spec: location of packetized telemetry XML specification. """ assert dictionary is not None and Path(dictionary).is_file(), f"Dictionary {dictionary} does not exist" + # File storage configuration for uplink and downlink + self.up_store = Path(file_store) / "fprime-uplink" + self.down_store = Path(file_store) / "fprime-downlink" self.dictionary_path = Path(dictionary) # Loads the distributor and client socket self.distributor = fprime_gds.common.distributor.distributor.Distributor(config) @@ -76,8 +82,7 @@ def setup( ) self.histories.setup_histories(self.coders) self.files.setup_file_handling( - # here - file_store, + self.down_store, self.coders.file_encoder, self.coders.file_decoder, self.distributor, diff --git a/src/fprime_gds/executables/cli.py b/src/fprime_gds/executables/cli.py index 4e3a2288..ccb0b4f6 100644 --- a/src/fprime_gds/executables/cli.py +++ b/src/fprime_gds/executables/cli.py @@ -535,18 +535,18 @@ def get_arguments(self) -> Dict[Tuple[str, ...], Dict[str, Any]]: return { ("--file-storage-directory",): { - "dest": "files_directory", + "dest": "files_storage_directory", "action": "store", - "default": "/tmp/" + username + "/fprime-downlink/", + "default": "/tmp/" + username, "required": False, "type": str, - "help": "File to store uplink and downlink files. Default: %(default)s", + "help": "Directory to store uplink and downlink files. Default: %(default)s", } } def handle_arguments(self, args, **kwargs): """Handle arguments as parsed""" - os.makedirs(args.files_directory, exist_ok=True) + os.makedirs(args.files_storage_directory, exist_ok=True) return args @@ -572,7 +572,7 @@ def pipeline_factory(args_ns, pipeline=None) -> StandardPipeline: pipeline_arguments = { "config": ConfigManager(), "dictionary": args_ns.dictionary, - "file_store": args_ns.files_directory, + "file_store": args_ns.files_storage_directory, "packet_spec": args_ns.packet_spec, "logging_prefix": args_ns.logs, } diff --git a/src/fprime_gds/flask/_flask_uploads.py b/src/fprime_gds/flask/_flask_uploads.py deleted file mode 100644 index 5d85cdf2..00000000 --- a/src/fprime_gds/flask/_flask_uploads.py +++ /dev/null @@ -1,542 +0,0 @@ -# -*- coding: utf-8 -*- -""" -flaskext.uploads -================ -This module provides upload support for Flask. The basic pattern is to set up -an `UploadSet` object and upload your files to it. - -:copyright: 2010 Matthew "LeafStorm" Frazier -:license: MIT/X11, see LICENSE for details - -Note: originally from https://github.com/maxcountryman/flask-uploads -""" - -import sys - -PY3 = sys.version_info[0] == 3 - -if PY3: - string_types = (str,) -else: - string_types = (basestring,) - -import os.path -import posixpath -from itertools import chain # lgtm [py/unused-import] - -from flask import Blueprint, abort, current_app, send_from_directory, url_for -from werkzeug.datastructures import FileStorage -from werkzeug.utils import secure_filename - -# Extension presets - -#: This just contains plain text files (.txt). -TEXT = ("txt",) - -#: This contains various office document formats (.rtf, .odf, .ods, .gnumeric, -#: .abw, .doc, .docx, .xls, .xlsx and .pdf). Note that the macro-enabled versions -#: of Microsoft Office 2007 files are not included. -DOCUMENTS = tuple("rtf odf ods gnumeric abw doc docx xls xlsx pdf".split()) - -#: This contains basic image types that are viewable from most browsers (.jpg, -#: .jpe, .jpeg, .png, .gif, .svg, .bmp and .webp). -IMAGES = tuple("jpg jpe jpeg png gif svg bmp webp".split()) - -#: This contains audio file types (.wav, .mp3, .aac, .ogg, .oga, and .flac). -AUDIO = tuple("wav mp3 aac ogg oga flac".split()) - -#: This is for structured data files (.csv, .ini, .json, .plist, .xml, .yaml, -#: and .yml). -DATA = tuple("csv ini json plist xml yaml yml".split()) - -#: This contains various types of scripts (.js, .php, .pl, .py .rb, and .sh). -#: If your Web server has PHP installed and set to auto-run, you might want to -#: add ``php`` to the DENY setting. -SCRIPTS = tuple("js php pl py rb sh".split()) - -#: This contains archive and compression formats (.gz, .bz2, .zip, .tar, -#: .tgz, .txz, and .7z). -ARCHIVES = tuple("gz bz2 zip tar tgz txz 7z".split()) - -#: This contains nonexecutable source files - those which need to be -#: compiled or assembled to binaries to be used. They are generally safe to -#: accept, as without an existing RCE vulnerability, they cannot be compiled, -#: assembled, linked, or executed. Supports C, C++, Ada, Rust, Go (Golang), -#: FORTRAN, D, Java, C Sharp, F Sharp (compiled only), COBOL, Haskell, and -#: assembly. -SOURCE = tuple( - ( - "c cpp c++ h hpp h++ cxx hxx hdl " # C/C++ - + "ada " # Ada - + "rs " # Rust - + "go " # Go - + "f for f90 f95 f03 " # FORTRAN - + "d dd di " # D - + "java " # Java - + "hs " # Haskell - + "cs " # C Sharp - + "fs " # F Sharp compiled source (NOT .fsx, which is interactive-ready) - + "cbl cob " # COBOL - + "asm s " # Assembly - ).split() -) - -#: This contains shared libraries and executable files (.so, .exe and .dll). -#: Most of the time, you will not want to allow this - it's better suited for -#: use with `AllExcept`. -EXECUTABLES = tuple("so exe dll".split()) - -#: The default allowed extensions - `TEXT`, `DOCUMENTS`, `DATA`, and `IMAGES`. -DEFAULTS = TEXT + DOCUMENTS + IMAGES + DATA - - -class UploadNotAllowed(Exception): - """ - This exception is raised if the upload was not allowed. You should catch - it in your view code and display an appropriate message to the user. - """ - - -def tuple_from(*iters): - return tuple(itertools.chain(*iters)) - - -def extension(filename): - ext = os.path.splitext(filename)[1] - if ext == "": - # add non-ascii filename support - ext = os.path.splitext(filename)[0] - if ext.startswith("."): - # os.path.splitext retains . separator - ext = ext[1:] - return ext - - -def lowercase_ext(filename): - """ - This is a helper used by UploadSet.save to provide lowercase extensions for - all processed files, to compare with configured extensions in the same - case. - - .. versionchanged:: 0.1.4 - Filenames without extensions are no longer lowercased, only the - extension is returned in lowercase, if an extension exists. - - :param filename: The filename to ensure has a lowercase extension. - """ - if "." in filename: - main, ext = os.path.splitext(filename) - return main + ext.lower() - # For consistency with os.path.splitext, - # do not treat a filename without an extension as an extension. - # That is, do not return filename.lower(). - return filename - - -def addslash(url): - return url if url.endswith("/") else f"{url}/" - - -def patch_request_class(app, size=64 * 1024 * 1024): - """ - By default, Flask will accept uploads to an arbitrary size. While Werkzeug - switches uploads from memory to a temporary file when they hit 500 KiB, - it's still possible for someone to overload your disk space with a - gigantic file. - - This patches the app's request class's - `~werkzeug.BaseRequest.max_content_length` attribute so that any upload - larger than the given size is rejected with an HTTP error. - - .. note:: - - In Flask 0.6, you can do this by setting the `MAX_CONTENT_LENGTH` - setting, without patching the request class. To emulate this behavior, - you can pass `None` as the size (you must pass it explicitly). That is - the best way to call this function, as it won't break the Flask 0.6 - functionality if it exists. - - .. versionchanged:: 0.1.1 - - :param app: The app to patch the request class of. - :param size: The maximum size to accept, in bytes. The default is 64 MiB. - If it is `None`, the app's `MAX_CONTENT_LENGTH` configuration - setting will be used to patch. - """ - if size is None: - if isinstance(app.request_class.__dict__["max_content_length"], property): - return - size = app.config.get("MAX_CONTENT_LENGTH") - reqclass = app.request_class - patched = type(reqclass.__name__, (reqclass,), {"max_content_length": size}) - app.request_class = patched - - -def config_for_set(uset, app, defaults=None): - """ - This is a helper function for `configure_uploads` that extracts the - configuration for a single set. - - :param uset: The upload set. - :param app: The app to load the configuration from. - :param defaults: A dict with keys `url` and `dest` from the - `UPLOADS_DEFAULT_DEST` and `DEFAULT_UPLOADS_URL` - settings. - """ - config = app.config - prefix = f'UPLOADED_{uset.name.upper()}_' - using_defaults = False - if defaults is None: - defaults = dict(dest=None, url=None) - - allow_extns = tuple(config.get(prefix + "ALLOW", ())) - deny_extns = tuple(config.get(prefix + "DENY", ())) - destination = config.get(prefix + "DEST") - base_url = config.get(prefix + "URL") - - if destination is None: - # the upload set's destination wasn't given - if uset.default_dest: - # use the "default_dest" callable - destination = uset.default_dest(app) - if destination is None: # still - # use the default dest from the config - if defaults["dest"] is not None: - using_defaults = True - destination = os.path.join(defaults["dest"], uset.name) - else: - msg = f"no destination for set {uset.name}" - raise RuntimeError(msg) - - if base_url is None and using_defaults and defaults["url"]: - base_url = addslash(defaults["url"]) + uset.name + "/" - - return UploadConfiguration(destination, base_url, allow_extns, deny_extns) - - -def configure_uploads(app, upload_sets): - """ - Call this after the app has been configured. It will go through all the - upload sets, get their configuration, and store the configuration on the - app. It will also register the uploads module if it hasn't been set. This - can be called multiple times with different upload sets. - - .. versionchanged:: 0.1.3 - The uploads module/blueprint will only be registered if it is needed - to serve the upload sets. - - :param app: The `~flask.Flask` instance to get the configuration from. - :param upload_sets: The `UploadSet` instances to configure. - """ - if isinstance(upload_sets, UploadSet): - upload_sets = (upload_sets,) - - if not hasattr(app, "upload_set_config"): - app.upload_set_config = {} - set_config = app.upload_set_config - defaults = dict( - dest=app.config.get("UPLOADS_DEFAULT_DEST"), - url=app.config.get("UPLOADS_DEFAULT_URL"), - ) - - for uset in upload_sets: - config = config_for_set(uset, app, defaults) - set_config[uset.name] = config - - should_serve = any(s.base_url is None for s in set_config.values()) - if "_uploads" not in app.blueprints and should_serve: - app.register_blueprint(uploads_mod) - - -class All(object): - """ - This type can be used to allow all extensions. There is a predefined - instance named `ALL`. - """ - - def __contains__(self, item): - return True - - -#: This "contains" all items. You can use it to allow all extensions to be -#: uploaded. -ALL = All() - - -class AllExcept(object): - """ - This can be used to allow all file types except certain ones. For example, - to ban .exe and .iso files, pass:: - - AllExcept(('exe', 'iso')) - - to the `UploadSet` constructor as `extensions`. You can use any container, - for example:: - - AllExcept(SCRIPTS + EXECUTABLES) - """ - - def __init__(self, items): - self.items = items - - def __contains__(self, item): - return item not in self.items - - -class UploadConfiguration(object): - """ - This holds the configuration for a single `UploadSet`. The constructor's - arguments are also the attributes. - - :param destination: The directory to save files to. - :param base_url: The URL (ending with a /) that files can be downloaded - from. If this is `None`, Flask-Uploads will serve the - files itself. - :param allow: A list of extensions to allow, even if they're not in the - `UploadSet` extensions list. - :param deny: A list of extensions to deny, even if they are in the - `UploadSet` extensions list. - """ - - def __init__(self, destination, base_url=None, allow=(), deny=()): - self.destination = destination - self.base_url = base_url - self.allow = allow - self.deny = deny - - @property - def tuple(self): - return (self.destination, self.base_url, self.allow, self.deny) - - def __eq__(self, other): - return self.tuple == other.tuple - - -class UploadSet(object): - """ - This represents a single set of uploaded files. Each upload set is - independent of the others. This can be reused across multiple application - instances, as all configuration is stored on the application object itself - and found with `flask.current_app`. - - :param name: The name of this upload set. It defaults to ``files``, but - you can pick any alphanumeric name you want. (For simplicity, - it's best to use a plural noun.) - :param extensions: The extensions to allow uploading in this set. The - easiest way to do this is to add together the extension - presets (for example, ``TEXT + DOCUMENTS + IMAGES``). - It can be overridden by the configuration with the - `UPLOADED_X_ALLOW` and `UPLOADED_X_DENY` configuration - parameters. The default is `DEFAULTS`. - :param default_dest: If given, this should be a callable. If you call it - with the app, it should return the default upload - destination path for that app. - """ - - def __init__(self, name="files", extensions=DEFAULTS, default_dest=None): - if not name.isalnum(): - raise ValueError("Name must be alphanumeric (no underscores)") - self.name = name - self.extensions = extensions - self._config = None - self.default_dest = default_dest - - @property - def config(self): - """ - This gets the current configuration. By default, it looks up the - current application and gets the configuration from there. But if you - don't want to go to the full effort of setting an application, or it's - otherwise outside of a request context, set the `_config` attribute to - an `UploadConfiguration` instance, then set it back to `None` when - you're done. - """ - if self._config is not None: - return self._config - try: - return current_app.upload_set_config[self.name] - except AttributeError: - raise RuntimeError("cannot access configuration outside request") - - def url(self, filename): - """ - This function gets the URL a file uploaded to this set would be - accessed at. It doesn't check whether said file exists. - - :param filename: The filename to return the URL for. - """ - base = self.config.base_url - if base is None: - return url_for( - "_uploads.uploaded_file", - setname=self.name, - filename=filename, - _external=True, - ) - return base + filename - - def path(self, filename, folder=None): - """ - This returns the absolute path of a file uploaded to this set. It - doesn't actually check whether said file exists. - - :param filename: The filename to return the path for. - :param folder: The subfolder within the upload set previously used - to save to. - """ - if folder is not None: - target_folder = os.path.join(self.config.destination, folder) - else: - target_folder = self.config.destination - return os.path.join(target_folder, filename) - - def file_allowed(self, storage, basename): - """ - This tells whether a file is allowed. It should return `True` if the - given `werkzeug.FileStorage` object can be saved with the given - basename, and `False` if it can't. The default implementation just - checks the extension, so you can override this if you want. - - :param storage: The `werkzeug.FileStorage` to check. - :param basename: The basename it will be saved under. - """ - return self.extension_allowed(extension(basename)) - - def extension_allowed(self, ext): - """ - This determines whether a specific extension is allowed. It is called - by `file_allowed`, so if you override that but still want to check - extensions, call back into this. - - :param ext: The extension to check, without the dot. - """ - return (ext in self.config.allow) or ( - ext in self.extensions and ext not in self.config.deny - ) - - def get_basename(self, filename): - return lowercase_ext(secure_filename(filename)) - - def save(self, storage, folder=None, name=None): - """ - This saves a `werkzeug.FileStorage` into this upload set. If the - upload is not allowed, an `UploadNotAllowed` error will be raised. - Otherwise, the file will be saved and its name (including the folder) - will be returned. - - :param storage: The uploaded file to save. - :param folder: The subfolder within the upload set to save to. - :param name: The name to save the file as. If it ends with a dot, the - file's extension will be appended to the end. (If you - are using `name`, you can include the folder in the - `name` instead of explicitly using `folder`, i.e. - ``uset.save(file, name="someguy/photo_123.")`` - """ - if not isinstance(storage, FileStorage): - raise TypeError("storage must be a werkzeug.FileStorage") - - if folder is None and name is not None and "/" in name: - folder, name = os.path.split(name) - - basename = self.get_basename(storage.filename) - - if not self.file_allowed(storage, basename): - raise UploadNotAllowed() - - if name: - basename = name + extension(basename) if name.endswith(".") else name - - if folder: - target_folder = os.path.join(self.config.destination, folder) - else: - target_folder = self.config.destination - if not os.path.exists(target_folder): - os.makedirs(target_folder) - if os.path.exists(os.path.join(target_folder, basename)): - basename = self.resolve_conflict(target_folder, basename) - - target = os.path.join(target_folder, basename) - storage.save(target) - return posixpath.join(folder, basename) if folder else basename - - def resolve_conflict(self, target_folder, basename): - """ - If a file with the selected name already exists in the target folder, - this method is called to resolve the conflict. It should return a new - basename for the file. - - The default implementation splits the name and extension and adds a - suffix to the name consisting of an underscore and a number, and tries - that until it finds one that doesn't exist. - - :param target_folder: The absolute path to the target. - :param basename: The file's original basename. - """ - name, ext = os.path.splitext(basename) - count = 0 - while True: - count = count + 1 - newname = "%s_%d%s" % (name, count, ext) - if not os.path.exists(os.path.join(target_folder, newname)): - return newname - - -uploads_mod = Blueprint("_uploads", __name__, url_prefix="/_uploads") - - -@uploads_mod.route("//") -def uploaded_file(setname, filename): - config = current_app.upload_set_config.get(setname) - if config is None: - abort(404) - return send_from_directory(config.destination, filename) - - -class TestingFileStorage(FileStorage): - """ - This is a helper for testing upload behavior in your application. You - can manually create it, and its save method is overloaded to set `saved` - to the name of the file it was saved to. All of these parameters are - optional, so only bother setting the ones relevant to your application. - - :param stream: A stream. The default is an empty stream. - :param filename: The filename uploaded from the client. The default is the - stream's name. - :param name: The name of the form field it was loaded from. The default is - `None`. - :param content_type: The content type it was uploaded as. The default is - ``application/octet-stream``. - :param content_length: How long it is. The default is -1. - :param headers: Multipart headers as a `werkzeug.Headers`. The default is - `None`. - """ - - def __init__( - self, - stream=None, - filename=None, - name=None, - content_type="application/octet-stream", - content_length=-1, - headers=None, - ): - FileStorage.__init__( - self, - stream, - filename, - name=name, - content_type=content_type, - content_length=content_length, - headers=None, - ) - self.saved = None - - def save(self, dst, buffer_size=16384): - """ - This marks the file as saved by setting the `saved` attribute to the - name of the file it was saved to. - - :param dst: The file to save to. - :param buffer_size: Ignored. - """ - self.saved = dst if isinstance(dst, string_types) else dst.name diff --git a/src/fprime_gds/flask/app.py b/src/fprime_gds/flask/app.py index f71b373b..82cdcd31 100644 --- a/src/fprime_gds/flask/app.py +++ b/src/fprime_gds/flask/app.py @@ -133,7 +133,7 @@ def construct_app(): api.add_resource( fprime_gds.flask.updown.FileUploads, "/upload/files", - resource_class_args=[pipeline.files.uplinker, pipeline.file_store], + resource_class_args=[pipeline.files.uplinker, pipeline.up_store], ) api.add_resource( fprime_gds.flask.updown.FileDownload, @@ -146,7 +146,7 @@ def construct_app(): "/sequence", resource_class_args=[ args_ns.dictionary, - app.config["UPLOADED_UPLINK_DEST"], + pipeline.up_store, pipeline.files.uplinker, app.config["REMOTE_SEQ_DIRECTORY"], ], diff --git a/src/fprime_gds/flask/default_settings.py b/src/fprime_gds/flask/default_settings.py index 5b8f8efd..b1aea337 100644 --- a/src/fprime_gds/flask/default_settings.py +++ b/src/fprime_gds/flask/default_settings.py @@ -9,25 +9,13 @@ # #### import os -import getpass - -# Select uploads directory and create it - -# username = getpass.getuser() -# uplink_dir = os.environ.get("UP_FILES_DIR", "/tmp/" + username + "/fprime-uplink/") -# DOWNLINK_DIR = os.environ.get("DOWN_FILES_DIR", "/tmp/" + username + "/fprime-downlink/") - STANDARD_PIPELINE_ARGUMENTS = os.environ.get("STANDARD_PIPELINE_ARGUMENTS").split("|") SERVE_LOGS = os.environ.get("SERVE_LOGS", "YES") == "YES" -# UPLOADED_UPLINK_DEST = uplink_dir -# UPLOADS_DEFAULT_DEST = uplink_dir + REMOTE_SEQ_DIRECTORY = "/seq" MAX_CONTENT_LENGTH = 32 * 1024 * 1024 # Max length of request is 32MiB -# for directory in [UPLOADED_UPLINK_DEST, UPLOADS_DEFAULT_DEST, DOWNLINK_DIR]: -# os.makedirs(directory, exist_ok=True) - # TODO: load real config diff --git a/src/fprime_gds/flask/updown.py b/src/fprime_gds/flask/updown.py index 0c005abc..fdbda90c 100644 --- a/src/fprime_gds/flask/updown.py +++ b/src/fprime_gds/flask/updown.py @@ -54,13 +54,12 @@ class FileUploads(flask_restful.Resource): A data model for the current uplinking file set. """ - def __init__(self, uplinker, dest_dir): #, uplink_set): + def __init__(self, uplinker, dest_dir): """ Constructor: setup the uplinker and argument parsing """ self.uplinker = uplinker - self.dest_dir = dest_dir / "fprime-uplink" - # self.uplink_set = uplink_set + self.dest_dir = dest_dir self.parser = flask_restful.reqparse.RequestParser() self.parser.add_argument( "action", required=True, help="Action to take against files" From 823af2fad33e5beb398c4111da35f2ce83660987 Mon Sep 17 00:00:00 2001 From: thomas-bc Date: Wed, 22 Nov 2023 12:11:24 -0800 Subject: [PATCH 3/8] Add option for remote sequence storage --- src/fprime_gds/executables/cli.py | 8 ++++++++ src/fprime_gds/flask/app.py | 2 +- src/fprime_gds/flask/default_settings.py | 1 - 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/fprime_gds/executables/cli.py b/src/fprime_gds/executables/cli.py index ccb0b4f6..fc8118a5 100644 --- a/src/fprime_gds/executables/cli.py +++ b/src/fprime_gds/executables/cli.py @@ -541,6 +541,14 @@ def get_arguments(self) -> Dict[Tuple[str, ...], Dict[str, Any]]: "required": False, "type": str, "help": "Directory to store uplink and downlink files. Default: %(default)s", + }, + ("--remote-sequence-directory",): { + "dest": "remote_sequence_directory", + "action": "store", + "default": "/seq", + "required": False, + "type": str, + "help": "Directory to save command sequence binaries, on the remote FSW. Default: %(default)s", } } diff --git a/src/fprime_gds/flask/app.py b/src/fprime_gds/flask/app.py index 82cdcd31..4be06db7 100644 --- a/src/fprime_gds/flask/app.py +++ b/src/fprime_gds/flask/app.py @@ -148,7 +148,7 @@ def construct_app(): args_ns.dictionary, pipeline.up_store, pipeline.files.uplinker, - app.config["REMOTE_SEQ_DIRECTORY"], + args_ns.remote_sequence_directory, ], ) api.add_resource( diff --git a/src/fprime_gds/flask/default_settings.py b/src/fprime_gds/flask/default_settings.py index b1aea337..1d2cca0a 100644 --- a/src/fprime_gds/flask/default_settings.py +++ b/src/fprime_gds/flask/default_settings.py @@ -14,7 +14,6 @@ SERVE_LOGS = os.environ.get("SERVE_LOGS", "YES") == "YES" -REMOTE_SEQ_DIRECTORY = "/seq" MAX_CONTENT_LENGTH = 32 * 1024 * 1024 # Max length of request is 32MiB From a0d4fad38f2e9fcb8a455072e4d1001552e5b324 Mon Sep 17 00:00:00 2001 From: thomas-bc Date: Wed, 22 Nov 2023 12:15:43 -0800 Subject: [PATCH 4/8] Formatting --- src/fprime_gds/common/files/uplinker.py | 8 ++++---- src/fprime_gds/common/pipeline/files.py | 4 +--- src/fprime_gds/common/pipeline/standard.py | 11 ++++++++--- src/fprime_gds/executables/cli.py | 19 ++++++++++--------- src/fprime_gds/flask/updown.py | 6 ++---- 5 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/fprime_gds/common/files/uplinker.py b/src/fprime_gds/common/files/uplinker.py index d1ead4c6..05fd11c7 100644 --- a/src/fprime_gds/common/files/uplinker.py +++ b/src/fprime_gds/common/files/uplinker.py @@ -47,7 +47,9 @@ def __init__(self, uplinker): self.queue = queue.Queue() self.__file_store = [] self.__exit = threading.Event() - self.__thread = threading.Thread(target=self.run, name="UplinkerThread", args=()) + self.__thread = threading.Thread( + target=self.run, name="UplinkerThread", args=() + ) self.__thread.start() def enqueue(self, filepath, destination): @@ -228,9 +230,7 @@ def start(self, file_obj): # Prevent multiple uplinks at once if self.state != FileStates.IDLE: msg = f"Currently uplinking file '{self.active.source}' cannot start uplinking '{file_obj.source}'" - raise FileUplinkerBusyException( - msg - ) + raise FileUplinkerBusyException(msg) self.state = FileStates.RUNNING self.active = file_obj self.active.open(TransmitFileState.READ) diff --git a/src/fprime_gds/common/pipeline/files.py b/src/fprime_gds/common/pipeline/files.py index 5e3e6a19..f88b475c 100644 --- a/src/fprime_gds/common/pipeline/files.py +++ b/src/fprime_gds/common/pipeline/files.py @@ -37,9 +37,7 @@ def setup_file_handling( :param distributor: data distributor to register handshaking to :param log_dir: log directory to output downlink logs """ - self.__uplinker = fprime_gds.common.files.uplinker.FileUplinker( - file_encoder - ) + self.__uplinker = fprime_gds.common.files.uplinker.FileUplinker(file_encoder) self.__downlinker = fprime_gds.common.files.downlinker.FileDownlinker( down_store, log_dir=log_dir ) diff --git a/src/fprime_gds/common/pipeline/standard.py b/src/fprime_gds/common/pipeline/standard.py index fbe9ad7b..242f12ae 100644 --- a/src/fprime_gds/common/pipeline/standard.py +++ b/src/fprime_gds/common/pipeline/standard.py @@ -47,7 +47,6 @@ def __init__(self): self.up_store = None self.down_store = None - self.__dictionaries = dictionaries.Dictionaries() self.__coders = encoding.EncodingDecoding() self.__histories = histories.Histories() @@ -67,7 +66,9 @@ def setup( :param logging_prefix: logging prefix. Defaults to not logging at all. :param packet_spec: location of packetized telemetry XML specification. """ - assert dictionary is not None and Path(dictionary).is_file(), f"Dictionary {dictionary} does not exist" + assert ( + dictionary is not None and Path(dictionary).is_file() + ), f"Dictionary {dictionary} does not exist" # File storage configuration for uplink and downlink self.up_store = Path(file_store) / "fprime-uplink" self.down_store = Path(file_store) / "fprime-downlink" @@ -158,7 +159,11 @@ def connect( outgoing_tag: this pipeline will produce data for supplied tag (FSW, GUI). Default: FSW """ # Backwards compatibility with the old method .connect(host, port) - if isinstance(incoming_tag, int) and ":" not in connection_uri and outgoing_tag == RoutingTag.FSW: + if ( + isinstance(incoming_tag, int) + and ":" not in connection_uri + and outgoing_tag == RoutingTag.FSW + ): connection_uri = f"{connection_uri}:{incoming_tag}" incoming_tag = RoutingTag.GUI self.client_socket.connect(connection_uri, incoming_tag, outgoing_tag) diff --git a/src/fprime_gds/executables/cli.py b/src/fprime_gds/executables/cli.py index fc8118a5..a24eeb96 100644 --- a/src/fprime_gds/executables/cli.py +++ b/src/fprime_gds/executables/cli.py @@ -226,12 +226,16 @@ def handle_arguments(self, args, **kwargs): if likely_deployment.exists(): args.deployment = likely_deployment return args - child_directories = [child for child in detected_toolchain.iterdir() if child.is_dir()] + child_directories = [ + child for child in detected_toolchain.iterdir() if child.is_dir() + ] if not child_directories: msg = f"No deployments found in {detected_toolchain}. Specify deployment with: --deployment" raise Exception(msg) # Works for the old structure where the bin, lib, and dict directories live immediately under the platform - elif len(child_directories) == 3 and set([path.name for path in child_directories]) == {"bin", "lib", "dict"}: + elif len(child_directories) == 3 and set( + [path.name for path in child_directories] + ) == {"bin", "lib", "dict"}: args.deployment = detected_toolchain return args elif len(child_directories) > 1: @@ -310,8 +314,7 @@ def get_arguments(self) -> Dict[Tuple[str, ...], Dict[str, Any]]: "action": "store", "type": str, "help": "Adapter for communicating to flight deployment. [default: %(default)s]", - "choices": ["none"] - + list(adapter_definition_dictionaries), + "choices": ["none"] + list(adapter_definition_dictionaries), "default": "ip", }, ("--comm-checksum-type",): { @@ -549,7 +552,7 @@ def get_arguments(self) -> Dict[Tuple[str, ...], Dict[str, Any]]: "required": False, "type": str, "help": "Directory to save command sequence binaries, on the remote FSW. Default: %(default)s", - } + }, } def handle_arguments(self, args, **kwargs): @@ -707,9 +710,7 @@ def handle_arguments(self, args, **kwargs): args.app = Path(args.app) if args.app else Path(find_app(args.deployment)) if not args.app.is_file(): msg = f"F prime binary '{args.app}' does not exist or is not a file" - raise ValueError( - msg - ) + raise ValueError(msg) return args @@ -736,7 +737,7 @@ def get_arguments(self) -> Dict[Tuple[str, ...], Dict[str, Any]]: "action": "store", "required": False, "type": int, - "nargs":'+', + "nargs": "+", "help": f"only show {self.command_name} matching the given type ID(s) 'ID'; can provide multiple IDs to show all given types", "metavar": "ID", }, diff --git a/src/fprime_gds/flask/updown.py b/src/fprime_gds/flask/updown.py index fdbda90c..596d9bb6 100644 --- a/src/fprime_gds/flask/updown.py +++ b/src/fprime_gds/flask/updown.py @@ -104,9 +104,7 @@ def post(self): try: filename = self.save(file) flask.current_app.logger.info(f"Received file. Saved to: {filename}") - self.uplinker.enqueue( - os.path.join(self.dest_dir, filename) - ) + self.uplinker.enqueue(os.path.join(self.dest_dir, filename)) successful.append(key) except Exception as exc: flask.current_app.logger.warning( @@ -131,7 +129,7 @@ def save(self, file_storage: FileStorage): if not dest_dir.exists(): dest_dir.mkdir(parents=True) - # resolve conflict may not be needed + # resolve conflict may not be needed if (dest_dir / filename).exists(): filename = self.resolve_conflict(dest_dir, filename) From d90a8a41886e904074c4a81e43348064f06ae4c8 Mon Sep 17 00:00:00 2001 From: thomas-bc Date: Wed, 22 Nov 2023 12:17:57 -0800 Subject: [PATCH 5/8] Remove legacy comment --- src/fprime_gds/flask/app.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/fprime_gds/flask/app.py b/src/fprime_gds/flask/app.py index 4be06db7..632e9881 100644 --- a/src/fprime_gds/flask/app.py +++ b/src/fprime_gds/flask/app.py @@ -75,7 +75,6 @@ def construct_app(): # Restful API registration api = fprime_gds.flask.errors.setup_error_handling(app) - # File upload configuration, 1 set for everything # Application routes api.add_resource( From 16dd40c7f38b9c60b55365be3d77cc54daf9de25 Mon Sep 17 00:00:00 2001 From: thomas-bc Date: Wed, 22 Nov 2023 12:57:06 -0800 Subject: [PATCH 6/8] Update workflow versions --- .github/workflows/fprime-gds-tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/fprime-gds-tests.yml b/.github/workflows/fprime-gds-tests.yml index f3592e0f..c030e632 100644 --- a/.github/workflows/fprime-gds-tests.yml +++ b/.github/workflows/fprime-gds-tests.yml @@ -11,9 +11,9 @@ jobs: python-version: ["3.8", "3.9", "3.10", "3.11"] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} - name: Install dependencies From f960b612e6793c87dfb6c784f6ca27c1db22ce49 Mon Sep 17 00:00:00 2001 From: thomas-bc Date: Wed, 10 Jan 2024 11:51:07 -0800 Subject: [PATCH 7/8] Fix permission check issues --- src/fprime_gds/common/pipeline/files.py | 9 +++++---- src/fprime_gds/common/pipeline/standard.py | 7 +++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/fprime_gds/common/pipeline/files.py b/src/fprime_gds/common/pipeline/files.py index f88b475c..ff2ee95e 100644 --- a/src/fprime_gds/common/pipeline/files.py +++ b/src/fprime_gds/common/pipeline/files.py @@ -7,7 +7,7 @@ @author mstarch """ -import os +from pathlib import Path import fprime_gds.common.files.downlinker import fprime_gds.common.files.uplinker @@ -43,10 +43,11 @@ def setup_file_handling( ) file_decoder.register(self.__downlinker) distributor.register("FW_PACKET_HAND", self.__uplinker) - if not os.access(down_store, os.W_OK): + try: + Path(down_store).mkdir(parents=True, exist_ok=True) + except PermissionError: raise PermissionError( - f"{down_store} is not writable. Downlinker not be able to save files. " - "Fix permissions or change storage directory with --file-storage-directory." + f"{down_store} is not writable. Fix permissions or change storage directory with --file-storage-directory." ) @property diff --git a/src/fprime_gds/common/pipeline/standard.py b/src/fprime_gds/common/pipeline/standard.py index 242f12ae..cc745c45 100644 --- a/src/fprime_gds/common/pipeline/standard.py +++ b/src/fprime_gds/common/pipeline/standard.py @@ -72,6 +72,13 @@ def setup( # File storage configuration for uplink and downlink self.up_store = Path(file_store) / "fprime-uplink" self.down_store = Path(file_store) / "fprime-downlink" + try: + self.down_store.mkdir(parents=True, exist_ok=True) + self.up_store.mkdir(parents=True, exist_ok=True) + except PermissionError: + raise PermissionError( + f"{file_store} is not writable. Fix permissions or change storage directory with --file-storage-directory." + ) self.dictionary_path = Path(dictionary) # Loads the distributor and client socket self.distributor = fprime_gds.common.distributor.distributor.Distributor(config) From 3f47f6be002a6a33f36884bd54b5489e841ffcf0 Mon Sep 17 00:00:00 2001 From: thomas-bc Date: Wed, 17 Jan 2024 16:28:05 -0800 Subject: [PATCH 8/8] Add more permission checks --- src/fprime_gds/executables/cli.py | 7 ++++++- src/fprime_gds/flask/updown.py | 9 ++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/fprime_gds/executables/cli.py b/src/fprime_gds/executables/cli.py index a24eeb96..31928aa4 100644 --- a/src/fprime_gds/executables/cli.py +++ b/src/fprime_gds/executables/cli.py @@ -557,7 +557,12 @@ def get_arguments(self) -> Dict[Tuple[str, ...], Dict[str, Any]]: def handle_arguments(self, args, **kwargs): """Handle arguments as parsed""" - os.makedirs(args.files_storage_directory, exist_ok=True) + try: + Path(args.files_storage_directory).mkdir(parents=True, exist_ok=True) + except PermissionError: + raise PermissionError( + f"{args.files_storage_directory} is not writable. Fix permissions or change storage directory with --file-storage-directory." + ) return args diff --git a/src/fprime_gds/flask/updown.py b/src/fprime_gds/flask/updown.py index 596d9bb6..cf56e013 100644 --- a/src/fprime_gds/flask/updown.py +++ b/src/fprime_gds/flask/updown.py @@ -125,9 +125,12 @@ def save(self, file_storage: FileStorage): filename = Path(secure_filename(file_storage.filename)).name dest_dir = Path(self.dest_dir) - # not necessary - if not dest_dir.exists(): - dest_dir.mkdir(parents=True) + try: + dest_dir.mkdir(parents=True, exist_ok=True) + except PermissionError: + raise PermissionError( + f"{dest_dir} is not writable. Fix permissions or change storage directory with --file-storage-directory." + ) # resolve conflict may not be needed if (dest_dir / filename).exists():