diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 23cfa7d7..8f25ca71 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -100,7 +100,7 @@ jobs: AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} integration: - needs: [linters,unit_tests] + #needs: [linters,unit_tests] runs-on: ${{ matrix.os }} strategy: matrix: @@ -138,7 +138,13 @@ jobs: - run: bash ci_helpers/helpers.sh enable_moto_server if: ${{ matrix.moto_server }} - - run: bash ci_helpers/helpers.sh create_ftp_server + - run: sudo apt-get install vsftpd ; bash ci_helpers/helpers.sh create_ftp_ftps_servers + env: + HOME_DIR: /home/user + USER: user + PASS: 123 + FTP_PORT: 21 + FTPS_PORT: 90 - name: Run integration tests run: python ci_helpers/run_integration_tests.py @@ -149,7 +155,7 @@ jobs: - run: bash ci_helpers/helpers.sh disable_moto_server if: ${{ matrix.moto_server }} - - run: bash ci_helpers/helpers.sh delete_ftp_server + - run: bash ci_helpers/helpers.sh delete_ftp_ftps_servers benchmarks: needs: [linters,unit_tests] diff --git a/ci_helpers/helpers.sh b/ci_helpers/helpers.sh old mode 100755 new mode 100644 index c24567a9..612162f3 --- a/ci_helpers/helpers.sh +++ b/ci_helpers/helpers.sh @@ -7,17 +7,61 @@ enable_moto_server(){ moto_server -p5000 2>/dev/null& } -create_ftp_server(){ - docker run -d -p 21:21 -p 21000-21010:21000-21010 -e USERS="user|123|/home/user/dir" -e ADDRESS=localhost --name my-ftp-server delfer/alpine-ftp-server +create_ftp_ftps_servers(){ + sudo mkdir $HOME_DIR + sudo useradd -p $(echo $PASS | openssl passwd -1 -stdin) -d $HOME_DIR $USER + sudo chown $USER:$USER $HOME_DIR + + server_setup=''' +listen=YES +listen_ipv6=NO +write_enable=YES +pasv_enable=YES +pasv_min_port=40000 +pasv_max_port=40009 +chroot_local_user=YES +allow_writeable_chroot=YES''' + + additional_ssl_setup=''' +ssl_enable=YES +allow_anon_ssl=NO +force_local_data_ssl=NO +force_local_logins_ssl=NO +require_ssl_reuse=NO +''' + cp /etc/vsftpd.conf /etc/vsftpd-ssl.conf + echo -e "$server_setup\nlisten_port=${FTP_PORT}" | sudo tee -a /etc/vsftpd.conf + echo -e "$server_setup\nlisten_port=${FTPS_PORT}\n$additional_ssl_setup" | sudo tee -a /etc/vsftpd-ssl.conf + # echo "listen=YES" | sudo tee -a /etc/vsftpd.conf + # echo "listen_ipv6=NO" | sudo tee -a /etc/vsftpd.conf + # echo "write_enable=YES" | sudo tee -a /etc/vsftpd.conf + # echo "pasv_enable=YES" | sudo tee -a /etc/vsftpd.conf + # echo "pasv_min_port=40000" | sudo tee -a /etc/vsftpd.conf + # echo "pasv_max_port=40009" | sudo tee -a /etc/vsftpd.conf + # echo "chroot_local_user=YES" | sudo tee -a /etc/vsftpd.conf + # echo "allow_writeable_chroot=YES" | sudo tee -a /etc/vsftpd.conf + + # sudo cp /etc/vsftpd.conf /etc/vsftpd-ssl.conf + + # echo "listen_port=${FTP_PORT}" | sudo tee -a /etc/vsftpd.conf + # echo "listen_port=${FTPS_PORT}" | sudo tee -a /etc/vsftpd-ssl.conf + + # echo "ssl_enable=YES" | sudo tee -a /etc/vsftpd-ssl.conf + # echo "allow_anon_ssl=NO" | sudo tee -a /etc/vsftpd-ssl.conf + # echo "force_local_data_ssl=NO" | sudo tee -a /etc/vsftpd-ssl.conf + # echo "force_local_logins_ssl=NO" | sudo tee -a /etc/vsftpd-ssl.conf + # echo "require_ssl_reuse=NO" | sudo tee -a /etc/vsftpd-ssl.conf + + sudo service vsftpd restart + sudo vsftpd /etc/vsftpd-ssl.conf & } disable_moto_server(){ lsof -i tcp:5000 | tail -n1 | cut -f2 -d" " | xargs kill -9 } -delete_ftp_server(){ - docker kill my-ftp-server - docker rm my-ftp-server +delete_ftp_ftps_servers(){ + sudo service vsftpd stop } "$@" diff --git a/integration-tests/test_ftp.py b/integration-tests/test_ftp.py index 79129dd8..000faae7 100644 --- a/integration-tests/test_ftp.py +++ b/integration-tests/test_ftp.py @@ -1,71 +1,84 @@ from __future__ import unicode_literals - +import pytest from smart_open import open -def test_nonbinary(): + +@pytest.fixture(params=[("ftp", 21), ("ftps", 90)]) +def server_info(request): + return request.param + +def test_nonbinary(server_info): + server_type = server_info[0] + port_num = server_info[1] file_contents = "Test Test \n new test \n another tests" appended_content1 = "Added \n to end" - with open("ftp://user:123@localhost:21/home/user/dir/file", "w") as f: + with open(f"{server_type}://user:123@localhost:{port_num}/file", "w") as f: f.write(file_contents) - with open("ftp://user:123@localhost:21/home/user/dir/file", "r") as f: + with open(f"{server_type}://user:123@localhost:{port_num}/file", "r") as f: read_contents = f.read() assert read_contents == file_contents - with open("ftp://user:123@localhost:21/home/user/dir/file", "a") as f: + with open(f"{server_type}://user:123@localhost:{port_num}/file", "a") as f: f.write(appended_content1) - with open("ftp://user:123@localhost:21/home/user/dir/file", "r") as f: + with open(f"{server_type}://user:123@localhost:{port_num}/file", "r") as f: read_contents = f.read() assert read_contents == file_contents + appended_content1 -def test_binary(): +def test_binary(server_info): + server_type = server_info[0] + port_num = server_info[1] file_contents = b"Test Test \n new test \n another tests" appended_content1 = b"Added \n to end" - with open("ftp://user:123@localhost:21/home/user/dir/file2", "wb") as f: + with open(f"{server_type}://user:123@localhost:{port_num}/file2", "wb") as f: f.write(file_contents) - with open("ftp://user:123@localhost:21/home/user/dir/file2", "rb") as f: + with open(f"{server_type}://user:123@localhost:{port_num}/file2", "rb") as f: read_contents = f.read() assert read_contents == file_contents - with open("ftp://user:123@localhost:21/home/user/dir/file2", "ab") as f: + with open(f"{server_type}://user:123@localhost:{port_num}/file2", "ab") as f: f.write(appended_content1) - with open("ftp://user:123@localhost:21/home/user/dir/file2", "rb") as f: + with open(f"{server_type}://user:123@localhost:{port_num}/file2", "rb") as f: read_contents = f.read() assert read_contents == file_contents + appended_content1 -def test_line_endings_non_binary(): +def test_line_endings_non_binary(server_info): + server_type = server_info[0] + port_num = server_info[1] B_CLRF = b'\r\n' CLRF = '\r\n' file_contents = f"Test Test {CLRF} new test {CLRF} another tests{CLRF}" - with open("ftp://user:123@localhost:21/home/user/dir/file3", "w") as f: + with open(f"{server_type}://user:123@localhost:{port_num}/file3", "w") as f: f.write(file_contents) - with open("ftp://user:123@localhost:21/home/user/dir/file3", "r") as f: + with open(f"{server_type}://user:123@localhost:{port_num}/file3", "r") as f: for line in f: assert not CLRF in line - with open("ftp://user:123@localhost:21/home/user/dir/file3", "rb") as f: + with open(f"{server_type}://user:123@localhost:{port_num}/file3", "rb") as f: for line in f: assert B_CLRF in line -def test_line_endings_binary(): +def test_line_endings_binary(server_info): + server_type = server_info[0] + port_num = server_info[1] B_CLRF = b'\r\n' CLRF = '\r\n' file_contents = f"Test Test {CLRF} new test {CLRF} another tests{CLRF}".encode('utf-8') - with open("ftp://user:123@localhost:21/home/user/dir/file4", "wb") as f: + with open(f"{server_type}://user:123@localhost:{port_num}/file4", "wb") as f: f.write(file_contents) - with open("ftp://user:123@localhost:21/home/user/dir/file4", "r") as f: + with open(f"{server_type}://user:123@localhost:{port_num}/file4", "r") as f: for line in f: assert not CLRF in line - with open("ftp://user:123@localhost:21/home/user/dir/file4", "rb") as f: + with open(f"{server_type}://user:123@localhost:{port_num}/file4", "rb") as f: for line in f: assert B_CLRF in line \ No newline at end of file diff --git a/smart_open/ftp.py b/smart_open/ftp.py index 36d86bbd..b60aa6ff 100644 --- a/smart_open/ftp.py +++ b/smart_open/ftp.py @@ -12,11 +12,11 @@ import logging import urllib.parse import smart_open.utils -from ftplib import FTP, error_reply +from ftplib import FTP, FTP_TLS, error_reply import types logger = logging.getLogger(__name__) -SCHEME = "ftp" +SCHEMES = ("ftp", "ftps") """Supported URL schemes.""" @@ -26,6 +26,9 @@ "ftp://username@host/path/file", "ftp://username:password@host/path/file", "ftp://username:password@host:port/path/file", + "ftps://username@host/path/file", + "ftps://username:password@host/path/file", + "ftps://username:password@host:port/path/file", ) @@ -35,7 +38,7 @@ def _unquote(text): def parse_uri(uri_as_string): split_uri = urllib.parse.urlsplit(uri_as_string) - assert split_uri.scheme == SCHEME, 'unexpected scheme: %r' % split_uri.scheme + assert split_uri.scheme in SCHEMES return dict( scheme=split_uri.scheme, uri_path=_unquote(split_uri.path), @@ -50,8 +53,10 @@ def open_uri(uri, mode, transport_params): smart_open.utils.check_kwargs(open, transport_params) parsed_uri = parse_uri(uri) uri_path = parsed_uri.pop("uri_path") - parsed_uri.pop("scheme") - return open(uri_path, mode, transport_params=transport_params, **parsed_uri) + scheme = parsed_uri.pop("scheme") + secure_conn = True if scheme == "ftps" else False + return open(uri_path, mode, secure_connection=secure_conn, + transport_params=transport_params, **parsed_uri) def convert_transport_params_to_args(transport_params): @@ -71,9 +76,12 @@ def convert_transport_params_to_args(transport_params): return kwargs -def _connect(hostname, username, port, password, transport_params): +def _connect(hostname, username, port, password, secure_connection, transport_params): kwargs = convert_transport_params_to_args(transport_params) - ftp = FTP(**kwargs) + if secure_connection: + ftp = FTP_TLS(**kwargs) + else: + ftp = FTP(**kwargs) try: ftp.connect(hostname, port) except Exception as e: @@ -84,6 +92,8 @@ def _connect(hostname, username, port, password, transport_params): except error_reply as e: logger.error("Unable to login to FTP server: try checking the username and password!") raise e + if secure_connection: + ftp.prot_p() return ftp @@ -95,6 +105,7 @@ def open( user=None, password=None, port=DEFAULT_PORT, + secure_connection=False, transport_params=None, ): if not host: @@ -103,7 +114,7 @@ def open( raise ValueError("you must specify the user") if not transport_params: transport_params = {} - conn = _connect(host, user, port, password, transport_params) + conn = _connect(host, user, port, password, secure_connection, transport_params) mode_to_ftp_cmds = { "r": ("RETR", "r"), "rb": ("RETR", "rb"),