Merge lp:~james-w/pkgme-service/base64-icon-data into lp:pkgme-service

Proposed by James Westby
Status: Merged
Approved by: James Westby
Approved revision: 156
Merged at revision: 155
Proposed branch: lp:~james-w/pkgme-service/base64-icon-data
Merge into: lp:pkgme-service
Diff against target: 49 lines (+7/-3)
2 files modified
src/djpkgme/tasks.py (+3/-1)
src/djpkgme/tests/test_tasks.py (+4/-2)
To merge this branch: bzr merge lp:~james-w/pkgme-service/base64-icon-data
Reviewer Review Type Date Requested Status
Martin Albisetti (community) Approve
Review via email: mp+203807@code.launchpad.net

Commit message

base64 the icon data so that it can be safely transmitted as json.

Description of the change

Hi,

Current the icon data, if any, is put raw in to the json string. It's safer
to encode it for transmission as base64.

Thanks,

James

To post a comment you must log in.
Revision history for this message
Martin Albisetti (beuno) :
review: Approve
156. By James Westby

Add a comment about why we base64. Thanks Martin.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/djpkgme/tasks.py'
--- src/djpkgme/tasks.py 2014-01-15 15:27:18 +0000
+++ src/djpkgme/tasks.py 2014-01-29 18:34:40 +0000
@@ -1,3 +1,4 @@
1import base64
1import json2import json
2import os3import os
3import shutil4import shutil
@@ -344,7 +345,8 @@
344 icon_data = None345 icon_data = None
345 try:346 try:
346 full_path = os.path.join(package, icon_path)347 full_path = os.path.join(package, icon_path)
347 icon_data = open(full_path, 'rb').read()348 # base64 icon data, as it is binary to be sent in json
349 icon_data = base64.b64encode(open(full_path, 'rb').read())
348 except:350 except:
349 # gracefully handle not being able to read icon data351 # gracefully handle not being able to read icon data
350 logger.warning(352 logger.warning(
351353
=== modified file 'src/djpkgme/tests/test_tasks.py'
--- src/djpkgme/tests/test_tasks.py 2014-01-15 15:27:18 +0000
+++ src/djpkgme/tests/test_tasks.py 2014-01-29 18:34:40 +0000
@@ -1,6 +1,7 @@
1# Run tasks eagerly for tests. Taken from1# Run tasks eagerly for tests. Taken from
2# http://ask.github.com/celery/cookbook/unit-testing.html2# http://ask.github.com/celery/cookbook/unit-testing.html
33
4import base64
4import json5import json
5import logging6import logging
6import os7import os
@@ -816,7 +817,8 @@
816817
817 @patch('__builtin__.open')818 @patch('__builtin__.open')
818 def test_get_package_icon_ok(self, mock_open):819 def test_get_package_icon_ok(self, mock_open):
819 mock_data = mock_open.return_value.read.return_value820 mock_icon_data = 'some icon'
821 mock_open.return_value.read.return_value = mock_icon_data
820 mock_logger = Mock()822 mock_logger = Mock()
821 job = ClickPackageInfoJob()823 job = ClickPackageInfoJob()
822 info = job.get_package_icon('/unpacked/',824 info = job.get_package_icon('/unpacked/',
@@ -827,7 +829,7 @@
827 self.assertEqual(info, {829 self.assertEqual(info, {
828 'path': 'path/to/icon.png',830 'path': 'path/to/icon.png',
829 'filename': 'icon.png',831 'filename': 'icon.png',
830 'data': mock_data,832 'data': base64.b64encode(mock_icon_data),
831 })833 })
832834
833 @patch('__builtin__.open')835 @patch('__builtin__.open')

Subscribers

People subscribed via source and target branches