#4901 Patch files generated by Pagure mangle non-utf-8 content
Opened by churchyard. Modified

Consider this commit:

https://src.fedoraproject.org/rpms/python3.8/c/6363f29fc2cbe487120dbcd6dbc5864676b0c177

It adds a patch that (amongst other things) converts an iso-8859-1 line to utf-8 (lines 20 and 21 of the added 00350-sqlite-fix-deterministic-test.patch).

When I download the patch file (by appending .patch to the URL) and compare it with git format-patch --no-numbered output, there are several cosmetic differences (date format, subject line, git version, verbosity). But mainly, the encoding is mixed up:

$ wget https://src.fedoraproject.org/rpms/python3.8/c/6363f29fc2cbe487120dbcd6dbc5864676b0c177.patch
...
$ diff -u 6363f29fc2cbe487120dbcd6dbc5864676b0c177.patch 0001-Fix-sqlite3-deterministic-test.patch 
--- 6363f29fc2cbe487120dbcd6dbc5864676b0c177.patch  2020-06-21 19:50:48.651683157 +0200
+++ 0001-Fix-sqlite3-deterministic-test.patch   2020-06-21 19:43:42.100553515 +0200
...
@@ -35,7 +37,7 @@
 + # pysqlite2/test/userfunctions.py: tests for user-defined functions and
 + #                                  aggregates.
 + #
-+-# Copyright (C) 2005-2007 Gerhard H�ring <gh@ghaering.de>
++-# Copyright (C) 2005-2007 Gerhard H�ring <gh@ghaering.de>
 ++# Copyright (C) 2005-2007 Gerhard Häring <gh@ghaering.de>
...

The downloaded patch mangles the non-utf-8 parts of the patch. When the patch is git amed, the content of 00350-sqlite-fix-deterministic-test.patch is different than expected.

When examining the file content by git, I see that the Pagure-generated patch has:

+-# Copyright (C) 2005-2007 Gerhard H\xef\xbf\xbdring <gh@ghaering.de>
++# Copyright (C) 2005-2007 Gerhard H\xc3\xa4ring <gh@ghaering.de>

While the git-generated patch has:

+-# Copyright (C) 2005-2007 Gerhard H\xe4ring <gh@ghaering.de>
++# Copyright (C) 2005-2007 Gerhard H\xc3\xa4ring <gh@ghaering.de>

I was able to reproduce the Pagure generated content by:

>>> b'H\xe4ring'.decode('utf-8', errors='replace').encode('utf-8')
b'H\xef\xbf\xbdring'

Hence I assume this is exactly what Pagure is doing. It reads the patch as utf-8 (with errors=replace to avoid UnicodeDecodeErrors) and writes the file as text. Instead, it should operate on bytes to avoid data corruption. I will try to search for the relevant code to confirm my assumption.


It seem that Pagure uses https://www.pygit2.org/diff.html#pygit2.Patch directly as text:

https://pagure.io/pagure/blob/master/f/pagure/lib/git.py function commit_to_patch()

Patch.patch assumes all content to be encoded in UTF-8 and forcefully replaces any non-decodeable sequences. This can lead to corruption for content that either does not conform to any specific encoding altogether, or uses an encoding that is incompatible with, or ambiguous to UTF-8.

From https://github.com/libgit2/pygit2/pull/893

Hence, it might use the .data attribute instead. I see @pingou is participating in that PR.

This would bump the minimal version of pygit2 required, but should be doable.

Metadata Update from @pingou:
- Issue tagged with: bug

Metadata