fix(gitea): tolerate binary file payloads (#2380) (#2440)

* fix(gitea): tolerate binary file payloads

* fix(gitea): tolerate binary file payloads

* fix(gitea): preserve non-UTF-8 text via decode_if_bytes fallback

Addresses Qodo review on #2440: get_file_content returned '' on any
UnicodeDecodeError, so legitimate non-UTF-8 text (e.g. UTF-16) was
indistinguishable from an empty file and lost downstream. Decode via the
shared decode_if_bytes fallback chain instead, which preserves such text and
still tolerates binary payloads (filtered downstream by extension). Update
tests to assert text preservation and non-crashing binary handling.

---------

Co-authored-by: Wang ChengMeng <79295644+cmw-creator@users.noreply.github.com>
Co-authored-by: naorpeled <me@naor.dev>

Author: raywcm

pr_agent/git_providers/gitea_provider.py

@@ -6,6 +6,7 @@
 from giteapy.rest import ApiException
 
 from pr_agent.algo.file_filter import filter_ignored
+from pr_agent.algo.git_patch_processing import decode_if_bytes
 from pr_agent.algo.language_handler import is_valid_file
 from pr_agent.algo.types import EDIT_TYPE
 from pr_agent.algo.utils import (clip_tokens,
@@ -819,10 +820,10 @@ def get_pull_request_diff(self, owner: str, repo: str, pr_number: int) -> str:
 
             if hasattr(response, 'data'):
                 raw_data = response.data.read()
-                return raw_data.decode('utf-8')
+                return raw_data.decode('utf-8', errors='replace')
             elif isinstance(response, tuple):
                 raw_data = response[0].read()
-                return raw_data.decode('utf-8')
+                return raw_data.decode('utf-8', errors='replace')
             else:
                 error_msg = f"Unexpected response format received from API: {type(response)}"
                 self.logger.error(error_msg)
@@ -940,12 +941,17 @@ def get_file_content(self, owner: str, repo: str, commit_sha: str, filepath: str
                 auth_settings=['AuthorizationHeaderToken']
             )
 
+            # Decode via the shared fallback chain (utf-8, then iso-8859-1/latin-1/
+            # ascii/utf-16) so legitimate non-UTF-8 *text* (e.g. UTF-16) is preserved
+            # rather than dropped, while binary payloads no longer crash the provider.
+            # decode_if_bytes returns "" only if every encoding fails; binary files are
+            # filtered downstream by extension (should_skip_patch).
             if hasattr(response, 'data'):
                 raw_data = response.data.read()
-                return raw_data.decode('utf-8')
+                return decode_if_bytes(raw_data)
             elif isinstance(response, tuple):
                 raw_data = response[0].read()
-                return raw_data.decode('utf-8')
+                return decode_if_bytes(raw_data)
 
             return ""
 

tests/unittest/test_gitea_provider.py

@@ -104,6 +104,103 @@ def call_api_side_effect(path, method, **kwargs):
         assert args[0] == '/repos/owner/repo/pulls/123/commits'
         assert kwargs.get('auth_settings') == ['AuthorizationHeaderToken']
 
+
+    @patch('pr_agent.git_providers.gitea_provider.get_settings')
+    @patch('pr_agent.git_providers.gitea_provider.giteapy.ApiClient')
+    def test_gitea_provider_preserves_non_utf8_text_file_content(self, mock_api_client_cls, mock_get_settings):
+        # Regression for the Qodo review on #2440: non-UTF-8 *text* (e.g. UTF-16)
+        # must not be dropped to "" (which is indistinguishable from an empty file
+        # and loses real content downstream). It is decoded via the shared
+        # decode_if_bytes fallback chain instead of crashing or returning "".
+        settings = MagicMock()
+        settings.get.side_effect = lambda k, d=None: {
+            'GITEA.URL': 'https://gitea.example.com',
+            'GITEA.PERSONAL_ACCESS_TOKEN': 'test-token',
+            'GITEA.REPO_SETTING': None,
+            'GITEA.SKIP_SSL_VERIFICATION': False,
+            'GITEA.SSL_CA_CERT': None
+        }.get(k, d)
+        mock_get_settings.return_value = settings
+
+        mock_api_client = mock_api_client_cls.return_value
+        mock_api_client.configuration.api_key = {'Authorization': 'token test-token'}
+        mock_resp = MagicMock()
+        # UTF-16-LE encoded text — not valid UTF-8, but legitimate text content.
+        mock_resp.data = BytesIO("hello world".encode("utf-16"))
+        mock_api_client.call_api.return_value = mock_resp
+
+        from pr_agent.git_providers.gitea_provider import RepoApi
+
+        repo_api = RepoApi(mock_api_client)
+
+        content = repo_api.get_file_content('owner', 'repo', 'sha1', 'notes.txt')
+        assert content != '', "non-UTF-8 text must not be dropped to an empty string"
+        assert all(ch in content for ch in "hello world"), "the underlying text should survive the fallback decode"
+        args, kwargs = mock_api_client.call_api.call_args
+        assert args[0] == '/repos/owner/repo/raw/notes.txt'
+        assert kwargs.get('query_params') == [('ref', 'sha1')]
+        assert kwargs.get('auth_settings') == ['AuthorizationHeaderToken']
+
+    @patch('pr_agent.git_providers.gitea_provider.get_settings')
+    @patch('pr_agent.git_providers.gitea_provider.giteapy.ApiClient')
+    def test_gitea_provider_does_not_crash_on_binary_file_content(self, mock_api_client_cls, mock_get_settings):
+        # The original #2380 crash path: raw binary bytes must not raise
+        # UnicodeDecodeError. decode_if_bytes yields a best-effort string; binary
+        # files are filtered downstream by extension, so this only needs to not crash.
+        settings = MagicMock()
+        settings.get.side_effect = lambda k, d=None: {
+            'GITEA.URL': 'https://gitea.example.com',
+            'GITEA.PERSONAL_ACCESS_TOKEN': 'test-token',
+            'GITEA.REPO_SETTING': None,
+            'GITEA.SKIP_SSL_VERIFICATION': False,
+            'GITEA.SSL_CA_CERT': None
+        }.get(k, d)
+        mock_get_settings.return_value = settings
+
+        mock_api_client = mock_api_client_cls.return_value
+        mock_api_client.configuration.api_key = {'Authorization': 'token test-token'}
+        mock_resp = MagicMock()
+        mock_resp.data = BytesIO(b'\xff\xd8\xff\xe0\x00\x10JFIF\x00\x01')  # JPEG header bytes
+        mock_api_client.call_api.return_value = mock_resp
+
+        from pr_agent.git_providers.gitea_provider import RepoApi
+
+        repo_api = RepoApi(mock_api_client)
+
+        # Must not raise; result is a string (content filtered by extension downstream).
+        assert isinstance(repo_api.get_file_content('owner', 'repo', 'sha1', 'assets/image.webp'), str)
+
+
+    @patch('pr_agent.git_providers.gitea_provider.get_settings')
+    @patch('pr_agent.git_providers.gitea_provider.giteapy.ApiClient')
+    def test_gitea_provider_decodes_non_utf8_diff_with_replacement(self, mock_api_client_cls, mock_get_settings):
+        settings = MagicMock()
+        settings.get.side_effect = lambda k, d=None: {
+            'GITEA.URL': 'https://gitea.example.com',
+            'GITEA.PERSONAL_ACCESS_TOKEN': 'test-token',
+            'GITEA.REPO_SETTING': None,
+            'GITEA.SKIP_SSL_VERIFICATION': False,
+            'GITEA.SSL_CA_CERT': None
+        }.get(k, d)
+        mock_get_settings.return_value = settings
+
+        mock_api_client = mock_api_client_cls.return_value
+        mock_api_client.configuration.api_key = {'Authorization': 'token test-token'}
+        mock_resp = MagicMock()
+        mock_resp.data = BytesIO(b'diff --git a/image.png b/image.webp\n+' + bytes([0xff]) + b'binary')
+        mock_api_client.call_api.return_value = mock_resp
+
+        from pr_agent.git_providers.gitea_provider import RepoApi
+
+        repo_api = RepoApi(mock_api_client)
+
+        diff = repo_api.get_pull_request_diff('owner', 'repo', 123)
+
+        assert 'diff --git a/image.png b/image.webp' in diff
+        assert '�' in diff
+        args, kwargs = mock_api_client.call_api.call_args
+        assert args[0] == '/repos/owner/repo/pulls/123.diff'
+        assert kwargs.get('auth_settings') == ['AuthorizationHeaderToken']
     def test_get_repo_settings_returns_bytes(self):
         """Regression for #2347: get_repo_settings must return bytes so that
         utils.apply_repo_settings can os.write() it and later .decode() it. The