From accbe2bdc67412315c2e70990e9d61f8a5399bb2 Mon Sep 17 00:00:00 2001 From: Eric Zhu Date: Tue, 2 Jan 2024 20:05:33 -0800 Subject: [PATCH 1/4] fix #1069 --- autogen/code_utils.py | 27 +++++++++++++++++++++++++-- test/test_code.py | 39 +++++++++++++++++++++++++++++++-------- 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/autogen/code_utils.py b/autogen/code_utils.py index dbb8637162d..076dec5ba43 100644 --- a/autogen/code_utils.py +++ b/autogen/code_utils.py @@ -224,6 +224,29 @@ def _cmd(lang): raise NotImplementedError(f"{lang} not recognized in code execution") +def _sanitize_filename_for_docker_tag(filename: str) -> str: + """Convert a filename to a valid docker tag. + See https://docs.docker.com/engine/reference/commandline/tag/ for valid tag + format. + + Args: + filename (str): The filename to be converted. + + Returns: + str: The sanitized Docker tag. + """ + # Replace any character not allowed with an underscore + allowed_chars = set("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.-") + sanitized = "".join(char if char in allowed_chars else "_" for char in filename) + + # Ensure it does not start with a period or a dash + if sanitized.startswith(".") or sanitized.startswith("-"): + sanitized = sanitized.replace(".", "_", 1).replace("-", "_", 1) + + # Truncate if longer than 128 characters + return sanitized[:128] + + def execute_code( code: Optional[str] = None, timeout: Optional[int] = None, @@ -374,7 +397,7 @@ def execute_code( cmd = [ "sh", "-c", - f"{_cmd(lang)} {filename}; exit_code=$?; echo -n {exit_code_str}; echo -n $exit_code; echo {exit_code_str}", + f'{_cmd(lang)} "{filename}"; exit_code=$?; echo -n {exit_code_str}; echo -n $exit_code; echo {exit_code_str}', ] # create a docker container container = client.containers.run( @@ -398,7 +421,7 @@ def execute_code( # get the container logs logs = container.logs().decode("utf-8").rstrip() # commit the image - tag = filename.replace("/", "") + tag = _sanitize_filename_for_docker_tag(filename) container.commit(repository="python", tag=tag) # remove the container container.remove() diff --git a/test/test_code.py b/test/test_code.py index ec8ada2c906..2f526584fc9 100644 --- a/test/test_code.py +++ b/test/test_code.py @@ -21,6 +21,13 @@ OAI_CONFIG_LIST = "OAI_CONFIG_LIST" here = os.path.abspath(os.path.dirname(__file__)) +try: + import docker + + docker_package_installed = True +except ImportError as exc: + docker_package_installed = False + # def test_find_code(): # try: @@ -293,10 +300,10 @@ def scrape(url): assert len(codeblocks) == 1 and codeblocks[0] == ("", "source setup.sh") -@pytest.mark.skipif( - sys.platform in ["darwin"], - reason="do not run on MacOS", -) +# @pytest.mark.skipif( +# sys.platform in ["darwin"], +# reason="do not run on MacOS", +# ) def test_execute_code(use_docker=None): try: import docker @@ -338,15 +345,31 @@ def test_execute_code(use_docker=None): assert isinstance(image, str) or docker is None or os.path.exists("/.dockerenv") or use_docker is False +@pytest.mark.skipif(docker_package_installed is False, reason="docker package not installed") +def test_execute_code_with_custom_filename_on_docker(): + exit_code, msg, image = execute_code("print('hello world')", filename="tmp/codetest.py", use_docker=True) + assert exit_code == 0 and msg == "hello world\n", msg + assert image == "python:tmp_codetest.py" + + +@pytest.mark.skipif(docker_package_installed is False, reason="docker package not installed") +def test_execute_code_with_misformed_filename_on_docker(): + exit_code, msg, image = execute_code( + "print('hello world')", filename="tmp/codetest.py (some extra information)", use_docker=True + ) + assert exit_code == 0 and msg == "hello world\n", msg + assert image == "python:tmp_codetest.py__some_extra_information_" + + def test_execute_code_raises_when_code_and_filename_are_both_none(): with pytest.raises(AssertionError): execute_code(code=None, filename=None) -@pytest.mark.skipif( - sys.platform in ["darwin"], - reason="do not run on MacOS", -) +# @pytest.mark.skipif( +# sys.platform in ["darwin"], +# reason="do not run on MacOS", +# ) def test_execute_code_nodocker(): test_execute_code(use_docker=False) From b3b4910e8136bceda57f961239d0e269bb13c01d Mon Sep 17 00:00:00 2001 From: Eric Zhu Date: Tue, 2 Jan 2024 20:33:03 -0800 Subject: [PATCH 2/4] uncomment skips --- test/test_code.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/test/test_code.py b/test/test_code.py index 2f526584fc9..6f3f3524b28 100644 --- a/test/test_code.py +++ b/test/test_code.py @@ -1,6 +1,7 @@ import os import sys import unittest +import pkg_resources import pytest @@ -21,12 +22,18 @@ OAI_CONFIG_LIST = "OAI_CONFIG_LIST" here = os.path.abspath(os.path.dirname(__file__)) -try: - import docker - docker_package_installed = True -except ImportError as exc: - docker_package_installed = False +def is_package_installed(package_name): + """Check if a package is installed. This is a preferred way to check if a + package is installed or not, avoiding name conflict with local modules.""" + try: + pkg_resources.get_distribution(package_name) + return True + except pkg_resources.DistributionNotFound: + return False + + +docker_package_installed = is_package_installed("docker") # def test_find_code(): @@ -300,10 +307,6 @@ def scrape(url): assert len(codeblocks) == 1 and codeblocks[0] == ("", "source setup.sh") -# @pytest.mark.skipif( -# sys.platform in ["darwin"], -# reason="do not run on MacOS", -# ) def test_execute_code(use_docker=None): try: import docker @@ -366,10 +369,6 @@ def test_execute_code_raises_when_code_and_filename_are_both_none(): execute_code(code=None, filename=None) -# @pytest.mark.skipif( -# sys.platform in ["darwin"], -# reason="do not run on MacOS", -# ) def test_execute_code_nodocker(): test_execute_code(use_docker=False) From 7afa39016c41455af08c0be0adcd7325d8d987ee Mon Sep 17 00:00:00 2001 From: Eric Zhu Date: Tue, 2 Jan 2024 20:55:00 -0800 Subject: [PATCH 3/4] use importlib to check package --- test/test_code.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/test_code.py b/test/test_code.py index 6f3f3524b28..e7306125de2 100644 --- a/test/test_code.py +++ b/test/test_code.py @@ -1,7 +1,7 @@ +import importlib.metadata import os import sys import unittest -import pkg_resources import pytest @@ -25,11 +25,13 @@ def is_package_installed(package_name): """Check if a package is installed. This is a preferred way to check if a - package is installed or not, avoiding name conflict with local modules.""" + package is installed or not than doing a try-catch around an import + because it avoids name conflict with local modules and + code execution in the imported module.""" try: - pkg_resources.get_distribution(package_name) + importlib.metadata.version(package_name) return True - except pkg_resources.DistributionNotFound: + except importlib.metadata.PackageNotFoundError: return False From 7a522f5bcb7dbb05a28294506db30b63283cd293 Mon Sep 17 00:00:00 2001 From: Eric Zhu Date: Fri, 12 Jan 2024 22:12:49 -0800 Subject: [PATCH 4/4] better code --- autogen/code_utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/autogen/code_utils.py b/autogen/code_utils.py index 24a7adb37c2..f468c3d1bfd 100644 --- a/autogen/code_utils.py +++ b/autogen/code_utils.py @@ -2,6 +2,7 @@ import os import pathlib import re +import string import subprocess import sys import time @@ -236,12 +237,12 @@ def _sanitize_filename_for_docker_tag(filename: str) -> str: str: The sanitized Docker tag. """ # Replace any character not allowed with an underscore - allowed_chars = set("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.-") + allowed_chars = set(string.ascii_letters + string.digits + "_.-") sanitized = "".join(char if char in allowed_chars else "_" for char in filename) # Ensure it does not start with a period or a dash if sanitized.startswith(".") or sanitized.startswith("-"): - sanitized = sanitized.replace(".", "_", 1).replace("-", "_", 1) + sanitized = "_" + sanitized[1:] # Truncate if longer than 128 characters return sanitized[:128]