From 2d8c1313291a321b40ac64f330f2bb196f744743 Mon Sep 17 00:00:00 2001 From: Johnny Sequeira Date: Sat, 21 Sep 2024 15:47:12 -0600 Subject: [PATCH 1/6] Adding validator for files on converter to cloud project --- qfieldsync/core/cloud_converter.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/qfieldsync/core/cloud_converter.py b/qfieldsync/core/cloud_converter.py index bd6560e3..20c5bd84 100644 --- a/qfieldsync/core/cloud_converter.py +++ b/qfieldsync/core/cloud_converter.py @@ -20,6 +20,7 @@ """ from pathlib import Path +import re from libqfieldsync.layer import LayerSource from libqfieldsync.utils.file_utils import copy_attachments @@ -51,6 +52,15 @@ def __init__( self.export_dirname = Path(export_dirname) + # Define forbidden characters for Windows filenames + self.forbidden_chars_pattern = re.compile(r'[<>:"/\\|?*]') + + def is_valid_filename(self, filename: str) -> bool: + """ + Check if the filename contains any forbidden characters. + """ + return not self.forbidden_chars_pattern.search(filename) + def convert(self) -> None: # noqa: C901 """ Convert the project to a cloud project. @@ -128,6 +138,17 @@ def convert(self) -> None: # noqa: C901 self.project.removeMapLayer(layer) continue else: + # Validate filenames before copying + if not self.is_valid_filename(layer.name()): + self.warning.emit( + self.tr("Cloud Converter"), + self.tr( + "The layer '{}' has an invalid filename and was therefore removed from the cloud project." + ).format(layer.name()), + ) + self.project.removeMapLayer(layer) + continue + layer_source.copy(self.export_dirname, list()) layer.setCustomProperty( "QFieldSync/cloud_action", layer_source.default_cloud_action From aca7df6eac5b694f1f0c7f23dcb38f5d760cdcfc Mon Sep 17 00:00:00 2001 From: Johnny Sequeira Date: Mon, 23 Sep 2024 15:44:07 -0600 Subject: [PATCH 2/6] Improving the regex for validation and moving it to the file_utils --- qfieldsync/core/cloud_converter.py | 24 ++++++++++-------------- qfieldsync/utils/file_utils.py | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/qfieldsync/core/cloud_converter.py b/qfieldsync/core/cloud_converter.py index 20c5bd84..d9798f8a 100644 --- a/qfieldsync/core/cloud_converter.py +++ b/qfieldsync/core/cloud_converter.py @@ -20,10 +20,13 @@ """ from pathlib import Path -import re from libqfieldsync.layer import LayerSource -from libqfieldsync.utils.file_utils import copy_attachments +from libqfieldsync.utils.file_utils import ( + copy_attachments, + is_valid_filename, + is_valid_path, +) from libqfieldsync.utils.qgis import get_qgis_files_within_dir, make_temp_qgis_file from qgis.core import QgsMapLayer, QgsProject, QgsVirtualLayerDefinition from qgis.PyQt.QtCore import QCoreApplication, QObject, QUrl, pyqtSignal @@ -52,15 +55,6 @@ def __init__( self.export_dirname = Path(export_dirname) - # Define forbidden characters for Windows filenames - self.forbidden_chars_pattern = re.compile(r'[<>:"/\\|?*]') - - def is_valid_filename(self, filename: str) -> bool: - """ - Check if the filename contains any forbidden characters. - """ - return not self.forbidden_chars_pattern.search(filename) - def convert(self) -> None: # noqa: C901 """ Convert the project to a cloud project. @@ -138,12 +132,14 @@ def convert(self) -> None: # noqa: C901 self.project.removeMapLayer(layer) continue else: - # Validate filenames before copying - if not self.is_valid_filename(layer.name()): + # Validate filenames and paths before copying + if not is_valid_filename(layer.name()) or not is_valid_path( + layer.source() + ): self.warning.emit( self.tr("Cloud Converter"), self.tr( - "The layer '{}' has an invalid filename and was therefore removed from the cloud project." + "The layer '{}' has an invalid filename or path and was therefore removed from the cloud project." ).format(layer.name()), ) self.project.removeMapLayer(layer) diff --git a/qfieldsync/utils/file_utils.py b/qfieldsync/utils/file_utils.py index 87d45555..fc5f4e5a 100644 --- a/qfieldsync/utils/file_utils.py +++ b/qfieldsync/utils/file_utils.py @@ -21,6 +21,7 @@ from enum import Enum from pathlib import Path from typing import List, TypedDict, Union +import re PathLike = Union[Path, str] @@ -61,3 +62,27 @@ def path_to_dict(path: PathLike, dirs_only: bool = False) -> DirectoryTreeDict: node["content"].sort(key=lambda node: node["path"].name) return node + + +def is_valid_filename(filename: str) -> bool: + """ + Check if the filename is valid. + """ + pattern = re.compile( + r'\A(?!(?:COM[0-9]|CON|LPT[0-9]|NUL|PRN|AUX|com[0-9]|con|lpt[0-9]|nul|prn|aux)|\s|[\.]{2,})[^\\\/:*"?<>|]{1,254}(? bool: + """ + Check if the entire path is valid. + """ + try: + path_obj = Path(path) + for part in path_obj.parts: + if not is_valid_filename(part): + return False + return True + except Exception: + return False From 9a03915f80073f58923fecf7a956359800a199c2 Mon Sep 17 00:00:00 2001 From: Johnny Sequeira Date: Tue, 24 Sep 2024 09:59:21 -0600 Subject: [PATCH 3/6] Fixing the import functions --- qfieldsync/core/cloud_converter.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/qfieldsync/core/cloud_converter.py b/qfieldsync/core/cloud_converter.py index d9798f8a..3bee31fe 100644 --- a/qfieldsync/core/cloud_converter.py +++ b/qfieldsync/core/cloud_converter.py @@ -22,11 +22,7 @@ from pathlib import Path from libqfieldsync.layer import LayerSource -from libqfieldsync.utils.file_utils import ( - copy_attachments, - is_valid_filename, - is_valid_path, -) +from libqfieldsync.utils.file_utils import copy_attachments from libqfieldsync.utils.qgis import get_qgis_files_within_dir, make_temp_qgis_file from qgis.core import QgsMapLayer, QgsProject, QgsVirtualLayerDefinition from qgis.PyQt.QtCore import QCoreApplication, QObject, QUrl, pyqtSignal @@ -34,6 +30,7 @@ from qfieldsync.core.preferences import Preferences from qfieldsync.utils.qgis_utils import open_project +from qfieldsync.utils.file_utils import is_valid_filename, is_valid_path class CloudConverter(QObject): From cc7d46ae5d1256ffcc1bb809053421f20a33dfb1 Mon Sep 17 00:00:00 2001 From: Johnny <77129293+SeqLaz@users.noreply.github.com> Date: Fri, 4 Oct 2024 10:59:39 -0600 Subject: [PATCH 4/6] Update qfieldsync/core/cloud_converter.py Co-authored-by: Ivan Ivanov --- qfieldsync/core/cloud_converter.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/qfieldsync/core/cloud_converter.py b/qfieldsync/core/cloud_converter.py index 3bee31fe..cf1055f1 100644 --- a/qfieldsync/core/cloud_converter.py +++ b/qfieldsync/core/cloud_converter.py @@ -130,9 +130,7 @@ def convert(self) -> None: # noqa: C901 continue else: # Validate filenames and paths before copying - if not is_valid_filename(layer.name()) or not is_valid_path( - layer.source() - ): + if not is_valid_path(str(layer.filename)): self.warning.emit( self.tr("Cloud Converter"), self.tr( From df9b740f173a3e19e0168fb91810eae9b7a43302 Mon Sep 17 00:00:00 2001 From: Johnny Sequeira Date: Fri, 4 Oct 2024 12:21:06 -0600 Subject: [PATCH 5/6] Changing the function name and improvement in the file name check --- qfieldsync/core/cloud_converter.py | 4 ++-- qfieldsync/utils/file_utils.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/qfieldsync/core/cloud_converter.py b/qfieldsync/core/cloud_converter.py index cf1055f1..d30ef019 100644 --- a/qfieldsync/core/cloud_converter.py +++ b/qfieldsync/core/cloud_converter.py @@ -30,7 +30,7 @@ from qfieldsync.core.preferences import Preferences from qfieldsync.utils.qgis_utils import open_project -from qfieldsync.utils.file_utils import is_valid_filename, is_valid_path +from qfieldsync.utils.file_utils import is_valid_filepath class CloudConverter(QObject): @@ -130,7 +130,7 @@ def convert(self) -> None: # noqa: C901 continue else: # Validate filenames and paths before copying - if not is_valid_path(str(layer.filename)): + if not is_valid_filepath(str(layer.filename)): self.warning.emit( self.tr("Cloud Converter"), self.tr( diff --git a/qfieldsync/utils/file_utils.py b/qfieldsync/utils/file_utils.py index fc5f4e5a..4267617a 100644 --- a/qfieldsync/utils/file_utils.py +++ b/qfieldsync/utils/file_utils.py @@ -74,7 +74,7 @@ def is_valid_filename(filename: str) -> bool: return bool(pattern.match(filename)) -def is_valid_path(path: str) -> bool: +def is_valid_filepath(path: str) -> bool: """ Check if the entire path is valid. """ From 3ab98cae98ceadf8a01809b65f0b569c0904006a Mon Sep 17 00:00:00 2001 From: Johnny Sequeira Date: Fri, 4 Oct 2024 16:19:00 -0600 Subject: [PATCH 6/6] Improving regex for adding future validation, and moving the validator to the correct location block --- qfieldsync/core/cloud_converter.py | 22 +++++++++++----------- qfieldsync/utils/file_utils.py | 5 ++++- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/qfieldsync/core/cloud_converter.py b/qfieldsync/core/cloud_converter.py index d30ef019..5ec3d138 100644 --- a/qfieldsync/core/cloud_converter.py +++ b/qfieldsync/core/cloud_converter.py @@ -128,18 +128,18 @@ def convert(self) -> None: # noqa: C901 ) self.project.removeMapLayer(layer) continue - else: - # Validate filenames and paths before copying - if not is_valid_filepath(str(layer.filename)): - self.warning.emit( - self.tr("Cloud Converter"), - self.tr( - "The layer '{}' has an invalid filename or path and was therefore removed from the cloud project." - ).format(layer.name()), - ) - self.project.removeMapLayer(layer) - continue + # Validate filenames and paths before copying + if not is_valid_filepath(layer_source.filename): + self.warning.emit( + self.tr("Cloud Converter"), + self.tr( + "The layer '{}' has an invalid filename or path and was therefore removed from the cloud project." + ).format(layer.name()), + ) + self.project.removeMapLayer(layer) + continue + else: layer_source.copy(self.export_dirname, list()) layer.setCustomProperty( "QFieldSync/cloud_action", layer_source.default_cloud_action diff --git a/qfieldsync/utils/file_utils.py b/qfieldsync/utils/file_utils.py index 4267617a..03967fa9 100644 --- a/qfieldsync/utils/file_utils.py +++ b/qfieldsync/utils/file_utils.py @@ -69,7 +69,10 @@ def is_valid_filename(filename: str) -> bool: Check if the filename is valid. """ pattern = re.compile( - r'\A(?!(?:COM[0-9]|CON|LPT[0-9]|NUL|PRN|AUX|com[0-9]|con|lpt[0-9]|nul|prn|aux)|\s|[\.]{2,})[^\\\/:*"?<>|]{1,254}(?:"/\\|?*])' + r"(?!(?:COM[0-9]|CON|LPT[0-9]|NUL|PRN|AUX|com[0-9]|con|lpt[0-9]|nul|prn|aux)$)" + r'[^\\\/:*"?<>|]{1,254}' + r"(?