Skip to content

Commit aab0d04

Browse files
authored
MNT Raise ValueError if local_filename is not a plain filename in sklearn.datasets.fetch_file (scikit-learn#34815)
1 parent 3086a75 commit aab0d04

3 files changed

Lines changed: 83 additions & 2 deletions

File tree

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
- :func:`datasets.fetch_file` now raises a `ValueError` when `local_filename`
2+
is a path instead of a plain filename.
3+
By :user:`Dea María Léon <DeaMariaLeon>`.

sklearn/datasets/_base.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1605,8 +1605,9 @@ def fetch_file(
16051605
scikit-learn data home folder.
16061606
16071607
local_filename : str, default=None
1608-
Name of the file to save. If None, the filename is inferred from the
1609-
URL.
1608+
Name of the file to save. It must be a plain filename. A value holding
1609+
a directory separator or a relative reference such as `".."`, raises
1610+
`ValueError`. If None, the filename is inferred from the URL.
16101611
16111612
sha256 : str, default=None
16121613
SHA256 checksum of the file. If None, no checksum is verified.
@@ -1626,6 +1627,15 @@ def fetch_file(
16261627

16271628
if local_filename is None:
16281629
local_filename = filename_from_url
1630+
elif Path(local_filename).name != local_filename or local_filename in (
1631+
"",
1632+
"..",
1633+
):
1634+
raise ValueError(
1635+
"`local_filename` should be a filename, not a path, got"
1636+
f" {local_filename!r}. Use the `folder` argument to control the"
1637+
" output folder."
1638+
)
16291639

16301640
if folder is None:
16311641
folder = Path(get_data_home()) / folder_from_url

sklearn/datasets/tests/test_base.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,3 +657,71 @@ def test_fetch_file_with_sha256(monkeypatch, tmpdir):
657657
folder=client_side,
658658
sha256=non_matching_sha256,
659659
)
660+
661+
662+
@pytest.mark.parametrize(
663+
"local_filename",
664+
[
665+
"../escaped.jsonl",
666+
"subfolder/data.jsonl",
667+
"/absolute/data.jsonl",
668+
"subfolder/",
669+
"/absolute/dir/",
670+
"../",
671+
"./",
672+
"..",
673+
".",
674+
"",
675+
],
676+
)
677+
def test_fetch_file_rejects_path_as_local_filename(monkeypatch, tmpdir, local_filename):
678+
# Non-regression test: a path-like `local_filename` must be rejected instead
679+
# of silently writing outside of `folder`.
680+
client_side = Path(tmpdir.mkdir("client_side"))
681+
682+
urlretrieve_mock = Mock()
683+
monkeypatch.setattr("sklearn.datasets._base.urlretrieve", urlretrieve_mock)
684+
685+
expected_error_msg = re.escape(
686+
"`local_filename` should be a filename, not a path, got"
687+
f" {local_filename!r}. Use the `folder` argument to control the"
688+
" output folder."
689+
)
690+
with pytest.raises(ValueError, match=expected_error_msg):
691+
fetch_file(
692+
"https://example.com/data.jsonl",
693+
folder=client_side,
694+
local_filename=local_filename,
695+
)
696+
697+
# The check happens before any download and nothing is written to disk.
698+
assert urlretrieve_mock.call_count == 0
699+
assert list(Path(tmpdir).rglob("*")) == [client_side]
700+
701+
702+
@pytest.mark.parametrize(
703+
"local_filename",
704+
[
705+
"renamed.jsonl",
706+
".hidden.jsonl",
707+
"data..jsonl",
708+
],
709+
)
710+
def test_fetch_file_accepts_plain_local_filename(monkeypatch, tmpdir, local_filename):
711+
server_side = tmpdir.mkdir("server_side")
712+
server_data = '{"a": 1, "b": 2}\n'
713+
Path(server_side / "data.jsonl").write_text(server_data, encoding="utf-8")
714+
715+
client_side = Path(tmpdir.mkdir("client_side"))
716+
717+
monkeypatch.setattr(
718+
"sklearn.datasets._base.urlretrieve", _mock_urlretrieve(server_side)
719+
)
720+
721+
fetched_file_path = fetch_file(
722+
"https://example.com/data.jsonl",
723+
folder=client_side,
724+
local_filename=local_filename,
725+
)
726+
assert fetched_file_path == client_side / local_filename
727+
assert fetched_file_path.read_text(encoding="utf-8") == server_data

0 commit comments

Comments
 (0)