Repository: pytest-dev/pytest
Pull request: Fixed an issue where `getpass.getuser()` contained illegal characters for file directories
closes #8317.  It is possible that `getpass.getuser()` can be illegal characters not appropriate for a directory name (platform dependent etc).  This PR addresses that by attempting it normally and backing off to use the default `-of-unknown` when it occurs.  This is consistent when `getuser()` is not Truthy.

Appreciate the feed back, thanks again.

Problem statement:
tmpdir creation fails when the username contains illegal characters for directory names
`tmpdir`, `tmpdir_factory` and `tmp_path_factory` rely on `getpass.getuser()` for determining the `basetemp` directory. I found that the user name returned by `getpass.getuser()` may return characters that are not allowed for directory names. This may lead to errors while creating the temporary directory.

The situation in which I reproduced this issue was while being logged in through an ssh connection into my Windows 10 x64 Enterprise version (1909) using an OpenSSH_for_Windows_7.7p1 server. In this configuration the command `python -c "import getpass; print(getpass.getuser())"` returns my domain username e.g. `contoso\john_doe` instead of `john_doe` as when logged in regularly using a local session.

When trying to create a temp directory in pytest through e.g. `tmpdir_factory.mktemp('foobar')` this fails with the following error message:
```
self = WindowsPath('C:/Users/john_doe/AppData/Local/Temp/pytest-of-contoso/john_doe')
mode = 511, parents = False, exist_ok = True

    def mkdir(self, mode=0o777, parents=False, exist_ok=False):
        """
        Create a new directory at this given path.
        """
        if self._closed:
            self._raise_closed()
        try:
>           self._accessor.mkdir(self, mode)
E           FileNotFoundError: [WinError 3] The system cannot find the path specified: 'C:\\Users\\john_doe\\AppData\\Local\\Temp\\pytest-of-contoso\\john_doe'

C:\Python38\lib\pathlib.py:1266: FileNotFoundError
```

I could also reproduce this without the complicated ssh/windows setup with pytest 6.2.2 using the following commands from a `cmd`:
```bat
echo def test_tmpdir(tmpdir):>test_tmp.py
echo   pass>>test_tmp.py
set LOGNAME=contoso\john_doe
py.test test_tmp.py
```

Thanks for having a look at this!

Hints from issue discussion:
Thanks for the report @pborsutzki!

Commit message: retry writing pytest-of dir when invalid chars are in directory name

Patch to review:
diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py
index 29c7e19d7b4..50c1caa6c1d 100644
--- a/src/_pytest/tmpdir.py
+++ b/src/_pytest/tmpdir.py
@@ -115,7 +115,14 @@ def getbasetemp(self) -> Path:
             # use a sub-directory in the temproot to speed-up
             # make_numbered_dir() call
             rootdir = temproot.joinpath(f"pytest-of-{user}")
-            rootdir.mkdir(exist_ok=True)
+            try:
+                rootdir.mkdir(exist_ok=True)
+            except OSError:
+                # Due to the weird and wonderful challenges of cross-platform compliant
+                # directory names, try to use whatever getuser() has provided and failing
+                # that, default back to unknown and try that.
+                rootdir = temproot.joinpath("pytest-of-unknown")
+                rootdir.mkdir(exist_ok=True)
             basetemp = make_numbered_dir_with_cleanup(
                 prefix="pytest-", root=rootdir, keep=3, lock_timeout=LOCK_TIMEOUT
             )
diff --git a/testing/test_tmpdir.py b/testing/test_tmpdir.py
index d123287aa38..3e3aeb4095f 100644
--- a/testing/test_tmpdir.py
+++ b/testing/test_tmpdir.py
@@ -11,6 +11,7 @@
 import pytest
 from _pytest import pathlib
 from _pytest.config import Config
+from _pytest.monkeypatch import MonkeyPatch
 from _pytest.pathlib import cleanup_numbered_dir
 from _pytest.pathlib import create_cleanup_lock
 from _pytest.pathlib import make_numbered_dir
@@ -445,3 +446,32 @@ def test(tmp_path):
     # running a second time and ensure we don't crash
     result = pytester.runpytest("--basetemp=tmp")
     assert result.ret == 0
+
+
+@pytest.mark.usefixtures("invalid_get_user")
+def test_tmpdir_factory_handles_invalid_dir_characters(
+    tmpdir_factory: TempdirFactory,
+) -> None:
+    assert "pytest-of-unknown" in str(tmpdir_factory.mktemp("test"))
+
+
+@pytest.mark.usefixtures("invalid_get_user")
+def test_tmp_path_factory_handles_invalid_dir_characters(
+    tmp_path_factory: TempPathFactory,
+) -> None:
+    assert "pytest-of-unknown" in str(tmp_path_factory.mktemp("test"))
+
+
+@pytest.mark.usefixtures("invalid_get_user")
+def test_tmpdir_handles_invalid_dir_characters(tmpdir) -> None:
+    assert "pytest-of-unknown" in str(tmpdir)
+
+
+@pytest.mark.usefixtures("invalid_get_user")
+def test_pytester_handles_invalid_dir_characters(pytester: Pytester) -> None:
+    assert "pytest-of-unknown" in str(pytester)
+
+
+@pytest.fixture
+def invalid_get_user(monkeypatch: MonkeyPatch) -> None:
+    monkeypatch.setattr("getpass.getuser", lambda: "os/agnostic")


Code review comments:
diff_hunk: @@ -445,3 +446,32 @@ def test(tmp_path):
     # running a second time and ensure we don't crash
     result = pytester.runpytest("--basetemp=tmp")
     assert result.ret == 0
+
+
+@pytest.mark.usefixtures("invalid_get_user")
+def test_tmpdir_factory_handles_invalid_dir_characters(
+    tmpdir_factory: TempdirFactory,
+) -> None:
+    assert "pytest-of-unknown" in str(tmpdir_factory.mktemp("test"))
+
+
+@pytest.mark.usefixtures("invalid_get_user")
+def test_tmp_path_factory_handles_invalid_dir_characters(
+    tmp_path_factory: TempPathFactory,
+) -> None:
+    assert "pytest-of-unknown" in str(tmp_path_factory.mktemp("test"))
+
+
+@pytest.mark.usefixtures("invalid_get_user")
+def test_tmpdir_handles_invalid_dir_characters(tmpdir) -> None:
+    assert "pytest-of-unknown" in str(tmpdir)
+
+
+@pytest.mark.usefixtures("invalid_get_user")
+def test_pytester_handles_invalid_dir_characters(pytester: Pytester) -> None:
+    assert "pytest-of-unknown" in str(pytester)
+
+
+@pytest.fixture
+def invalid_get_user(monkeypatch: MonkeyPatch) -> None:
+    monkeypatch.setattr("getpass.getuser", lambda: "os/agnostic")
path: testing/test_tmpdir.py
text: @user1:
Cool, you might want to throw a few other characters there too:

```suggestion
    monkeypatch.setattr("getpass.getuser", lambda: "os/:?*agnostic")
```



@author:
thanks, @user1 - any idea why the monkeypatch here is applied when running a single test from pycharm but not when running via `pytest testing/tmp_dir.py` or via `tox -e py38` etc?  Will update this after work later

@user1:
I think it might be because the base temp directory is cached? I suggest to manually reset the base temp directory inside the test, or better yet, just extract the functionality from `getbasetemp` into a function and unittest that.

diff_hunk: @@ -445,3 +446,32 @@ def test(tmp_path):
     # running a second time and ensure we don't crash
     result = pytester.runpytest("--basetemp=tmp")
     assert result.ret == 0
+
+
+@pytest.mark.usefixtures("invalid_get_user")
+def test_tmpdir_factory_handles_invalid_dir_characters(
+    tmpdir_factory: TempdirFactory,
+) -> None:
+    assert "pytest-of-unknown" in str(tmpdir_factory.mktemp("test"))
+
+
+@pytest.mark.usefixtures("invalid_get_user")
+def test_tmp_path_factory_handles_invalid_dir_characters(
+    tmp_path_factory: TempPathFactory,
+) -> None:
+    assert "pytest-of-unknown" in str(tmp_path_factory.mktemp("test"))
+
+
+@pytest.mark.usefixtures("invalid_get_user")
+def test_tmpdir_handles_invalid_dir_characters(tmpdir) -> None:
path: testing/test_tmpdir.py
text: @author:
I've left the type hint here, as i'm unsure if covering `tmpdir` is worth it, should i remove this as `pytester` is covered?

@user1:
Yeah only making sure the fix works for `tmp_path_factory` is enough! 👍