From 226f0c48bf6863ec452a9c304bde3d7ebed0f752 Mon Sep 17 00:00:00 2001 From: gftea Date: Fri, 6 Dec 2019 22:09:42 +0100 Subject: [PATCH 1/2] fix #5686, mktemp now fails given absolute and non-normalized paths. --- changelog/5686.improvement.rst | 1 + src/_pytest/tmpdir.py | 9 +++++++++ testing/test_tmpdir.py | 33 ++++++++++++++++++++++++++------- 3 files changed, 36 insertions(+), 7 deletions(-) create mode 100644 changelog/5686.improvement.rst diff --git a/changelog/5686.improvement.rst b/changelog/5686.improvement.rst new file mode 100644 index 000000000..e77997d87 --- /dev/null +++ b/changelog/5686.improvement.rst @@ -0,0 +1 @@ +``tmpdir_factory.mktemp`` now fails when given absolute and non-normalized paths. diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index bd8fb7d8a..b87e37167 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -45,8 +45,17 @@ class TempPathFactory: given_basetemp=config.option.basetemp, trace=config.trace.get("tmpdir") ) + def _ensure_relative_to_basetemp(self, basename: str): + basename = os.path.normpath(basename) + if (self.getbasetemp() / basename).resolve().parent != self.getbasetemp(): + raise ValueError( + "{} is not a normalized and relative path".format(basename) + ) + return basename + def mktemp(self, basename: str, numbered: bool = True) -> Path: """makes a temporary directory managed by the factory""" + basename = self._ensure_relative_to_basetemp(basename) if not numbered: p = self.getbasetemp().joinpath(basename) p.mkdir() diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py index eb1c1f300..b7cf8d2b5 100644 --- a/testing/test_tmpdir.py +++ b/testing/test_tmpdir.py @@ -74,19 +74,38 @@ class TestConfigTmpdir: assert not mytemp.join("hello").check() -def test_basetemp(testdir): +testdata = [ + ("mypath", True), + ("/mypath1", False), + ("./mypath1", True), + ("../mypath3", False), + ("../../mypath4", False), + ("mypath5/..", False), + ("mypath6/../mypath6", True), + ("mypath7/../mypath7/..", False), +] + + +@pytest.mark.parametrize("basename, is_ok", testdata) +def test_mktemp(testdir, basename, is_ok): mytemp = testdir.tmpdir.mkdir("mytemp") p = testdir.makepyfile( """ import pytest - def test_1(tmpdir_factory): - tmpdir_factory.mktemp('hello', numbered=False) - """ + def test_abs_path(tmpdir_factory): + tmpdir_factory.mktemp('{}', numbered=False) + """.format( + basename + ) ) + result = testdir.runpytest(p, "--basetemp=%s" % mytemp) - assert result.ret == 0 - print(mytemp) - assert mytemp.join("hello").check() + if is_ok: + assert result.ret == 0 + assert mytemp.join(basename).check() + else: + assert result.ret == 1 + result.stdout.fnmatch_lines("*ValueError*") def test_tmpdir_always_is_realpath(testdir): From fa645a7003c3338abe37eed51f6495b4cc376e53 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 9 Jan 2020 18:20:46 -0300 Subject: [PATCH 2/2] Improve docstrings for mktemp --- src/_pytest/tmpdir.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index b87e37167..85c5b8381 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -54,7 +54,20 @@ class TempPathFactory: return basename def mktemp(self, basename: str, numbered: bool = True) -> Path: - """makes a temporary directory managed by the factory""" + """Creates a new temporary directory managed by the factory. + + :param basename: + Directory base name, must be a relative path. + + :param numbered: + If True, ensure the directory is unique by adding a number + prefix greater than any existing one: ``basename="foo"`` and ``numbered=True`` + means that this function will create directories named ``"foo-0"``, + ``"foo-1"``, ``"foo-2"`` and so on. + + :return: + The path to the new directory. + """ basename = self._ensure_relative_to_basetemp(basename) if not numbered: p = self.getbasetemp().joinpath(basename) @@ -99,10 +112,9 @@ class TempdirFactory: _tmppath_factory = attr.ib(type=TempPathFactory) - def mktemp(self, basename: str, numbered: bool = True): - """Create a subdirectory of the base temporary directory and return it. - If ``numbered``, ensure the directory is unique by adding a number - prefix greater than any existing one. + def mktemp(self, basename: str, numbered: bool = True) -> py.path.local: + """ + Same as :meth:`TempPathFactory.mkdir`, but returns a ``py.path.local`` object. """ return py.path.local(self._tmppath_factory.mktemp(basename, numbered).resolve())