Merge pull request #15 from rodrigc/py3

Fix more bytes/unicode issues

master/buildbot/test/unit/test_www_hooks_github.py

@@ -299,7 +299,7 @@ def _prepare_request(event, payload, _secret=None, headers=None):
                                  msg=unicode2bytes(payload),
                                  digestmod=sha1)
             request.received_headers[_HEADER_SIGNATURE] = \
-                'sha1=%s' % (signature.hexdigest(),)
+                'sha1={}'.format(signature.hexdigest())
     else:
         request.args['payload'] = payload
         request.received_headers[_HEADER_CT] = _CT_ENCODED
@@ -318,21 +318,21 @@ def setUp(self):
 
     @defer.inlineCallbacks
     def test_unknown_event(self):
-        bad_event = 'whatever'
+        bad_event = b'whatever'
         self.request = _prepare_request(bad_event, gitJsonPayload)
         yield self.request.test_render(self.changeHook)
-        expected = b'Unknown event: %r' % (bad_event,)
+        expected = b'Unknown event: ' + bad_event
         self.assertEqual(len(self.changeHook.master.addedChanges), 0)
         self.assertEqual(self.request.written, expected)
 
     @defer.inlineCallbacks
     def test_unknown_content_type(self):
-        bad_content_type = 'application/x-useful'
+        bad_content_type = b'application/x-useful'
         self.request = _prepare_request('push', gitJsonPayload, headers={
             _HEADER_CT: bad_content_type
         })
         yield self.request.test_render(self.changeHook)
-        expected = b'Unknown content type: %r' % (bad_content_type,)
+        expected = b'Unknown content type: ' + bad_content_type
         self.assertEqual(len(self.changeHook.master.addedChanges), 0)
         self.assertEqual(self.request.written, expected)
 
@@ -572,13 +572,12 @@ def test_signature_ok(self):
     def test_unknown_hash(self):
         bad_hash_type = b'blah'
         self.request = _prepare_request('push', gitJsonPayload, headers={
-            _HEADER_SIGNATURE: '%s=doesnotmatter' % (bad_hash_type,)
+            _HEADER_SIGNATURE: bad_hash_type + b'=doesnotmatter'
         })
         yield self.request.test_render(self.changeHook)
-        expected = b'Unknown hash type: '
+        expected = b'Unknown hash type: ' + bad_hash_type
         self.assertEqual(len(self.changeHook.master.addedChanges), 0)
-        self.assertIn(expected, self.request.written)
-        self.assertIn(bad_hash_type, self.request.written)
+        self.assertEqual(self.request.written, expected)
 
     @defer.inlineCallbacks
     def test_signature_nok(self):
@@ -603,12 +602,12 @@ def test_missing_secret(self):
 
     @defer.inlineCallbacks
     def test_wrong_signature_format(self):
-        bad_signature = 'hash=value=something'
+        bad_signature = b'hash=value=something'
         self.request = _prepare_request('push', gitJsonPayload, headers={
             _HEADER_SIGNATURE: bad_signature
         })
         yield self.request.test_render(self.changeHook)
-        expected = b'Wrong signature format: %r' % (bad_signature,)
+        expected = b'Wrong signature format: ' + bad_signature
         self.assertEqual(len(self.changeHook.master.addedChanges), 0)
         self.assertEqual(self.request.written, expected)
 

master/buildbot/www/hooks/github.py

@@ -25,6 +25,7 @@
 
 from twisted.python import log
 
+from buildbot.util import bytes2NativeString
 from buildbot.util import unicode2bytes
 
 try:
@@ -34,9 +35,9 @@
     import simplejson as json
 
 
-_HEADER_CT = 'Content-Type'
-_HEADER_EVENT = 'X-GitHub-Event'
-_HEADER_SIGNATURE = 'X-Hub-Signature'
+_HEADER_CT = b'Content-Type'
+_HEADER_EVENT = b'X-GitHub-Event'
+_HEADER_SIGNATURE = b'X-Hub-Signature'
 
 
 class GitHubEventHandler(object):
@@ -54,19 +55,23 @@ def process(self, request):
         payload = self._get_payload(request)
 
         event_type = request.getHeader(_HEADER_EVENT)
-        log.msg("X-GitHub-Event: %r" % (event_type,), logLevel=logging.DEBUG)
+        event_type = bytes2NativeString(event_type)
+        log.msg("X-GitHub-Event: {}".format(
+            event_type), logLevel=logging.DEBUG)
 
-        handler = getattr(self, 'handle_%s' % event_type, None)
+        handler = getattr(self, 'handle_{}'.format(event_type), None)
 
         if handler is None:
-            raise ValueError('Unknown event: %r' % (event_type,))
+            raise ValueError('Unknown event: {}'.format(event_type))
 
         return handler(payload)
 
     def _get_payload(self, request):
         content = request.content.read()
+        content = bytes2NativeString(content)
 
         signature = request.getHeader(_HEADER_SIGNATURE)
+        signature = bytes2NativeString(signature)
 
         if not signature and self._strict:
             raise ValueError('Request has no required signature')
@@ -75,10 +80,11 @@ def _get_payload(self, request):
             try:
                 hash_type, hexdigest = signature.split('=')
             except ValueError:
-                raise ValueError('Wrong signature format: %r' % (signature,))
+                raise ValueError(
+                    'Wrong signature format: {}'.format(signature))
 
             if hash_type != 'sha1':
-                raise ValueError('Unknown hash type: %s' % (hash_type,))
+                raise ValueError('Unknown hash type: {}'.format(hash_type))
 
             mac = hmac.new(unicode2bytes(self._secret),
                            msg=unicode2bytes(content),
@@ -89,15 +95,16 @@ def _get_payload(self, request):
                 raise ValueError('Hash mismatch')
 
         content_type = request.getHeader(_HEADER_CT)
+        content_type = bytes2NativeString(content_type)
 
         if content_type == 'application/json':
             payload = json.loads(content)
         elif content_type == 'application/x-www-form-urlencoded':
             payload = json.loads(request.args['payload'][0])
         else:
-            raise ValueError('Unknown content type: %r' % (content_type,))
+            raise ValueError('Unknown content type: {}'.format(content_type))
 
-        log.msg("Payload: %r" % payload, logLevel=logging.DEBUG)
+        log.msg("Payload: {}".format(payload), logLevel=logging.DEBUG)
 
         return payload
 
@@ -116,21 +123,22 @@ def handle_push(self, payload):
 
         changes = self._process_change(payload, user, repo, repo_url, project)
 
-        log.msg("Received %d changes from github" % len(changes))
+        log.msg("Received {} changes from github".format(len(changes)))
 
         return changes, 'git'
 
     def handle_pull_request(self, payload):
         changes = []
         number = payload['number']
-        refname = 'refs/pull/%d/merge' % (number,)
+        refname = 'refs/pull/{}/merge'.format(number)
         commits = payload['pull_request']['commits']
 
-        log.msg('Processing GitHub PR #%d' % number, logLevel=logging.DEBUG)
+        log.msg('Processing GitHub PR #{}'.format(number),
+                logLevel=logging.DEBUG)
 
         action = payload.get('action')
         if action not in ('opened', 'reopened', 'synchronize'):
-            log.msg("GitHub PR #%d %s, ignoring" % (number, action))
+            log.msg("GitHub PR #{} {}, ignoring".format(number, action))
             return changes, 'git'
 
         change = {
@@ -143,8 +151,8 @@ def handle_pull_request(self, payload):
             'category': 'pull',
             # TODO: Get author name based on login id using txgithub module
             'author': payload['sender']['login'],
-            'comments': 'GitHub Pull Request #%d (%d commit%s)' % (
-                number, commits, 's' if commits != 1 else ''),
+            'comments': 'GitHub Pull Request #{} ({} commit{})'.format(
+                number, commits, 's' if commits != 1 else '')
         }
 
         if callable(self._codebase):
@@ -154,7 +162,7 @@ def handle_pull_request(self, payload):
 
         changes.append(change)
 
-        log.msg("Received %d changes from GitHub PR #%d" % (
+        log.msg("Received {} changes from GitHub PR #{}".format(
             len(changes), number))
         return changes, 'git'
 
@@ -173,12 +181,12 @@ def _process_change(self, payload, user, repo, repo_url, project):
         # We only care about regular heads, i.e. branches
         match = re.match(r"^refs/heads/(.+)$", refname)
         if not match:
-            log.msg("Ignoring refname `%s': Not a branch" % refname)
+            log.msg("Ignoring refname `{}': Not a branch".format(refname))
             return changes
 
         branch = match.group(1)
         if payload.get('deleted'):
-            log.msg("Branch `%s' deleted, ignoring" % branch)
+            log.msg("Branch `{}' deleted, ignoring".format(branch))
             return changes
 
         for commit in payload['commits']:
@@ -188,11 +196,11 @@ def _process_change(self, payload, user, repo, repo_url, project):
 
             when_timestamp = dateparse(commit['timestamp'])
 
-            log.msg("New revision: %s" % commit['id'][:8])
+            log.msg("New revision: {}".format(commit['id'][:8]))
 
             change = {
-                'author': '%s <%s>' % (commit['author']['name'],
-                                       commit['author']['email']),
+                'author': '{} <{}>'.format(commit['author']['name'],
+                                           commit['author']['email']),
                 'files': files,
                 'comments': commit['message'],
                 'revision': commit['id'],