core: various fixes of bytes vs str usage based on rhodecode-ce tests outputs

vcsserver/base.py

@@ -25,6 +25,7 b' from vcsserver.lib.rc_cache import regio'
 from vcsserver import exceptions
 from vcsserver.exceptions import NoContentException
 from vcsserver.hgcompat import (archival)
+from vcsserver.str_utils import safe_bytes
 
 log = logging.getLogger(__name__)
 
@@ -112,13 +113,13 b' def archive_repo(walker, archive_dest_pa'
             'Remote does not support: "%s" archive type.' % kind)
 
     for f in walker(commit_id, archive_at_path):
-        f_path = os.path.join(archive_dir_name, f.path.lstrip('/'))
+        f_path = os.path.join(safe_bytes(archive_dir_name), f.path.lstrip(b'/'))
         try:
             archiver.addfile(f_path, f.mode, f.is_link, f.raw_bytes())
         except NoContentException:
             # NOTE(marcink): this is a special case for SVN so we can create "empty"
             # directories which arent supported by archiver
-            archiver.addfile(os.path.join(f_path, '.dir'), f.mode, f.is_link, '')
+            archiver.addfile(os.path.join(f_path, b'.dir'), f.mode, f.is_link, '')
 
     if write_metadata:
         metadata = dict([
@@ -127,8 +128,8 b' def archive_repo(walker, archive_dest_pa'
         ])
         metadata.update(extra_metadata)
 
-        meta = ["%s:%s" % (f_name, value) for f_name, value in metadata.items()]
-        f_path = os.path.join(archive_dir_name, '.archival.txt')
-        archiver.addfile(f_path, 0o644, False, '\n'.join(meta))
+        meta = [safe_bytes(f"{f_name}:{value}") for f_name, value in metadata.items()]
+        f_path = os.path.join(safe_bytes(archive_dir_name), b'.archival.txt')
+        archiver.addfile(f_path, 0o644, False, b'\n'.join(meta))
 
     return archiver.done()

vcsserver/git_lfs/app.py

@@ -42,7 +42,7 b' def write_response_error(http_exception,'
     _exception = http_exception(content_type=content_type)
     _exception.content_type = content_type
     if text:
-        _exception.text = json.dumps({'message': text})
+        _exception.body = json.dumps({'message': text})
     log.debug('LFS: writing response of type %s to client with text:%s',
               http_exception, text)
     return _exception

vcsserver/http_main.py

@@ -39,7 +39,7 b' from pyramid.response import Response'
 
 from vcsserver.lib.rc_json import json
 from vcsserver.config.settings_maker import SettingsMaker
-from vcsserver.str_utils import safe_int
+from vcsserver.str_utils import safe_int, safe_bytes, safe_str
 from vcsserver.lib.statsd_client import StatsdClient
 
 log = logging.getLogger(__name__)

vcsserver/remote/git.py

@@ -50,7 +50,7 b' from vcsserver.vcs_base import RemoteBas'
 DIR_STAT = stat.S_IFDIR
 FILE_MODE = stat.S_IFMT
 GIT_LINK = objects.S_IFGITLINK
-PEELED_REF_MARKER = '^{}'
+PEELED_REF_MARKER = b'^{}'
 
 
 log = logging.getLogger(__name__)
@@ -156,7 +156,7 b' class GitRemote(RemoteBase):'
         prefix = b'git version'
         if stdout.startswith(prefix):
             stdout = stdout[len(prefix):]
-        return stdout.strip()
+        return safe_str(stdout.strip())
 
     @reraise_safe_exceptions
     def is_empty(self, wire):
@@ -559,15 +559,24 b' class GitRemote(RemoteBase):'
         # Create commit
         commit = objects.Commit()
         commit.tree = commit_tree.id
+        bytes_keys = [
+            'author',
+            'committer',
+            'message',
+            'encoding'
+        ]
+
         for k, v in commit_data.items():
+            if k in bytes_keys:
+                v = safe_bytes(v)
             setattr(commit, k, v)
+
         object_store.add_object(commit)
 
-        self.create_branch(wire, branch, commit.id)
+        self.create_branch(wire, branch, safe_str(commit.id))
 
         # dulwich set-ref
-        ref = 'refs/heads/%s' % branch
-        repo.refs[ref] = commit.id
+        repo.refs[safe_bytes(f'refs/heads/{branch}')] = commit.id
 
         return commit.id
 
@@ -645,7 +654,7 b' class GitRemote(RemoteBase):'
         fetch_refs = []
 
         for ref_line in output.splitlines():
-            sha, ref = ref_line.split('\t')
+            sha, ref = ref_line.split(b'\t')
             sha = sha.strip()
             if ref in remote_refs:
                 # duplicate, skip
@@ -654,16 +663,16 b' class GitRemote(RemoteBase):'
                 log.debug("Skipping peeled reference %s", ref)
                 continue
             # don't sync HEAD
-            if ref in ['HEAD']:
+            if ref in [b'HEAD']:
                 continue
 
             remote_refs[ref] = sha
 
             if refs and sha in refs:
                 # we filter fetch using our specified refs
-                fetch_refs.append('{}:{}'.format(ref, ref))
+                fetch_refs.append(f'{safe_str(ref)}:{safe_str(ref)}')
             elif not refs:
-                fetch_refs.append('{}:{}'.format(ref, ref))
+                fetch_refs.append(f'{safe_str(ref)}:{safe_str(ref)}')
         log.debug('Finished obtaining fetch refs, total: %s', len(fetch_refs))
 
         if fetch_refs:
@@ -916,6 +925,7 b' class GitRemote(RemoteBase):'
     def parents(self, wire, commit_id):
         cache_on, context_uid, repo_id = self._cache_on(wire)
         region = self._region(wire)
+
         @region.conditional_cache_on_arguments(condition=cache_on)
         def _parents(_repo_id, _commit_id):
             repo_init = self._factory.repo_libgit2(wire)
@@ -1241,7 +1251,7 b' class GitRemote(RemoteBase):'
 
             return b''.join(proc), b''.join(proc.stderr)
         except OSError as err:
-            cmd = ' '.join(cmd)  # human friendly CMD
+            cmd = ' '.join(map(safe_str, cmd))  # human friendly CMD
             tb_err = ("Couldn't run git command (%s).\n"
                       "Original error was:%s\n"
                       "Call options:%s\n"

vcsserver/remote/hg.py

@@ -200,7 +200,7 b' class HgRemote(RemoteBase):'
     @reraise_safe_exceptions
     def discover_hg_version(self):
         from mercurial import util
-        return util.version()
+        return safe_str(util.version())
 
     @reraise_safe_exceptions
     def is_empty(self, wire):
@@ -216,10 +216,11 b' class HgRemote(RemoteBase):'
     def bookmarks(self, wire):
         cache_on, context_uid, repo_id = self._cache_on(wire)
         region = self._region(wire)
+
         @region.conditional_cache_on_arguments(condition=cache_on)
         def _bookmarks(_context_uid, _repo_id):
             repo = self._factory.repo(wire)
-            return dict(repo._bookmarks)
+            return {safe_str(name): ascii_str(hex(sha)) for name, sha in repo._bookmarks.items()}
 
         return _bookmarks(context_uid, repo_id)
 
@@ -227,16 +228,17 b' class HgRemote(RemoteBase):'
     def branches(self, wire, normal, closed):
         cache_on, context_uid, repo_id = self._cache_on(wire)
         region = self._region(wire)
+
         @region.conditional_cache_on_arguments(condition=cache_on)
         def _branches(_context_uid, _repo_id, _normal, _closed):
             repo = self._factory.repo(wire)
             iter_branches = repo.branchmap().iterbranches()
             bt = {}
-            for branch_name, _heads, tip, is_closed in iter_branches:
+            for branch_name, _heads, tip_node, is_closed in iter_branches:
                 if normal and not is_closed:
-                    bt[branch_name] = tip
+                    bt[safe_str(branch_name)] = ascii_str(hex(tip_node))
                 if closed and is_closed:
-                    bt[branch_name] = tip
+                    bt[safe_str(branch_name)] = ascii_str(hex(tip_node))
 
             return bt
 
@@ -246,6 +248,7 b' class HgRemote(RemoteBase):'
     def bulk_request(self, wire, commit_id, pre_load):
         cache_on, context_uid, repo_id = self._cache_on(wire)
         region = self._region(wire)
+
         @region.conditional_cache_on_arguments(condition=cache_on)
         def _bulk_request(_repo_id, _commit_id, _pre_load):
             result = {}
@@ -264,6 +267,7 b' class HgRemote(RemoteBase):'
     def ctx_branch(self, wire, commit_id):
         cache_on, context_uid, repo_id = self._cache_on(wire)
         region = self._region(wire)
+
         @region.conditional_cache_on_arguments(condition=cache_on)
         def _ctx_branch(_repo_id, _commit_id):
             repo = self._factory.repo(wire)
@@ -275,6 +279,7 b' class HgRemote(RemoteBase):'
     def ctx_date(self, wire, commit_id):
         cache_on, context_uid, repo_id = self._cache_on(wire)
         region = self._region(wire)
+
         @region.conditional_cache_on_arguments(condition=cache_on)
         def _ctx_date(_repo_id, _commit_id):
             repo = self._factory.repo(wire)
@@ -292,6 +297,7 b' class HgRemote(RemoteBase):'
     def ctx_files(self, wire, commit_id):
         cache_on, context_uid, repo_id = self._cache_on(wire)
         region = self._region(wire)
+
         @region.conditional_cache_on_arguments(condition=cache_on)
         def _ctx_files(_repo_id, _commit_id):
             repo = self._factory.repo(wire)
@@ -310,6 +316,7 b' class HgRemote(RemoteBase):'
     def ctx_parents(self, wire, commit_id):
         cache_on, context_uid, repo_id = self._cache_on(wire)
         region = self._region(wire)
+
         @region.conditional_cache_on_arguments(condition=cache_on)
         def _ctx_parents(_repo_id, _commit_id):
             repo = self._factory.repo(wire)
@@ -323,6 +330,7 b' class HgRemote(RemoteBase):'
     def ctx_children(self, wire, commit_id):
         cache_on, context_uid, repo_id = self._cache_on(wire)
         region = self._region(wire)
+
         @region.conditional_cache_on_arguments(condition=cache_on)
         def _ctx_children(_repo_id, _commit_id):
             repo = self._factory.repo(wire)
@@ -336,6 +344,7 b' class HgRemote(RemoteBase):'
     def ctx_phase(self, wire, commit_id):
         cache_on, context_uid, repo_id = self._cache_on(wire)
         region = self._region(wire)
+
         @region.conditional_cache_on_arguments(condition=cache_on)
         def _ctx_phase(_context_uid, _repo_id, _commit_id):
             repo = self._factory.repo(wire)
@@ -348,6 +357,7 b' class HgRemote(RemoteBase):'
     def ctx_obsolete(self, wire, commit_id):
         cache_on, context_uid, repo_id = self._cache_on(wire)
         region = self._region(wire)
+
         @region.conditional_cache_on_arguments(condition=cache_on)
         def _ctx_obsolete(_context_uid, _repo_id, _commit_id):
             repo = self._factory.repo(wire)
@@ -359,6 +369,7 b' class HgRemote(RemoteBase):'
     def ctx_hidden(self, wire, commit_id):
         cache_on, context_uid, repo_id = self._cache_on(wire)
         region = self._region(wire)
+
         @region.conditional_cache_on_arguments(condition=cache_on)
         def _ctx_hidden(_context_uid, _repo_id, _commit_id):
             repo = self._factory.repo(wire)
@@ -464,8 +475,9 b' class HgRemote(RemoteBase):'
         opts = diffopts(git=opt_git, ignorews=opt_ignorews, context=context, showfunc=1)
 
         try:
-            return "".join(patch.diff(
-                repo, node1=commit_id_1, node2=commit_id_2, match=match_filter, opts=opts))
+            diff_iter = patch.diff(
+                repo, node1=commit_id_1, node2=commit_id_2, match=match_filter, opts=opts)
+            return b"".join(diff_iter)
         except RepoLookupError as e:
             raise exceptions.LookupException(e)()
 
@@ -583,7 +595,7 b' class HgRemote(RemoteBase):'
     @reraise_safe_exceptions
     def get_config_value(self, wire, section, name, untrusted=False):
         repo = self._factory.repo(wire)
-        return repo.ui.config(section, name, untrusted=untrusted)
+        return repo.ui.config(ascii_bytes(section), ascii_bytes(name), untrusted=untrusted)
 
     @reraise_safe_exceptions
     def is_large_file(self, wire, commit_id, path):
@@ -681,7 +693,7 b' class HgRemote(RemoteBase):'
         repo.ui.setconfig(b'ui', b'interactive', b'off', b'-y')
 
         bookmarks = list(dict(repo._bookmarks).keys())
-        remote = peer(repo, {}, url)
+        remote = peer(repo, {}, safe_bytes(url))
         # Disable any prompts for this remote
         remote.ui.setconfig(b'ui', b'interactive', b'off', b'-y')
 
@@ -782,7 +794,7 b' class HgRemote(RemoteBase):'
         @region.conditional_cache_on_arguments(condition=cache_on)
         def _tags(_context_uid, _repo_id):
             repo = self._factory.repo(wire)
-            return repo.tags()
+            return {safe_str(name): ascii_str(hex(sha)) for name, sha in repo.tags().items()}
 
         return _tags(context_uid, repo_id)
 
@@ -815,10 +827,10 b' class HgRemote(RemoteBase):'
 
         baseui.write = write
         if branch:
-            args = [branch]
+            args = [safe_bytes(branch)]
         else:
             args = []
-        commands.heads(baseui, repo, template='{node} ', *args)
+        commands.heads(baseui, repo, template=b'{node} ', *args)
 
         return output.getvalue()
 
@@ -833,57 +845,55 b' class HgRemote(RemoteBase):'
     @reraise_safe_exceptions
     def clone(self, wire, source, dest, update_after_clone=False, hooks=True):
         baseui = self._factory._create_config(wire["config"], hooks=hooks)
-        clone(baseui, source, dest, noupdate=not update_after_clone)
+        clone(baseui, safe_bytes(source), safe_bytes(dest), noupdate=not update_after_clone)
 
     @reraise_safe_exceptions
     def commitctx(self, wire, message, parents, commit_time, commit_timezone, user, files, extra, removed, updated):
 
         repo = self._factory.repo(wire)
         baseui = self._factory._create_config(wire['config'])
-        publishing = baseui.configbool('phases', 'publish')
-        if publishing:
-            new_commit = 'public'
-        else:
-            new_commit = 'draft'
+        publishing = baseui.configbool(b'phases', b'publish')
 
-        def _filectxfn(_repo, ctx, path):
+        def _filectxfn(_repo, ctx, path: bytes):
             """
             Marks given path as added/changed/removed in a given _repo. This is
             for internal mercurial commit function.
             """
 
             # check if this path is removed
-            if path in removed:
+            if safe_str(path) in removed:
                 # returning None is a way to mark node for removal
                 return None
 
             # check if this path is added
             for node in updated:
-                if node['path'] == path:
+                if safe_bytes(node['path']) == path:
                     return memfilectx(
                         _repo,
                         changectx=ctx,
-                        path=node['path'],
-                        data=node['content'],
+                        path=safe_bytes(node['path']),
+                        data=safe_bytes(node['content']),
                         islink=False,
                         isexec=bool(node['mode'] & stat.S_IXUSR),
                         copysource=False)
+            abort_exc = exceptions.AbortException()
+            raise abort_exc(f"Given path haven't been marked as added, changed or removed ({path})")
 
-            raise exceptions.AbortException()(
-                "Given path haven't been marked as added, "
-                "changed or removed (%s)" % path)
-
-        with repo.ui.configoverride({('phases', 'new-commit'): new_commit}):
-
+        if publishing:
+            new_commit_phase = b'public'
+        else:
+            new_commit_phase = b'draft'
+        with repo.ui.configoverride({(b'phases', b'new-commit'): new_commit_phase}):
+            kwargs = {safe_bytes(k): safe_bytes(v) for k, v in extra.items()}
             commit_ctx = memctx(
                 repo=repo,
                 parents=parents,
-                text=message,
-                files=files,
+                text=safe_bytes(message),
+                files=[safe_bytes(x) for x in files],
                 filectxfn=_filectxfn,
-                user=user,
+                user=safe_bytes(user),
                 date=(commit_time, commit_timezone),
-                extra=extra)
+                extra=kwargs)
 
             n = repo.commitctx(commit_ctx)
             new_id = hex(n)
@@ -896,7 +906,7 b' class HgRemote(RemoteBase):'
         # Disable any prompts for this repo
         repo.ui.setconfig(b'ui', b'interactive', b'off', b'-y')
 
-        remote = peer(repo, {}, url)
+        remote = peer(repo, {}, safe_bytes(url))
         # Disable any prompts for this remote
         remote.ui.setconfig(b'ui', b'interactive', b'off', b'-y')
 

vcsserver/remote/svn.py

@@ -115,7 +115,7 b' class SvnRemote(RemoteBase):'
             svn_ver = svn.core.SVN_VERSION
         except ImportError:
             svn_ver = None
-        return svn_ver
+        return safe_str(svn_ver)
 
     @reraise_safe_exceptions
     def is_empty(self, wire):
@@ -504,7 +504,7 b' class SvnRemote(RemoteBase):'
             if safe_call:
                 return '', safe_str(err).strip()
             else:
-                cmd = ' '.join(cmd)  # human friendly CMD
+                cmd = ' '.join(map(safe_str, cmd))  # human friendly CMD
                 tb_err = ("Couldn't run svn command (%s).\n"
                           "Original error was:%s\n"
                           "Call options:%s\n"

vcsserver/subprocessio.py

@@ -28,6 +28,8 b' import logging'
 import subprocess
 import threading
 
+from vcsserver.str_utils import safe_str
+
 log = logging.getLogger(__name__)
 
 
@@ -550,7 +552,7 b' def run_command(arguments, env=None):'
         proc = SubprocessIOChunker(cmd, **_opts)
         return b''.join(proc), b''.join(proc.stderr)
     except OSError as err:
-        cmd = ' '.join(cmd)  # human friendly CMD
+        cmd = ' '.join(map(safe_str, cmd)) # human friendly CMD
         tb_err = ("Couldn't run subprocessio command (%s).\n"
                   "Original error was:%s\n" % (cmd, err))
         log.exception(tb_err)