From f4d05f43dbb9a11ef0c7819722adf57a74dd6a13 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 6 Jul 2026 20:44:50 +0000 Subject: [PATCH] fix: correct SFTP factory and handler initialization Fixed a bug where the SFTP session would hang after authentication because the `sftp_factory` was incorrectly returning an instance instead of a callable. Changes: - Updated `sftp_factory` in `dcfs/app/sftp/__init__.py` to return a lambda that accepts a channel. - Updated `DCFSSFTPHandler.__init__` in `dcfs/app/sftp/handler.py` to explicitly accept and pass the channel to `super().__init__`. - Added `tests/test_sftp_factory.py` to verify the factory and handler initialization logic. Co-authored-by: VulcanoSoftware <113239901+VulcanoSoftware@users.noreply.github.com> --- dcfs/app/sftp/__init__.py | 2 +- dcfs/app/sftp/handler.py | 4 ++-- tests/test_sftp_factory.py | 46 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 tests/test_sftp_factory.py diff --git a/dcfs/app/sftp/__init__.py b/dcfs/app/sftp/__init__.py index 92eb663..6f523c9 100644 --- a/dcfs/app/sftp/__init__.py +++ b/dcfs/app/sftp/__init__.py @@ -51,7 +51,7 @@ def server_factory(): return DCFSSFTPServer(clients, config) def sftp_factory(conn): - return DCFSSFTPHandler(clients, conn) + return lambda chan: DCFSSFTPHandler(clients, chan) return await asyncssh.create_server( server_factory, diff --git a/dcfs/app/sftp/handler.py b/dcfs/app/sftp/handler.py index 1b23e04..72ddf5a 100644 --- a/dcfs/app/sftp/handler.py +++ b/dcfs/app/sftp/handler.py @@ -18,9 +18,9 @@ class DCFSSFTPHandler(asyncssh.SFTPServer): - def __init__(self, clients: Clients, *args: Any, **kwargs: Any): + def __init__(self, clients: Clients, chan: Any): self.clients = clients - super().__init__(*args, **kwargs) + super().__init__(chan) def _get_ops(self, path_bytes: bytes) -> tuple[Optional[Ops], str]: path = path_bytes.decode("utf-8", errors="replace") diff --git a/tests/test_sftp_factory.py b/tests/test_sftp_factory.py new file mode 100644 index 0000000..47ba2a6 --- /dev/null +++ b/tests/test_sftp_factory.py @@ -0,0 +1,46 @@ +import pytest +from unittest.mock import MagicMock +from dcfs.app.sftp import create_sftp_server +from dcfs.app.sftp.handler import DCFSSFTPHandler + +@pytest.mark.asyncio +async def test_sftp_factory_logic(mocker): + # Mock dependencies + clients = MagicMock() + config = MagicMock() + config.dcfs.sftp.host = '0.0.0.0' + config.dcfs.sftp.port = 2022 + + # Mock asyncssh.create_server to capture the factory + mock_create_server = mocker.patch('asyncssh.create_server', mocker.AsyncMock()) + mocker.patch('asyncssh.read_private_key') + mocker.patch('os.path.exists', return_value=True) + + await create_sftp_server(clients, config) + + # Get the sftp_factory from the call to create_server + args, kwargs = mock_create_server.call_args + sftp_factory = kwargs.get('sftp_factory') + + assert sftp_factory is not None + + # Simulate asyncssh calling sftp_factory(conn) + mock_conn = MagicMock() + handler_factory = sftp_factory(mock_conn) + + # It should return a callable (the handler factory) + assert callable(handler_factory) + + # Simulate asyncssh calling handler_factory(chan) + mock_chan = MagicMock() + # We need to mock DCFSSFTPHandler because it calls super().__init__(chan) + # which might try to do things with the mock_chan. + # Actually, let's just see if it instantiates. + + # Mock SFTPServer.__init__ to avoid side effects + mocker.patch('asyncssh.SFTPServer.__init__', return_value=None) + + handler = handler_factory(mock_chan) + + assert isinstance(handler, DCFSSFTPHandler) + assert handler.clients == clients