Skip to content

Commit 8e95c66

Browse files
Julfriedmollyheamazon
authored andcommitted
fix: resolve infinite loop in _find_config on Windows systems (#4970)
* fix: resolve Windows path handling in _find_config * Replace Path.match("/") with Path.anchor comparison * Fix infinite loop in _studio.py path traversal * test: Add tests for the new root path exploration * Fix formatting style * Fixed line to long * Fix docstyle by running black manually * Fix testcase with \\ when running on non-windows machines * Fix formatting style * cleanup unused import
1 parent 80e5cd7 commit 8e95c66

File tree

2 files changed

+66
-2
lines changed

2 files changed

+66
-2
lines changed

src/sagemaker/_studio.py

+4-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,10 @@ def _find_config(working_dir=None):
6565
wd = Path(working_dir) if working_dir else Path.cwd()
6666

6767
path = None
68-
while path is None and not wd.match("/"):
68+
69+
# Get the root of the current working directory for both Windows and Unix-like systems
70+
root = Path(wd.anchor)
71+
while path is None and wd != root:
6972
candidate = wd / STUDIO_PROJECT_CONFIG
7073
if Path.exists(candidate):
7174
path = candidate

tests/unit/sagemaker/test_studio.py

+62-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
# language governing permissions and limitations under the License.
1313
# language governing permissions and limitations under the License.
1414
from __future__ import absolute_import
15-
15+
import os
16+
from pathlib import Path
1617
from sagemaker._studio import (
1718
_append_project_tags,
1819
_find_config,
@@ -21,6 +22,66 @@
2122
)
2223

2324

25+
def test_find_config_cross_platform(tmpdir):
26+
"""Test _find_config works correctly across different platforms."""
27+
# Create a completely separate directory for isolated tests
28+
import tempfile
29+
30+
with tempfile.TemporaryDirectory() as isolated_root:
31+
# Setup test directory structure for positive tests
32+
config = tmpdir.join(".sagemaker-code-config")
33+
config.write('{"sagemakerProjectId": "proj-1234"}')
34+
35+
# Test 1: Direct parent directory
36+
working_dir = tmpdir.mkdir("sub")
37+
found_path = _find_config(working_dir)
38+
assert found_path == config
39+
40+
# Test 2: Deeply nested directories
41+
nested_dir = tmpdir.mkdir("deep").mkdir("nested").mkdir("path")
42+
found_path = _find_config(nested_dir)
43+
assert found_path == config
44+
45+
# Test 3: Start from root directory
46+
import os
47+
48+
root_dir = os.path.abspath(os.sep)
49+
found_path = _find_config(root_dir)
50+
assert found_path is None
51+
52+
# Test 4: No config file in path - using truly isolated directory
53+
isolated_path = Path(isolated_root) / "nested" / "path"
54+
isolated_path.mkdir(parents=True)
55+
found_path = _find_config(isolated_path)
56+
assert found_path is None
57+
58+
59+
def test_find_config_path_separators(tmpdir):
60+
"""Test _find_config handles different path separator styles.
61+
62+
Tests:
63+
1. Forward slashes
64+
2. Backslashes
65+
3. Mixed separators
66+
"""
67+
# Setup
68+
config = tmpdir.join(".sagemaker-code-config")
69+
config.write('{"sagemakerProjectId": "proj-1234"}')
70+
base_path = str(tmpdir)
71+
72+
# Always include the OS native path and forward slashes (which are equivalent on all OS)
73+
paths = [os.path.join(base_path, "dir1", "dir2"), "/".join([base_path, "dir1", "dir2"])]
74+
75+
# Only on Windows add the backslashes and mixed separator test cases.
76+
if os.name == "nt":
77+
paths.extend(["\\".join([base_path, "dir1", "dir2"]), base_path + "/dir1\\dir2"])
78+
79+
for path in paths:
80+
os.makedirs(path, exist_ok=True)
81+
found_path = _find_config(path)
82+
assert found_path == config
83+
84+
2485
def test_find_config(tmpdir):
2586
path = tmpdir.join(".sagemaker-code-config")
2687
path.write('{"sagemakerProjectId": "proj-1234"}')

0 commit comments

Comments
 (0)