Skip to content

Commit 5da2ddb

Browse files
committed
Deduplicate remote upload_dir walk, cloud-tab layout, and SSRF checks
* Add remote/_upload_tree.walk_and_upload, the shared rglob-and-upload helper the s3, azure, dropbox, and sftp backends now use instead of re-implementing the same walker. * Introduce ui/tabs/base.RemoteBackendTab — the cloud/SFTP tabs no longer repeat the same QVBoxLayout + _init_group + _ops_group scaffold or carry per-tab _button helpers; they inherit from RemoteBackendTab and use BaseTab.make_button. * Split url_validator.validate_http_url into _require_host / _resolve_ips / _is_disallowed_ip helpers so the top-level function stops tripping the cyclomatic-complexity and boolean-expressions thresholds.
1 parent 186a3b7 commit 5da2ddb

11 files changed

Lines changed: 146 additions & 130 deletions

File tree

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
"""Shared directory-walker for cloud/SFTP ``*_upload_dir`` operations.
2+
3+
Every backend implements the same pattern: iterate ``Path.rglob('*')``,
4+
skip non-files, compute a POSIX-relative remote identifier, call
5+
``upload_file`` for each, and collect the successful remote keys. This
6+
module factors that walk out so each backend only supplies the two
7+
parts that actually differ — how to assemble the remote identifier
8+
and which per-file upload function to call.
9+
"""
10+
11+
from __future__ import annotations
12+
13+
from collections.abc import Callable
14+
from pathlib import Path
15+
16+
17+
def walk_and_upload(
18+
source: Path,
19+
make_remote: Callable[[str], str],
20+
upload_one: Callable[[Path, str], bool],
21+
) -> list[str]:
22+
"""Return the list of remote identifiers successfully uploaded from ``source``.
23+
24+
``make_remote`` is called with the POSIX relative path of each file
25+
(no leading slash) and must return the backend-specific remote key.
26+
``upload_one`` receives ``(local_path, remote_key)`` and returns True
27+
on success. Per-file failures are not raised — they are simply
28+
omitted from the returned list, matching the existing backend
29+
contract.
30+
"""
31+
uploaded: list[str] = []
32+
for entry in source.rglob("*"):
33+
if not entry.is_file():
34+
continue
35+
rel = entry.relative_to(source).as_posix()
36+
remote = make_remote(rel)
37+
if upload_one(entry, remote):
38+
uploaded.append(remote)
39+
return uploaded

automation_file/remote/azure_blob/upload_ops.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
from automation_file.exceptions import DirNotExistsException, FileNotExistsException
88
from automation_file.logging_config import file_automation_logger
9+
from automation_file.remote._upload_tree import walk_and_upload
910
from automation_file.remote.azure_blob.client import azure_blob_instance
1011

1112

@@ -45,15 +46,12 @@ def azure_blob_upload_dir(
4546
source = Path(dir_path)
4647
if not source.is_dir():
4748
raise DirNotExistsException(str(source))
48-
uploaded: list[str] = []
4949
prefix = name_prefix.rstrip("/")
50-
for entry in source.rglob("*"):
51-
if not entry.is_file():
52-
continue
53-
rel = entry.relative_to(source).as_posix()
54-
blob_name = f"{prefix}/{rel}" if prefix else rel
55-
if azure_blob_upload_file(str(entry), container, blob_name):
56-
uploaded.append(blob_name)
50+
uploaded = walk_and_upload(
51+
source,
52+
lambda rel: f"{prefix}/{rel}" if prefix else rel,
53+
lambda local, blob_name: azure_blob_upload_file(str(local), container, blob_name),
54+
)
5755
file_automation_logger.info(
5856
"azure_blob_upload_dir: %s -> %s/%s (%d files)",
5957
source,

automation_file/remote/dropbox_api/upload_ops.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
from automation_file.exceptions import DirNotExistsException, FileNotExistsException
88
from automation_file.logging_config import file_automation_logger
9+
from automation_file.remote._upload_tree import walk_and_upload
910
from automation_file.remote.dropbox_api.client import dropbox_instance
1011

1112

@@ -48,15 +49,12 @@ def dropbox_upload_dir(dir_path: str, remote_prefix: str = "/") -> list[str]:
4849
source = Path(dir_path)
4950
if not source.is_dir():
5051
raise DirNotExistsException(str(source))
51-
uploaded: list[str] = []
5252
prefix = remote_prefix.rstrip("/")
53-
for entry in source.rglob("*"):
54-
if not entry.is_file():
55-
continue
56-
rel = entry.relative_to(source).as_posix()
57-
remote = f"{prefix}/{rel}" if prefix else f"/{rel}"
58-
if dropbox_upload_file(str(entry), remote):
59-
uploaded.append(remote)
53+
uploaded = walk_and_upload(
54+
source,
55+
lambda rel: f"{prefix}/{rel}" if prefix else f"/{rel}",
56+
lambda local, remote: dropbox_upload_file(str(local), remote),
57+
)
6058
file_automation_logger.info(
6159
"dropbox_upload_dir: %s -> %s (%d files)",
6260
source,

automation_file/remote/s3/upload_ops.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
from automation_file.exceptions import DirNotExistsException, FileNotExistsException
88
from automation_file.logging_config import file_automation_logger
9+
from automation_file.remote._upload_tree import walk_and_upload
910
from automation_file.remote.s3.client import s3_instance
1011

1112

@@ -29,15 +30,12 @@ def s3_upload_dir(dir_path: str, bucket: str, key_prefix: str = "") -> list[str]
2930
source = Path(dir_path)
3031
if not source.is_dir():
3132
raise DirNotExistsException(str(source))
32-
uploaded: list[str] = []
3333
prefix = key_prefix.rstrip("/")
34-
for entry in source.rglob("*"):
35-
if not entry.is_file():
36-
continue
37-
rel = entry.relative_to(source).as_posix()
38-
key = f"{prefix}/{rel}" if prefix else rel
39-
if s3_upload_file(str(entry), bucket, key):
40-
uploaded.append(key)
34+
uploaded = walk_and_upload(
35+
source,
36+
lambda rel: f"{prefix}/{rel}" if prefix else rel,
37+
lambda local, key: s3_upload_file(str(local), bucket, key),
38+
)
4139
file_automation_logger.info(
4240
"s3_upload_dir: %s -> s3://%s/%s (%d files)",
4341
source,

automation_file/remote/sftp/upload_ops.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
from automation_file.exceptions import DirNotExistsException, FileNotExistsException
99
from automation_file.logging_config import file_automation_logger
10+
from automation_file.remote._upload_tree import walk_and_upload
1011
from automation_file.remote.sftp.client import sftp_instance
1112

1213

@@ -46,15 +47,12 @@ def sftp_upload_dir(dir_path: str, remote_prefix: str) -> list[str]:
4647
source = Path(dir_path)
4748
if not source.is_dir():
4849
raise DirNotExistsException(str(source))
49-
uploaded: list[str] = []
5050
prefix = remote_prefix.rstrip("/")
51-
for entry in source.rglob("*"):
52-
if not entry.is_file():
53-
continue
54-
rel = entry.relative_to(source).as_posix()
55-
remote = f"{prefix}/{rel}" if prefix else rel
56-
if sftp_upload_file(str(entry), remote):
57-
uploaded.append(remote)
51+
uploaded = walk_and_upload(
52+
source,
53+
lambda rel: f"{prefix}/{rel}" if prefix else rel,
54+
lambda local, remote: sftp_upload_file(str(local), remote),
55+
)
5856
file_automation_logger.info(
5957
"sftp_upload_dir: %s -> %s (%d files)",
6058
source,

automation_file/remote/url_validator.py

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,36 +16,45 @@
1616
_ALLOWED_SCHEMES = frozenset({"http", "https"})
1717

1818

19-
def validate_http_url(url: str) -> str:
20-
"""Return ``url`` if safe; raise :class:`UrlValidationException` otherwise."""
19+
def _require_host(url: str) -> str:
2120
if not isinstance(url, str) or not url:
2221
raise UrlValidationException("url must be a non-empty string")
23-
2422
parsed = urlparse(url)
2523
if parsed.scheme not in _ALLOWED_SCHEMES:
2624
raise UrlValidationException(f"disallowed scheme: {parsed.scheme!r}")
2725
host = parsed.hostname
2826
if not host:
2927
raise UrlValidationException("url must contain a host")
28+
return host
3029

30+
31+
def _resolve_ips(host: str) -> list[str]:
3132
try:
32-
addr_infos = socket.getaddrinfo(host, None)
33+
infos = socket.getaddrinfo(host, None)
3334
except socket.gaierror as error:
3435
raise UrlValidationException(f"cannot resolve host: {host}") from error
36+
return [str(info[4][0]) for info in infos]
37+
3538

36-
for info in addr_infos:
37-
ip_str = info[4][0]
39+
def _is_disallowed_ip(ip_obj: ipaddress.IPv4Address | ipaddress.IPv6Address) -> bool:
40+
return (
41+
ip_obj.is_private
42+
or ip_obj.is_loopback
43+
or ip_obj.is_link_local
44+
or ip_obj.is_reserved
45+
or ip_obj.is_multicast
46+
or ip_obj.is_unspecified
47+
)
48+
49+
50+
def validate_http_url(url: str) -> str:
51+
"""Return ``url`` if safe; raise :class:`UrlValidationException` otherwise."""
52+
host = _require_host(url)
53+
for ip_str in _resolve_ips(host):
3854
try:
3955
ip_obj = ipaddress.ip_address(ip_str)
4056
except ValueError as error:
4157
raise UrlValidationException(f"cannot parse resolved ip: {ip_str}") from error
42-
if (
43-
ip_obj.is_private
44-
or ip_obj.is_loopback
45-
or ip_obj.is_link_local
46-
or ip_obj.is_reserved
47-
or ip_obj.is_multicast
48-
or ip_obj.is_unspecified
49-
):
58+
if _is_disallowed_ip(ip_obj):
5059
raise UrlValidationException(f"disallowed ip: {ip_str}")
5160
return url

automation_file/ui/tabs/azure_tab.py

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
QGroupBox,
88
QLineEdit,
99
QPushButton,
10-
QVBoxLayout,
1110
)
1211

1312
from automation_file.remote.azure_blob.client import azure_blob_instance
@@ -18,19 +17,12 @@
1817
azure_blob_upload_dir,
1918
azure_blob_upload_file,
2019
)
21-
from automation_file.ui.tabs.base import BaseTab
20+
from automation_file.ui.tabs.base import RemoteBackendTab
2221

2322

24-
class AzureBlobTab(BaseTab):
23+
class AzureBlobTab(RemoteBackendTab):
2524
"""Form-driven Azure Blob operations."""
2625

27-
def __init__(self, log, pool) -> None:
28-
super().__init__(log, pool)
29-
root = QVBoxLayout(self)
30-
root.addWidget(self._init_group())
31-
root.addWidget(self._ops_group())
32-
root.addStretch()
33-
3426
def _init_group(self) -> QGroupBox:
3527
box = QGroupBox("Client")
3628
form = QFormLayout(box)
@@ -53,19 +45,13 @@ def _ops_group(self) -> QGroupBox:
5345
form.addRow("Local path", self._local)
5446
form.addRow("Container", self._container)
5547
form.addRow("Blob name / prefix", self._blob)
56-
form.addRow(self._button("Upload file", self._on_upload_file))
57-
form.addRow(self._button("Upload dir", self._on_upload_dir))
58-
form.addRow(self._button("Download to local", self._on_download))
59-
form.addRow(self._button("Delete blob", self._on_delete))
60-
form.addRow(self._button("List container", self._on_list))
48+
form.addRow(self.make_button("Upload file", self._on_upload_file))
49+
form.addRow(self.make_button("Upload dir", self._on_upload_dir))
50+
form.addRow(self.make_button("Download to local", self._on_download))
51+
form.addRow(self.make_button("Delete blob", self._on_delete))
52+
form.addRow(self.make_button("List container", self._on_list))
6153
return box
6254

63-
@staticmethod
64-
def _button(label: str, handler) -> QPushButton:
65-
button = QPushButton(label)
66-
button.clicked.connect(handler)
67-
return button
68-
6955
def _on_init(self) -> None:
7056
conn = self._conn_string.text().strip()
7157
account = self._account_url.text().strip()

automation_file/ui/tabs/base.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@
1212
from PySide6.QtCore import QThreadPool
1313
from PySide6.QtWidgets import (
1414
QFileDialog,
15+
QGroupBox,
1516
QHBoxLayout,
1617
QLineEdit,
1718
QPushButton,
19+
QVBoxLayout,
1820
QWidget,
1921
)
2022

@@ -30,6 +32,13 @@ def __init__(self, log: LogPanel, pool: QThreadPool) -> None:
3032
self._log = log
3133
self._pool = pool
3234

35+
@staticmethod
36+
def make_button(label: str, handler: Callable[[], Any]) -> QPushButton:
37+
"""Build a ``QPushButton`` wired to ``handler`` — the cloud tab idiom."""
38+
button = QPushButton(label)
39+
button.clicked.connect(handler)
40+
return button
41+
3342
def run_action(
3443
self,
3544
target: Callable[..., Any],
@@ -77,3 +86,26 @@ def pick_save_file(parent: QWidget) -> str | None:
7786
def pick_directory(parent: QWidget) -> str | None:
7887
path = QFileDialog.getExistingDirectory(parent, "Select directory")
7988
return path or None
89+
90+
91+
class RemoteBackendTab(BaseTab):
92+
"""Shared layout template for cloud/SFTP tabs.
93+
94+
Subclasses supply ``_init_group`` (credentials / session setup) and
95+
``_ops_group`` (file transfer actions). The base class stacks both
96+
inside a ``QVBoxLayout`` with a trailing stretch so the groups pin
97+
to the top of the tab.
98+
"""
99+
100+
def __init__(self, log: LogPanel, pool: QThreadPool) -> None:
101+
super().__init__(log, pool)
102+
root = QVBoxLayout(self)
103+
root.addWidget(self._init_group())
104+
root.addWidget(self._ops_group())
105+
root.addStretch()
106+
107+
def _init_group(self) -> QGroupBox:
108+
raise NotImplementedError
109+
110+
def _ops_group(self) -> QGroupBox:
111+
raise NotImplementedError

automation_file/ui/tabs/dropbox_tab.py

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
QGroupBox,
99
QLineEdit,
1010
QPushButton,
11-
QVBoxLayout,
1211
)
1312

1413
from automation_file.remote.dropbox_api.client import dropbox_instance
@@ -19,19 +18,12 @@
1918
dropbox_upload_dir,
2019
dropbox_upload_file,
2120
)
22-
from automation_file.ui.tabs.base import BaseTab
21+
from automation_file.ui.tabs.base import RemoteBackendTab
2322

2423

25-
class DropboxTab(BaseTab):
24+
class DropboxTab(RemoteBackendTab):
2625
"""Form-driven Dropbox operations."""
2726

28-
def __init__(self, log, pool) -> None:
29-
super().__init__(log, pool)
30-
root = QVBoxLayout(self)
31-
root.addWidget(self._init_group())
32-
root.addWidget(self._ops_group())
33-
root.addStretch()
34-
3527
def _init_group(self) -> QGroupBox:
3628
box = QGroupBox("Client")
3729
form = QFormLayout(box)
@@ -53,19 +45,13 @@ def _ops_group(self) -> QGroupBox:
5345
form.addRow("Local path", self._local)
5446
form.addRow("Remote path", self._remote)
5547
form.addRow(self._recursive)
56-
form.addRow(self._button("Upload file", self._on_upload_file))
57-
form.addRow(self._button("Upload dir", self._on_upload_dir))
58-
form.addRow(self._button("Download", self._on_download))
59-
form.addRow(self._button("Delete path", self._on_delete))
60-
form.addRow(self._button("List folder", self._on_list))
48+
form.addRow(self.make_button("Upload file", self._on_upload_file))
49+
form.addRow(self.make_button("Upload dir", self._on_upload_dir))
50+
form.addRow(self.make_button("Download", self._on_download))
51+
form.addRow(self.make_button("Delete path", self._on_delete))
52+
form.addRow(self.make_button("List folder", self._on_list))
6153
return box
6254

63-
@staticmethod
64-
def _button(label: str, handler) -> QPushButton:
65-
button = QPushButton(label)
66-
button.clicked.connect(handler)
67-
return button
68-
6955
def _on_init(self) -> None:
7056
token = self._token.text().strip()
7157
self.run_action(

0 commit comments

Comments
 (0)