From 0b3319244502c867517a697045517148557432b1 Mon Sep 17 00:00:00 2001
From: "John L. Villalovos" <john@sodarock.com>
Date: Thu, 4 Jan 2024 08:54:13 -0800
Subject: [PATCH] fix(cli): support binary files with `@` notation

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
---
 gitlab/cli.py                       | 14 ++++++++++----
 tests/functional/cli/test_cli_v4.py |  7 ++++++-
 tests/unit/test_cli.py              | 11 +++++------
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/gitlab/cli.py b/gitlab/cli.py
index 2687f1fd7..30c4b323b 100644
--- 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
index 684293f30..a5b989700 100644
--- 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
index 3c227a67d..e40440ed6 100644
--- 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