OS errors because of py.path => pathlib transition in tmpdir.

[WloHu]

• pip list of the virtual environment you are using
  Python 3.6.5rc1
  pytest 3.9.1

• pytest and operating system versions
  ArchLinux, 4.12.13-1-ARCH
  Docker container

• Include a detailed description of the bug or suggestion

• Minimal example if possible

pathlib.PosixPath.joinpath which is used in _pytest.tmpdir.TempPathFactory.mktemp:44 works differently from previously used py.path.local.mkdir in such way that given code:

@pytest.fixture
def tmp_work_dir(tmpdir_factory):
    return tmpdir_factory.mktemp('/hello', numbered=False)

It tries to create temporary directory "/hello" instead "/tmp/pytest-of-user/pytest-0/hello" and fails when pathlib.PosixPath.mkdir() is run with different errors depending where I run it.
In Docker container as "root" FileNotFoundError :

================================================ ERRORS =================================================
________________________________________ ERROR at setup of test _________________________________________

tmpdir_factory = TempdirFactory(_tmppath_factory=TempPathFactory(_given_basetemp=None, _trace=<pluggy._tracing.TagTracerSub object at 0x7fa093ec9d30>, _basetemp=PosixPath('/tmp/pytest-of-root/pytest-95')))

    @pytest.fixture
    def tmp_work_dir(tmpdir_factory):
>       return tmpdir_factory.mktemp('/hello', numbered=False)

src/testcases/conftest.py:31: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
/usr/lib/python3.6/pathlib.py:1246: in mkdir
    self._accessor.mkdir(self, mode)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

pathobj = PosixPath('/hello')
args = (511,)

    @functools.wraps(strfunc)
    def wrapped(pathobj, *args):
>       return strfunc(str(pathobj), *args)
E       FileNotFoundError: [Errno 2] No such file or directory: '/hello'

/usr/lib/python3.6/pathlib.py:387: FileNotFoundError

In my OS w/o root privileges OSError:

================================================ ERRORS =================================================
________________________________________ ERROR at setup of test _________________________________________

tmpdir_factory = TempdirFactory(_tmppath_factory=TempPathFactory(_given_basetemp=None, _trace=<pluggy._tracing.TagTracerSub object at 0x7f384edf6a58>, _basetemp=PosixPath('/tmp/pytest-of-user/pytest-2')))

    @pytest.fixture
    def hi(tmpdir_factory):
        import pdb; pdb.set_trace()
>       return tmpdir_factory.mktemp('/hello')
E       OSError: could not create numbered dir with prefix /hello in /tmp/pytest-of-user/pytest-2 after 10 tries

sandbox/blahpytest/conftest.py:6: OSError
======================================= 1 error in 18.22 seconds ========================================

When run with sudo it works in my system however it doesn't help in Docker container.

Now in such case should it work in the new or old way?

[nicoddemus]

Thanks @WloHu for the report.

cc @RonnyPfannschmidt

[asottile]

tbh, the old code looks like a bug and the new code looks fixed. I would expect .mkdir('/hello') to try and write to the root of the filesystem -- you probably want .mkdir('hello')

[RonnyPfannschmidt]

we should fail flat if an absolute path is passed, the old code just accepted it and i have no idea off hand what id did (one of the reasons why i want to get away from py.path.local is that it fixes so many messes and bad mistakes in the name of convenience, that one misses bad implementations)

[nicoddemus]

Perhaps we should just close this as working as intended then? I agree it seems like the original intent was to write .mkdir("hello") and it worked mostly by accident.

@WloHu what do you think?

[WloHu]

@RonnyPfannschmidt answered my question that "we should fail flat if an absolute path is passed". So it means that old behaviour wasn't valid.

@nicoddemus It seems like it can be closed but is it ok for pytest to create temporary directory which "jumps out" from default "/tmp/pytest-of-user/" or user defined directory? I can still run this example test on my machine with sudo so "failing flat" isn't assured by pytest.

[nicoddemus]

It seems like it can be closed but is it ok for pytest to create temporary directory which "jumps out" from default "/tmp/pytest-of-user/" or user defined directory?

@WloHu I don't think we should support this out of the box because of the potential for misuse, so IMHO the current failing behavior is fine.

I personally don't think it is necessary, but @RonnyPfannschmidt would you like to open a separate issue to explicitly forbid receiving absolute paths in mkdir?

[RonnyPfannschmidt]

Reopening until ii create aa followup (on my mobile ATM)

[Zac-HD]

We should also reject relative paths where the destination is outside the tempdir - e.g. '../../bin/' could potentially cause serious problems. We should not pretend that we can make this safe against malicious test suites, but we can remove some footguns.

[nicoddemus]

@RonnyPfannschmidt do you think this is still relevant.

[RonnyPfannschmidt]

Yes

[nicoddemus]

Reopening until ii create aa followup (on my mobile ATM)

Sorry, I meant this ^

[RonnyPfannschmidt]

closing as the followup was made, thanks for the reminder/pointer, i missed it on mobile