fix(cli): support binary files with @ notation

JohnVillalovos · nejch · commit 57749d46de1d · 2024-01-04T18:04:51.000+01:00
Support binary files being used in the CLI with arguments using the `@` notation. For example `--avatar @/path/to/avatar.png` Also explicitly catch the common OSError exception, which is the parent exception for things like: FileNotFoundError, PermissionError and more exceptions. Remove the bare exception handling. We would rather have the full traceback of any exceptions that we don't know about and add them later if needed. Closes: #2752
diff --git a/gitlab/cli.py b/gitlab/cli.py
@@ -1,6 +1,7 @@
 import argparse
 import functools
 import os
+import pathlib
 import re
 import sys
 import textwrap
@@ -298,12 +299,17 @@ def _parse_value(v: Any) -> Any:
         return v[1:]
     if isinstance(v, str) and v.startswith("@"):
         # If the user-provided value starts with @, we try to read the file
-        # path provided after @ as the real value. Exit on any error.
+        # path provided after @ as the real value.
+        filepath = pathlib.Path(v[1:]).expanduser().resolve()
         try:
-            with open(v[1:], encoding="utf-8") as f:
+            with open(filepath, encoding="utf-8") as f:
                 return f.read()
-        except Exception as e:
-            sys.stderr.write(f"{e}\n")
+        except UnicodeDecodeError:
+            with open(filepath, "rb") as f:
+                return f.read()
+        except OSError as exc:
+            exc_name = type(exc).__name__
+            sys.stderr.write(f"{exc_name}: {exc}\n")
             sys.exit(1)
 
     return v
diff --git a/tests/functional/cli/test_cli_v4.py b/tests/functional/cli/test_cli_v4.py
@@ -540,12 +540,15 @@ def test_update_application_settings(gitlab_cli):
     assert ret.success
 
 
-def test_create_project_with_values_from_file(gitlab_cli, tmpdir):
+def test_create_project_with_values_from_file(gitlab_cli, fixture_dir, tmpdir):
     name = "gitlab-project-from-file"
     description = "Multiline\n\nData\n"
     from_file = tmpdir.join(name)
     from_file.write(description)
     from_file_path = f"@{str(from_file)}"
+    avatar_file = fixture_dir / "avatar.png"
+    assert avatar_file.exists()
+    avatar_file_path = f"@{avatar_file}"
 
     cmd = [
         "-v",
@@ -555,6 +558,8 @@ def test_create_project_with_values_from_file(gitlab_cli, tmpdir):
         name,
         "--description",
         from_file_path,
+        "--avatar",
+        avatar_file_path,
     ]
     ret = gitlab_cli(cmd)
 
diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py
@@ -1,9 +1,9 @@
 import argparse
+import contextlib
 import io
 import os
 import sys
 import tempfile
-from contextlib import redirect_stderr  # noqa: H302
 from unittest import mock
 
 import pytest
@@ -62,7 +62,7 @@ def test_cls_to_gitlab_resource(class_name, expected_gitlab_resource):
 )
 def test_die(message, error, expected):
     fl = io.StringIO()
-    with redirect_stderr(fl):
+    with contextlib.redirect_stderr(fl):
         with pytest.raises(SystemExit) as test:
             cli.die(message, error)
     assert fl.getvalue() == expected
@@ -90,12 +90,11 @@ def test_parse_value():
     os.unlink(temp_path)
 
     fl = io.StringIO()
-    with redirect_stderr(fl):
+    with contextlib.redirect_stderr(fl):
         with pytest.raises(SystemExit) as exc:
             cli._parse_value("@/thisfileprobablydoesntexist")
-        assert (
-            fl.getvalue() == "[Errno 2] No such file or directory:"
-            " '/thisfileprobablydoesntexist'\n"
+        assert fl.getvalue().startswith(
+            "FileNotFoundError: [Errno 2] No such file or directory:"
         )
         assert exc.value.code == 1
 
