From 9452419ba06282a6ba271d88f4f5e0e83e5b4690 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 11:49:21 +0000 Subject: [PATCH 1/6] Make the binary file really binary --- diff --git a/tests/__init__.py b/tests/__init__.py index 3359767..a5545a4 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -489,20 +489,11 @@ def add_binary_git_repo(folder, filename): newfolder = tempfile.mkdtemp(prefix='pagure-tests') repo = pygit2.clone_repository(folder, newfolder) - content = """<89>PNG^M -^Z -^@^@^@^MIHDR^@^@^@K^@^@^@K^H^F^@^@^@8Nzê^@^@^@^FbKGD^@ÿ^@ÿ^@ÿ ½§<93>^@^@^@ pHYs^@^@^M×^@^@^M×^AB(<9b>x^@^@^@^GtIM -E^GÞ -^N^U^F^[<88>]·<9c>^@^@ <8a>IDATxÚí<9c>ÛO^Tg^_Ç?3³»ì^B -<8b>®ËË<8b>X^NÕõ^EQÚ^Z­^Qc<82>^Pk5Úô¦iMÄ^[{×^K<9b>&^^Xÿ^A<8d>WM^S^SmÒ<8b>j¯Zê<8d> 6^QO^Dª¶´/Ö^M^T5^^*¼¬<9c>^Oî<8 -1><99>÷<82>Y<8b>03;3»<83>hù&d óìÃÌw~§çûüf`^Q<8b>XÄ"^V±<88>^?:<84>^Er^N^R ª¿^K3ÎK<99>ñ3^EÈêïÿ8²ò<81> <90>¥C^T^Z<84> -É@^Tè^E<86>_g²²<80>^\<95>$^?<86>æ^\TI^[SI|åÉ^R<81>Õ*QNb^\èVÝõ<95>#Ë^M^T^C^Eóì-<83>ÀC þ*<90>%^B+<80>^?¿äÄñ^XèϤ¥e<9 -a>,^O°^Vp-<90>l<9f>^@Â<99><8a>gR^FOÌ^O<84>TËZ(HZù3õ'íÉ2<81>^R Ìé+oll¤½½<9d>þþ~^TEAQ^T"<91>^HW¯^åèÑ£¸\º^F]^F¬|Ùn(^@ -å@<9e>S^DíÚµ<8b>cÇ<8e>±iÓ¦<94>cãñ8Ç<8f>^_§©©<89>^[7nh^M^Y^Fþ|YdU8ET0^X¤©©<89>Í<9b>7[þî^W_|ÁÄÄ^DçÏ<9f>çÑ£G^Y#,<9d>< -98>µ^RXæ^DQõõõ´¶¶RVfϳÇÇÇyøð!<95><95><95>dggsïÞ½<99><87>½j^B^Z<99>¯<98>åW^CgƱsçN<9a><9b><9b>ÉÎζ=G<þw<89>µaÃ^F^Z^ -Z^Zf^OYag^UaDz§ã<83>`?B<9c>9sæï<85>¥¢^P^L^Fµ,Ì^O^LX©Ã$^[<96>XéTyðË/¿<90><9b><9b>kûûCCC<9c>:u<8a>ÁÁÁ -^WÈN^RöøñcFF^ð¾^B bVÉ°Z<^F<9c>*8¿ùæ^[<82>Á á<98>X,FKK^K'O<9e>äâÅ<8b>ȲLAA^A[·n¥¸¸^XA^Pp»ÝºV¹wï^¾üòËÙ×^_PU<8c><8c>f -C7Pí^DQeee<84>ÃaÜn·î<98><9e><9e>^^¶oß®<95>ݦM^^T©®®¦®®<8e>©©)Ý1ׯ_§½½}ö¡ßͬ%­¸S±SµÔ<9e>={^L<89>úé§<9f>¨¨¨Ð% + content = b"""\x00\x00\x01\x00\x01\x00\x18\x18\x00\x00\x01\x00 \x00\x88 +\t\x00\x00\x16\x00\x00\x00(\x00\x00\x00\x18\x00x00\x00\x01\x00 \x00\x00\x00 +\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00 +00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xa7lM\x01\xa6kM\t\xa6kM\x01 +\xa4fF\x04\xa2dE\x95\xa2cD8\xa1a """ parents = [] @@ -515,7 +506,7 @@ C7Pí^DQeee<84>ÃaÜn·î<98><9e><9e>^^¶oß®<95>ݦM^^T©®®¦®®<8e>©©)� parents = [commit.oid.hex] # Create a file in that git repo - with open(os.path.join(newfolder, filename), 'w') as stream: + with open(os.path.join(newfolder, filename), 'wb') as stream: stream.write(content) repo.index.add(filename) repo.index.write() From 56a350d2152ca088996728626822a1bff42d6298 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 11:49:21 +0000 Subject: [PATCH 2/6] Adjust the checks in the tests to the new binary file --- diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index 4a518f5..ca0808d 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -976,7 +976,7 @@ class PagureFlaskRepotests(tests.Modeltests): # View what's supposed to be an image output = self.app.get('/test/raw/master/f/test.jpg') self.assertEqual(output.status_code, 200) - self.assertTrue(output.data.startswith('<89>PNG^M')) + self.assertTrue(output.data.startswith('\x00\x00\x01\x00')) # View by commit id repo = pygit2.Repository(os.path.join(tests.HERE, 'test.git')) @@ -984,17 +984,17 @@ class PagureFlaskRepotests(tests.Modeltests): output = self.app.get('/test/raw/%s/f/test.jpg' % commit.oid.hex) self.assertEqual(output.status_code, 200) - self.assertTrue(output.data.startswith('<89>PNG^M')) + self.assertTrue(output.data.startswith('\x00\x00\x01\x00')) # View by image name -- somehow we support this output = self.app.get('/test/raw/sources/f/test.jpg') self.assertEqual(output.status_code, 200) - self.assertTrue(output.data.startswith('<89>PNG^M')) + self.assertTrue(output.data.startswith('\x00\x00\x01\x00')) # View binary file output = self.app.get('/test/raw/sources/f/test_binary') self.assertEqual(output.status_code, 200) - self.assertTrue(output.data.startswith('<89>PNG^M')) + self.assertTrue(output.data.startswith('\x00\x00\x01\x00')) # View folder output = self.app.get('/test/raw/master/f/folder1') From 801c6f3f2d6c41f310bff62c2d9ee671a1113a9e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 11:49:21 +0000 Subject: [PATCH 3/6] Move the binary detection to the python module binaryornot --- diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 00f039f..b0ff1ab 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -30,6 +30,8 @@ from sqlalchemy.exc import SQLAlchemyError import mimetypes import chardet +from binaryornot.helpers import is_binary_string + import pagure.exceptions import pagure.lib import pagure.lib.git @@ -434,7 +436,7 @@ def view_file(repo, identifier, filename, username=None): elif ext in ('.rst', '.mk', '.md') and not rawtext: content, safe = pagure.doc_utils.convert_readme(content.data, ext) output_type = 'markup' - elif not content.is_binary and pagure.lib.could_be_text(content.data): + elif not is_binary_string(content.data): file_content = content.data.decode('utf-8') try: lexer = guess_lexer_for_filename( @@ -1458,7 +1460,7 @@ def edit_file(repo, branchname, filename, username=None): flask.abort(404, 'File not found') data = repo_obj[content.oid].data.decode('utf-8') - if content.is_binary or not pagure.lib.could_be_text(data): + if is_binary_string(content.data): flask.abort(400, 'Cannot edit binary files') else: data = form.content.data.decode('utf-8') From 1a932de7a757788e3c06bd433c09ef3d352be149 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 11:49:21 +0000 Subject: [PATCH 4/6] Add binaryornot to the list of dependencies --- diff --git a/files/pagure.spec b/files/pagure.spec index afbf3ed..dc666f3 100644 --- a/files/pagure.spec +++ b/files/pagure.spec @@ -19,6 +19,7 @@ BuildRequires: python-nose BuildRequires: py-bcrypt BuildRequires: python-alembic BuildRequires: python-arrow +BuildRequires: python-binaryornot BuildRequires: python-bleach BuildRequires: python-blinker BuildRequires: python-chardet @@ -53,6 +54,7 @@ Requires: python-sqlalchemy > 0.8 Requires: py-bcrypt Requires: python-alembic Requires: python-arrow +Requires: python-binaryornot Requires: python-bleach Requires: python-blinker Requires: python-chardet diff --git a/requirements-fedora.txt b/requirements-fedora.txt index d062896..673b1f8 100644 --- a/requirements-fedora.txt +++ b/requirements-fedora.txt @@ -1,5 +1,6 @@ python-alembic python-arrow +python-binaryornot python-bleach python-blinker python-chardet diff --git a/requirements.txt b/requirements.txt index 9b40360..3ac1742 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,6 +2,7 @@ # Use this file by running "$ pip install -r requirements.txt" alembic arrow +binaryornot bleach blinker chardet From 03b9736aa0fbf3fc3503de4b06e5107a1d03e8ce Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 11:49:21 +0000 Subject: [PATCH 5/6] Only decode the content of the file once we've check if it is a binary or not --- diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index b0ff1ab..c09be9d 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1459,9 +1459,10 @@ def edit_file(repo, branchname, filename, username=None): if not content or isinstance(content, pygit2.Tree): flask.abort(404, 'File not found') - data = repo_obj[content.oid].data.decode('utf-8') if is_binary_string(content.data): flask.abort(400, 'Cannot edit binary files') + + data = repo_obj[content.oid].data.decode('utf-8') else: data = form.content.data.decode('utf-8') From 7ae126914709dc3b65c01af534d37a6791bfe09f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 22 2016 14:50:22 +0000 Subject: [PATCH 6/6] By not relying on pygit2's behavior we're better at detecting binaries And we can enable again this part of the tests :) --- diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index ca0808d..dfea3c6 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -883,11 +883,10 @@ class PagureFlaskRepotests(tests.Modeltests): # View binary file output = self.app.get('/test/blob/sources/f/test_binary') self.assertEqual(output.status_code, 200) - # In newer pygit2 patch.is_binary behaves differently - #self.assertIn('/f/test_binary">view the raw version', output.data) - #self.assertTrue( - #'Binary files cannot be rendered.
' - #in output.data) + self.assertIn('/f/test_binary">view the raw version', output.data) + self.assertTrue( + 'Binary files cannot be rendered.
' + in output.data) # View folder output = self.app.get('/test/blob/master/f/folder1')