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

[raywcm]

Summary

Fixes #2380.

Gitea PRs that contain binary media changes can currently crash the provider when raw bytes are decoded as UTF-8. This affects both raw file fetches and .diff retrieval before ignored binary extensions can be filtered out cleanly.

This PR makes the Gitea provider tolerate those binary payloads:

• raw file content with invalid UTF-8 is skipped and treated as empty content
• PR diff payloads are decoded with replacement so the provider can still parse the textual diff header/context instead of crashing
• regression tests cover both paths

Testing

Ran focused local verification against the changed RepoApi methods with invalid UTF-8 bytes:

diff decode ok: 'diff --git a/old.png b/new.webp\n+�binary'
LOG_WARNING Skipping non-UTF-8 file content for asset.webp: 'utf-8' codec can't decode byte 0xff in position 0: invalid start byte
file content decode ok: ''

Also ran syntax checks:

python3 -m py_compile pr_agent/git_providers/gitea_provider.py tests/unittest/test_gitea_provider.py

The full pytest test module could not be run in this minimal environment without installing PR-Agent's full pinned dependency set; the added tests follow the existing RepoApi mock style in tests/unittest/test_gitea_provider.py.

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (1) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 [Bug] UnicodeDecodeError in gitea_provider.py when parsing binary files before extension filtering

1. get_file_content() decodes skipped binaries 📎 Requirement gap ☼ Reliability ⭐ New

Description

RepoApi.get_file_content() always retrieves raw file bytes and decodes them via decode_if_bytes()
before applying patch_extension_skip_types-based skipping, violating the requirement to skip
ignored/binary extensions prior to any raw content retrieval/decoding. Because decode_if_bytes will
convert arbitrary binary payloads to full-length strings (latin-1/iso-8859-1) and
GiteaProvider.__add_file_content eagerly fetches content for files that pass is_valid_file (which
may not exclude .webp by default), large binary assets can be unnecessarily loaded, decoded, and
stored, increasing overhead and risk of downstream mis-handling.

Code

pr_agent/git_providers/gitea_provider.py[R944-954]

+            # 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)

Evidence

Compliance requires applying patch_extension_skip_types filtering before any raw content
retrieval/decoding, but the current get_file_content() path explicitly decodes raw bytes via
decode_if_bytes(raw_data) and notes that binary files are only filtered “downstream,” demonstrating
that the skip check is not enforced early. Separately, __add_file_content unconditionally fetches
content for each path deemed is_valid_file, and since is_valid_file does not exclude extensions like
.webp by default, those files can reach get_file_content(); decode_if_bytes then decodes any bytes
using latin-1/iso-8859-1 without failing, producing a full-size string and ensuring the memory/CPU
cost is paid even for likely-binary payloads.

Skip ignored binary extensions before decoding file content in Gitea provider
pr_agent/git_providers/gitea_provider.py[924-956]
pr_agent/algo/git_patch_processing.py[54-58]
pr_agent/git_providers/gitea_provider.py[105-124]
pr_agent/git_providers/gitea_provider.py[924-964]
pr_agent/algo/language_handler.py[15-35]
pr_agent/settings/language_extensions.toml[1-59]
pr_agent/algo/git_patch_processing.py[39-51]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Ensure ignored/binary extensions (configured via `patch_extension_skip_types`, and/or other extension skip lists) are skipped *before* any raw-content API fetch and before `decode_if_bytes()` runs, so the Gitea provider does not eagerly retrieve, decode, and store large binary files (e.g., `.webp`) as full-length strings.

## Issue Context
The repo already has `should_skip_patch(filename)` driven by `get_settings().config.patch_extension_skip_types`, but the Gitea provider’s raw-content path currently calls the raw-file API and then decodes bytes via `decode_if_bytes()` without consulting this skip logic first (binary filtering is deferred “downstream”). In addition, `GiteaProvider.__add_file_content` fetches/stores content for every file that passes `is_valid_file`, and because `decode_if_bytes` falls back to latin-1/iso-8859-1 it will successfully decode arbitrary binary bytes into a non-empty, full-length Python string—so large binary assets not excluded by default extension lists (like `.webp`) can be unnecessarily loaded into memory.

## Fix Focus Areas
- pr_agent/git_providers/gitea_provider.py[105-124]
- pr_agent/git_providers/gitea_provider.py[924-964]
- pr_agent/algo/git_patch_processing.py[54-58]
- pr_agent/settings/language_extensions.toml[1-59]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. UTF-16 fallback unreachable 🐞 Bug ≡ Correctness ⭐ New

Description

RepoApi.get_file_content() now relies on decode_if_bytes() to preserve non‑UTF‑8 text, but the
fallback order tries iso-8859-1/latin-1 before utf-16, which succeeds for any byte sequence and
prevents UTF‑16 from being decoded correctly. This can leave UTF‑16 content full of embedded
NULs/BOM characters and degrade downstream line-based processing (e.g., patch context extension and
hunk matching).

Code

pr_agent/git_providers/gitea_provider.py[R944-954]

+            # 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)

Evidence

decode_if_bytes returns on the first successful decode; because iso-8859-1/latin-1 decoding
won’t raise on arbitrary bytes, UTF‑16 bytes won’t reach the utf-16 attempt. That mis-decoded
content then feeds line-based patch extension/hunk matching which expects normal text lines.

pr_agent/algo/git_patch_processing.py[39-51]
pr_agent/algo/git_patch_processing.py[61-87]
pr_agent/algo/git_patch_processing.py[190-214]
pr_agent/git_providers/gitea_provider.py[924-964]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`decode_if_bytes()` tries `iso-8859-1/latin-1` before `utf-16`. Since latin-1-style decodes don’t throw on arbitrary bytes, UTF-16 payloads will be mis-decoded and the UTF-16 fallback is effectively never used.

### Issue Context
This PR switched Gitea raw-file decoding to `decode_if_bytes()` and added a comment/test expecting UTF-16 preservation. Downstream patch-context expansion (`extend_patch`) is line-based and can be impacted by embedded NULs/BOM artifacts from mis-decoding.

### Fix Focus Areas
- pr_agent/algo/git_patch_processing.py[39-51]
- pr_agent/git_providers/gitea_provider.py[924-964]
- tests/unittest/test_gitea_provider.py[108-143]

### Implementation sketch
- Update `decode_if_bytes` to attempt UTF-16 earlier **when it’s likely** (e.g., BOM `b"\xff\xfe"/b"\xfe\xff"`, or a high ratio of `\x00` bytes), and only fall back to latin-1/iso-8859-1 after UTF-16 fails.
- Consider removing the duplicate `iso-8859-1` vs `latin-1` entry.
- Tighten the UTF-16 test to assert correct decoding once behavior is fixed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. UTF-16 test too weak 🐞 Bug ⚙ Maintainability ⭐ New

Description

test_gitea_provider_preserves_non_utf8_text_file_content only checks that each character from
"hello world" appears somewhere in the decoded output, which can pass even if UTF-16 bytes are
mis-decoded as latin-1 (e.g., "h\x00e\x00..."). This makes the regression test ineffective at
proving correct preservation of non‑UTF‑8 text.

Code

tests/unittest/test_gitea_provider.py[R136-139]

+        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

Evidence

The test’s all(ch in content ...) condition is satisfied by many incorrect decodes of UTF-16 data,
and the decoder currently prefers latin-1-style decodes before UTF-16.

tests/unittest/test_gitea_provider.py[108-143]
pr_agent/algo/git_patch_processing.py[39-51]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The UTF-16 regression test uses an assertion that can pass even when decoding is incorrect (latin-1 decode of UTF-16 still contains all ASCII letters).

### Issue Context
The PR claims non-UTF-8 text (UTF-16) should be preserved via `decode_if_bytes`, but current test coverage doesn’t validate that outcome.

### Fix Focus Areas
- tests/unittest/test_gitea_provider.py[108-143]
- pr_agent/algo/git_patch_processing.py[39-51]

### Suggested changes
- After fixing `decode_if_bytes` to actually decode UTF-16 when appropriate, change the assertion to something strict, e.g. `assert content == "hello world"` (or `.strip('\ufeff') == "hello world"` if you keep a BOM).
- Optionally assert that the returned string contains no NUL characters for this case: `assert "\x00" not in content`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Non-UTF8 text lost ✓ Resolved 🐞 Bug ≡ Correctness

Description

RepoApi.get_file_content now returns "" on UnicodeDecodeError, so valid non-UTF-8 text files (e.g.,
UTF-16) are indistinguishable from empty files. This causes downstream patch/context expansion to
miss real content for those files, reducing review/diff accuracy in that case.

Code

pr_agent/git_providers/gitea_provider.py[R949-953]

+        except UnicodeDecodeError as e:
+            self.logger.warning(
+                f"Skipping non-UTF-8 file content for {filepath}: {e}"
+            )
+            return ""

Evidence

get_file_content() still decodes strictly as UTF-8 and the new handler returns an empty string on
decode failure. Patch processing already has a fallback decoder for byte inputs and returns early
when content is empty, so returning "" here prevents using those fallbacks and makes non-UTF-8 text
effectively disappear.

pr_agent/git_providers/gitea_provider.py[917-956]
pr_agent/algo/git_patch_processing.py[16-28]
pr_agent/algo/git_patch_processing.py[39-51]
tests/unittest/test_gitea_provider.py[108-135]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RepoApi.get_file_content()` decodes raw bytes strictly as UTF-8 and returns an empty string on `UnicodeDecodeError`. This fixes binary crashes, but it also drops legitimate non-UTF-8 *text* files (e.g., UTF-16), making them appear empty downstream and preventing patch/context logic from using the actual content.
## Issue Context
The repo already has `decode_if_bytes()` logic with fallback encodings (including `utf-16`) used by patch processing, but it only helps if the provider passes bytes (or performs similar fallback decoding itself). Returning `""` on decode failure bypasses these fallbacks entirely.
## Fix Focus Areas
- pr_agent/git_providers/gitea_provider.py[917-956]
- pr_agent/algo/git_patch_processing.py[16-28]
- pr_agent/algo/git_patch_processing.py[39-51]
### Concrete fix options
1) On `UnicodeDecodeError`, return `raw_data` (bytes) instead of `""`, and adjust type hints / downstream callers accordingly (they already have `decode_if_bytes()` for bytes).
2) Alternatively, attempt a small, safe set of fallback decodes (mirroring `decode_if_bytes()`), and only return `""` if all fail.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{ⓘ  1 issues published inline · 3 in summary}

• Author self-review: I have reviewed the code review findings, and addressed the relevant ones.

Previous review results

Review updated until commit 93687f2

Results up to commit 3a147a8

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Context used

✅ Tickets: 🎫 [Bug] UnicodeDecodeError in gitea_provider.py when parsing binary files before extension filtering

1. Non-UTF8 text lost 🐞 Bug ≡ Correctness

Description

RepoApi.get_file_content now returns "" on UnicodeDecodeError, so valid non-UTF-8 text files (e.g.,
UTF-16) are indistinguishable from empty files. This causes downstream patch/context expansion to
miss real content for those files, reducing review/diff accuracy in that case.

Code

pr_agent/git_providers/gitea_provider.py[R949-953]

+        except UnicodeDecodeError as e:
+            self.logger.warning(
+                f"Skipping non-UTF-8 file content for {filepath}: {e}"
+            )
+            return ""

Evidence

get_file_content() still decodes strictly as UTF-8 and the new handler returns an empty string on
decode failure. Patch processing already has a fallback decoder for byte inputs and returns early
when content is empty, so returning "" here prevents using those fallbacks and makes non-UTF-8 text
effectively disappear.

pr_agent/git_providers/gitea_provider.py[917-956]
pr_agent/algo/git_patch_processing.py[16-28]
pr_agent/algo/git_patch_processing.py[39-51]
tests/unittest/test_gitea_provider.py[108-135]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RepoApi.get_file_content()` decodes raw bytes strictly as UTF-8 and returns an empty string on `UnicodeDecodeError`. This fixes binary crashes, but it also drops legitimate non-UTF-8 *text* files (e.g., UTF-16), making them appear empty downstream and preventing patch/context logic from using the actual content.

## Issue Context
The repo already has `decode_if_bytes()` logic with fallback encodings (including `utf-16`) used by patch processing, but it only helps if the provider passes bytes (or performs similar fallback decoding itself). Returning `""` on decode failure bypasses these fallbacks entirely.

## Fix Focus Areas
- pr_agent/git_providers/gitea_provider.py[917-956]
- pr_agent/algo/git_patch_processing.py[16-28]
- pr_agent/algo/git_patch_processing.py[39-51]

### Concrete fix options
1) On `UnicodeDecodeError`, return `raw_data` (bytes) instead of `""`, and adjust type hints / downstream callers accordingly (they already have `decode_if_bytes()` for bytes).
2) Alternatively, attempt a small, safe set of fallback decodes (mirroring `decode_if_bytes()`), and only return `""` if all fail.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Gitea provider: tolerate non-UTF-8 binary payloads in diffs and raw files
🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Walkthroughs

User Description

Summary

Fixes #2380.

Gitea PRs that contain binary media changes can currently crash the provider when raw bytes are decoded as UTF-8. This affects both raw file fetches and .diff retrieval before ignored binary extensions can be filtered out cleanly.

This PR makes the Gitea provider tolerate those binary payloads:

• raw file content with invalid UTF-8 is skipped and treated as empty content
• PR diff payloads are decoded with replacement so the provider can still parse the textual diff header/context instead of crashing
• regression tests cover both paths

Testing

Ran focused local verification against the changed RepoApi methods with invalid UTF-8 bytes:

diff decode ok: 'diff --git a/old.png b/new.webp\n+�binary'
LOG_WARNING Skipping non-UTF-8 file content for asset.webp: 'utf-8' codec can't decode byte 0xff in position 0: invalid start byte
file content decode ok: ''

Also ran syntax checks:

python3 -m py_compile pr_agent/git_providers/gitea_provider.py tests/unittest/test_gitea_provider.py

The full pytest test module could not be run in this minimal environment without installing PR-Agent's full pinned dependency set; the added tests follow the existing RepoApi mock style in tests/unittest/test_gitea_provider.py.

AI Description

• Decode Gitea PR .diff payloads with replacement to avoid UTF-8 decode crashes.
• Skip non-UTF-8 raw file contents and return empty content with a warning.
• Add regression tests covering both binary diff and binary raw file paths.

Diagram

graph TD
  A["PR-Agent (Gitea)" ] --> B["RepoApi" ] --> C{{"Gitea HTTP API"}} --> D[("Bytes payload")]
  D --> E["Diff decode" ] --> F["Text diff parse" ]
  D --> G["File decode" ] --> H["Empty content" ]

  subgraph Legend
    direction LR
    _svc["Component" ] ~~~ _ext{{"External"}} ~~~ _db[("Data")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Detect binary via Content-Type/headers before decoding

• ➕ Avoids introducing replacement characters into diffs
• ➕ Can fast-path known binary payloads without attempting decode
• ➖ Gitea responses may not reliably provide accurate content-type for all endpoints
• ➖ Still requires a fallback when headers are missing/incorrect

2. Decode diffs as latin-1 (lossless byte mapping)

• ➕ Never fails decoding; preserves 1:1 byte values
• ➕ Avoids U+FFFD replacement characters in output
• ➖ Downstream diff parsing may assume UTF-8 semantics; non-UTF-8 text could be misinterpreted
• ➖ Can silently produce misleading characters for truly UTF-8 content if misapplied globally

3. Pre-scan bytes and strip/skip binary hunks

• ➕ Keeps diff text cleaner by removing binary noise
• ➕ Could reduce parsing overhead for large binary sections
• ➖ More complex and error-prone; may miss edge cases in diff formats
• ➖ Harder to maintain than using standard decode error handling

Recommendation: The PR’s approach is the best tradeoff: it preserves provider stability with minimal, targeted changes. Using errors='replace' for .diff keeps the textual headers/context parseable, while skipping non-UTF-8 raw file content prevents crashes on binary assets. Header-based binary detection or binary-hunk filtering could be considered later if replacement characters materially affect downstream behavior, but they add complexity and rely on less reliable signals.

File Changes

Bug fix (1)

gitea_provider.py Harden diff/file decoding against binary (non-UTF-8) payloads +7/-2

Harden diff/file decoding against binary (non-UTF-8) payloads

• Decodes PR diff bytes using UTF-8 with replacement to avoid UnicodeDecodeError. Adds a UnicodeDecodeError handler for raw file content fetches that logs a warning and returns an empty string when content is not valid UTF-8.

pr_agent/git_providers/gitea_provider.py

---

Tests (1)

test_gitea_provider.py Add regression tests for binary diff and binary raw file handling +62/-0

Add regression tests for binary diff and binary raw file handling

• Adds unit tests that simulate invalid UTF-8 bytes returned from Gitea for raw file content and for PR .diff retrieval. Verifies the provider returns empty content for files and successfully returns a diff string containing the replacement character.

tests/unittest/test_gitea_provider.py

---

[naorpeled]

Finalized this based on Qodo's review — pushed two commits to the branch:

1. Addressed Qodo's finding (non-UTF-8 text lost). RepoApi.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. It now decodes via the shared decode_if_bytes() fallback chain (utf-8 → iso-8859-1/latin-1/ascii/utf-16), which preserves such text and still tolerates binary payloads without crashing — binary files remain filtered downstream by extension (should_skip_patch). Dropped the now-dead except UnicodeDecodeError branch. Updated the tests to assert text is preserved (not dropped to "") and that binary content doesn't crash.

2. Resolved the merge conflict with main. The conflict was an add/add in test_gitea_provider.py (main's #2347 get_repo_settings tests landed in the same spot) — kept both test sets.

Verified locally: test_gitea_provider.py + test_extend_patch.py = 21 passed, and the provider imports cleanly (no circular import from the new decode_if_bytes dependency).

Thanks @raywcm for the fix — the binary-crash handling and the diff-replacement decoding were spot on. 🙌

[qodo-free-for-open-source-projects]

1. get_file_content() decodes skipped binaries 📎 Requirement gap ☼ Reliability

RepoApi.get_file_content() always retrieves raw file bytes and decodes them via decode_if_bytes()
before applying patch_extension_skip_types-based skipping, violating the requirement to skip
ignored/binary extensions prior to any raw content retrieval/decoding. Because decode_if_bytes will
convert arbitrary binary payloads to full-length strings (latin-1/iso-8859-1) and
GiteaProvider.__add_file_content eagerly fetches content for files that pass is_valid_file (which
may not exclude .webp by default), large binary assets can be unnecessarily loaded, decoded, and
stored, increasing overhead and risk of downstream mis-handling.

Agent Prompt

## Issue description
Ensure ignored/binary extensions (configured via `patch_extension_skip_types`, and/or other extension skip lists) are skipped *before* any raw-content API fetch and before `decode_if_bytes()` runs, so the Gitea provider does not eagerly retrieve, decode, and store large binary files (e.g., `.webp`) as full-length strings.

## Issue Context
The repo already has `should_skip_patch(filename)` driven by `get_settings().config.patch_extension_skip_types`, but the Gitea provider’s raw-content path currently calls the raw-file API and then decodes bytes via `decode_if_bytes()` without consulting this skip logic first (binary filtering is deferred “downstream”). In addition, `GiteaProvider.__add_file_content` fetches/stores content for every file that passes `is_valid_file`, and because `decode_if_bytes` falls back to latin-1/iso-8859-1 it will successfully decode arbitrary binary bytes into a non-empty, full-length Python string—so large binary assets not excluded by default extension lists (like `.webp`) can be unnecessarily loaded into memory.

## Fix Focus Areas
- pr_agent/git_providers/gitea_provider.py[105-124]
- pr_agent/git_providers/gitea_provider.py[924-964]
- pr_agent/algo/git_patch_processing.py[54-58]
- pr_agent/settings/language_extensions.toml[1-59]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 93687f2