diff --git a/.bumpversion.cfg b/.bumpversion.cfg --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,6 +1,5 @@ [bumpversion] -current_version = 4.25.2 +current_version = 4.26.0 message = release: Bump version {current_version} to {new_version} [bumpversion:file:rhodecode/VERSION] - diff --git a/.release.cfg b/.release.cfg --- a/.release.cfg +++ b/.release.cfg @@ -5,25 +5,20 @@ done = false done = true [task:rc_tools_pinned] -done = true [task:fixes_on_stable] -done = true [task:pip2nix_generated] -done = true [task:changelog_updated] -done = true [task:generate_api_docs] -done = true + +[task:updated_translation] [release] -state = prepared -version = 4.25.2 - -[task:updated_translation] +state = in_progress +version = 4.26.0 [task:generate_js_routes] diff --git a/configs/production.ini b/configs/production.ini --- a/configs/production.ini +++ b/configs/production.ini @@ -391,6 +391,8 @@ rc_cache.cache_perms.expiration_time = 3 ; more Redis options: https://dogpilecache.sqlalchemy.org/en/latest/api.html#redis-backends #rc_cache.cache_perms.arguments.distributed_lock = true +; auto-renew lock to prevent stale locks, slower but safer. Use only if problems happen +#rc_cache.cache_perms.arguments.lock_auto_renewal = true ; *************************************************** ; `cache_repo` cache for file tree, Readme, RSS FEEDS @@ -414,6 +416,8 @@ rc_cache.cache_repo.expiration_time = 25 ; more Redis options: https://dogpilecache.sqlalchemy.org/en/latest/api.html#redis-backends #rc_cache.cache_repo.arguments.distributed_lock = true +; auto-renew lock to prevent stale locks, slower but safer. Use only if problems happen +#rc_cache.cache_repo.arguments.lock_auto_renewal = true ; ############## ; BEAKER SESSION diff --git a/docs/release-notes/release-notes-4.26.0.rst b/docs/release-notes/release-notes-4.26.0.rst new file mode 100644 --- /dev/null +++ b/docs/release-notes/release-notes-4.26.0.rst @@ -0,0 +1,55 @@ +|RCE| 4.26.0 |RNS| +------------------ + +Release Date +^^^^^^^^^^^^ + +- 2021-08-06 + + +New Features +^^^^^^^^^^^^ + + + +General +^^^^^^^ + +- Caches: introduce invalidation as a safer ways to expire keys, deleting them are more problematic. +- Caches: improved locking problems with distributed lock new cache backend. +- Pull requests: optimize db transaction logic. + This should prevent potential problems with locking of pull-requests that have a lot of reviewers. +- Pull requests: updates use retry logic in case of update is locked/fails for some concurrency issues. +- Pull requests: allow forced state change to repo admins too. +- SSH: handle subrepos better when using SSH communication. + + +Security +^^^^^^^^ + +- Drafts comments: don't allow to view history for others than owner. +- Validators: apply username validator to prevent bad values being searched in DB, and potential XSS payload sent via validators. + + +Performance +^^^^^^^^^^^ + +- SSH: use pre-compiled backends for faster matching of vcs detection. +- Routing: don't check channelstream connections for faster handling of this route. +- Routing: skip vcsdetection for ops view so they are not checked against the vcs operations. + + +Fixes +^^^^^ + +- Permissions: flush all users permissions when creating a new user group. +- Repos: recover properly from bad extraction of repo_id from URL and DB calls. +- Comments history: fixed fetching of history for comments +- Pull requests: fix potential crash on providing a wrong order-by type column. +- Caches: report damaged DB on key iterations too not only the GET call +- API: added proper full permission flush on API calls when creating repos and repo groups. + +Upgrade notes +^^^^^^^^^^^^^ + +- Scheduled release 4.26.0. diff --git a/docs/release-notes/release-notes.rst b/docs/release-notes/release-notes.rst --- a/docs/release-notes/release-notes.rst +++ b/docs/release-notes/release-notes.rst @@ -9,6 +9,7 @@ Release Notes .. toctree:: :maxdepth: 1 + release-notes-4.26.0.rst release-notes-4.25.2.rst release-notes-4.25.1.rst release-notes-4.25.0.rst diff --git a/pkgs/python-packages.nix b/pkgs/python-packages.nix --- a/pkgs/python-packages.nix +++ b/pkgs/python-packages.nix @@ -1883,7 +1883,7 @@ self: super: { }; }; "rhodecode-enterprise-ce" = super.buildPythonPackage { - name = "rhodecode-enterprise-ce-4.25.2"; + name = "rhodecode-enterprise-ce-4.26.0"; buildInputs = [ self."pytest" self."py" diff --git a/rhodecode/VERSION b/rhodecode/VERSION --- a/rhodecode/VERSION +++ b/rhodecode/VERSION @@ -1,1 +1,1 @@ -4.25.2 \ No newline at end of file +4.26.0 \ No newline at end of file diff --git a/rhodecode/apps/_base/subscribers.py b/rhodecode/apps/_base/subscribers.py --- a/rhodecode/apps/_base/subscribers.py +++ b/rhodecode/apps/_base/subscribers.py @@ -41,13 +41,14 @@ def trigger_user_permission_flush(event) automatic flush of permission caches, so the users affected receive new permissions Right Away """ - + invalidate = True affected_user_ids = set(event.user_ids) for user_id in affected_user_ids: for cache_namespace_uid_tmpl in cache_namespaces: cache_namespace_uid = cache_namespace_uid_tmpl.format(user_id) - del_keys = rc_cache.clear_cache_namespace('cache_perms', cache_namespace_uid) - log.debug('Deleted %s cache keys for user_id: %s and namespace %s', + del_keys = rc_cache.clear_cache_namespace( + 'cache_perms', cache_namespace_uid, invalidate=invalidate) + log.debug('Invalidated %s cache keys for user_id: %s and namespace %s', del_keys, user_id, cache_namespace_uid) diff --git a/rhodecode/apps/admin/views/user_groups.py b/rhodecode/apps/admin/views/user_groups.py --- a/rhodecode/apps/admin/views/user_groups.py +++ b/rhodecode/apps/admin/views/user_groups.py @@ -247,8 +247,7 @@ class AdminUserGroupsView(BaseAppView, D % user_group_name, category='error') raise HTTPFound(h.route_path('user_groups_new')) - affected_user_ids = [self._rhodecode_user.user_id] - PermissionModel().trigger_permission_flush(affected_user_ids) + PermissionModel().trigger_permission_flush() raise HTTPFound( h.route_path('edit_user_group', user_group_id=user_group_id)) diff --git a/rhodecode/apps/repository/__init__.py b/rhodecode/apps/repository/__init__.py --- a/rhodecode/apps/repository/__init__.py +++ b/rhodecode/apps/repository/__init__.py @@ -173,7 +173,7 @@ def includeme(config): config.add_route( name='repo_commit_comment_history_view', - pattern='/{repo_name:.*?[^/]}/changeset/{commit_id}/comment/{comment_history_id}/history_view', repo_route=True) + pattern='/{repo_name:.*?[^/]}/changeset/{commit_id}/comment/{comment_id}/history_view/{comment_history_id}', repo_route=True) config.add_view( RepoCommitsView, attr='repo_commit_comment_history_view', diff --git a/rhodecode/apps/repository/views/repo_changelog.py b/rhodecode/apps/repository/views/repo_changelog.py --- a/rhodecode/apps/repository/views/repo_changelog.py +++ b/rhodecode/apps/repository/views/repo_changelog.py @@ -72,7 +72,7 @@ class RepoChangelogView(RepoAppView): h.flash(msg, category='error') raise HTTPNotFound() except RepositoryError as e: - h.flash(safe_str(h.escape(e)), category='error') + h.flash(h.escape(safe_str(e)), category='error') raise HTTPNotFound() def _graph(self, repo, commits, prev_data=None, next_data=None): @@ -238,14 +238,14 @@ class RepoChangelogView(RepoAppView): f_path=f_path, commit_id=commit_id) except EmptyRepositoryError as e: - h.flash(safe_str(h.escape(e)), category='warning') + h.flash(h.escape(safe_str(e)), category='warning') raise HTTPFound( h.route_path('repo_summary', repo_name=self.db_repo_name)) except HTTPFound: raise except (RepositoryError, CommitDoesNotExistError, Exception) as e: log.exception(safe_str(e)) - h.flash(safe_str(h.escape(e)), category='error') + h.flash(h.escape(safe_str(e)), category='error') if commit_id: # from single commit page, we redirect to main commits diff --git a/rhodecode/apps/repository/views/repo_commits.py b/rhodecode/apps/repository/views/repo_commits.py --- a/rhodecode/apps/repository/views/repo_commits.py +++ b/rhodecode/apps/repository/views/repo_commits.py @@ -539,9 +539,10 @@ class RepoCommitsView(RepoAppView): @CSRFRequired() def repo_commit_comment_history_view(self): c = self.load_default_context() + comment_id = self.request.matchdict['comment_id'] comment_history_id = self.request.matchdict['comment_history_id'] - comment = ChangesetComment.get_or_404(comment_history_id) + comment = ChangesetComment.get_or_404(comment_id) comment_owner = (comment.author.user_id == self._rhodecode_db_user.user_id) if comment.draft and not comment_owner: # if we see draft comments history, we only allow this for owner diff --git a/rhodecode/apps/repository/views/repo_compare.py b/rhodecode/apps/repository/views/repo_compare.py --- a/rhodecode/apps/repository/views/repo_compare.py +++ b/rhodecode/apps/repository/views/repo_compare.py @@ -70,7 +70,7 @@ class RepoCompareView(RepoAppView): except RepositoryError as e: log.exception(safe_str(e)) - h.flash(safe_str(h.escape(e)), category='warning') + h.flash(h.escape(safe_str(e)), category='warning') if not partial: raise HTTPFound( h.route_path('repo_summary', repo_name=repo.repo_name)) diff --git a/rhodecode/apps/repository/views/repo_files.py b/rhodecode/apps/repository/views/repo_files.py --- a/rhodecode/apps/repository/views/repo_files.py +++ b/rhodecode/apps/repository/views/repo_files.py @@ -186,7 +186,7 @@ class RepoFilesView(RepoAppView): h.flash(msg, category='error') raise HTTPNotFound() except RepositoryError as e: - h.flash(safe_str(h.escape(e)), category='error') + h.flash(h.escape(safe_str(e)), category='error') raise HTTPNotFound() def _get_filenode_or_redirect(self, commit_obj, path): @@ -206,7 +206,7 @@ class RepoFilesView(RepoAppView): raise HTTPNotFound() except RepositoryError as e: log.warning('Repository error while fetching filenode `%s`. Err:%s', path, e) - h.flash(safe_str(h.escape(e)), category='error') + h.flash(h.escape(safe_str(e)), category='error') raise HTTPNotFound() return file_node @@ -733,7 +733,7 @@ class RepoFilesView(RepoAppView): c.commit.raw_id, f_path) except RepositoryError as e: - h.flash(safe_str(h.escape(e)), category='error') + h.flash(h.escape(safe_str(e)), category='error') raise HTTPNotFound() if self.request.environ.get('HTTP_X_PJAX'): @@ -927,7 +927,7 @@ class RepoFilesView(RepoAppView): _d, _f = ScmModel().get_quick_filter_nodes(repo_name, _commit_id, _f_path) except (RepositoryError, CommitDoesNotExistError, Exception) as e: log.exception(safe_str(e)) - h.flash(safe_str(h.escape(e)), category='error') + h.flash(h.escape(safe_str(e)), category='error') raise HTTPFound(h.route_path( 'repo_files', repo_name=self.db_repo_name, commit_id='tip', f_path='/')) @@ -1444,7 +1444,7 @@ class RepoFilesView(RepoAppView): 'contain .. in the path'), category='warning') raise HTTPFound(default_redirect_url) except (NodeError, NodeAlreadyExistsError) as e: - h.flash(_(h.escape(e)), category='error') + h.flash(h.escape(safe_str(e)), category='error') except Exception: log.exception('Error occurred during commit') h.flash(_('Error occurred during commit'), category='error') diff --git a/rhodecode/lib/_vendor/redis_lock/__init__.py b/rhodecode/lib/_vendor/redis_lock/__init__.py --- a/rhodecode/lib/_vendor/redis_lock/__init__.py +++ b/rhodecode/lib/_vendor/redis_lock/__init__.py @@ -10,7 +10,7 @@ from redis import StrictRedis __version__ = '3.7.0' loggers = { - k: getLogger("rhodecode" + ".".join((__name__, k))) + k: getLogger("rhodecode." + ".".join((__name__, k))) for k in [ "acquire", "refresh.thread.start", @@ -221,10 +221,11 @@ class Lock(object): """ logger = loggers["acquire"] - logger.debug("Getting %r ...", self._name) + logger.debug("Getting acquire on %r ...", self._name) if self._held: - raise AlreadyAcquired("Already acquired from this Lock instance.") + owner_id = self.get_owner_id() + raise AlreadyAcquired("Already acquired from this Lock instance. Lock id: {}".format(owner_id)) if not blocking and timeout is not None: raise TimeoutNotUsable("Timeout cannot be used if blocking=False") diff --git a/rhodecode/lib/middleware/vcs.py b/rhodecode/lib/middleware/vcs.py --- a/rhodecode/lib/middleware/vcs.py +++ b/rhodecode/lib/middleware/vcs.py @@ -166,6 +166,9 @@ def detect_vcs_request(environ, backends # static files no detection '_static', + # skip ops ping + '_admin/ops/ping', + # full channelstream connect should be VCS skipped '_admin/channelstream/connect', ] diff --git a/rhodecode/lib/rc_cache/backends.py b/rhodecode/lib/rc_cache/backends.py --- a/rhodecode/lib/rc_cache/backends.py +++ b/rhodecode/lib/rc_cache/backends.py @@ -33,6 +33,8 @@ from dogpile.cache.backends import redis from dogpile.cache.backends.file import NO_VALUE, compat, FileLock from dogpile.cache.util import memoized_property +from pyramid.settings import asbool + from rhodecode.lib.memory_lru_dict import LRUDict, LRUDictDebug @@ -224,6 +226,16 @@ class FileNamespaceBackend(PickleSeriali class BaseRedisBackend(redis_backend.RedisBackend): + key_prefix = '' + + def __init__(self, arguments): + super(BaseRedisBackend, self).__init__(arguments) + self._lock_timeout = self.lock_timeout + self._lock_auto_renewal = asbool(arguments.pop("lock_auto_renewal", True)) + + if self._lock_auto_renewal and not self._lock_timeout: + # set default timeout for auto_renewal + self._lock_timeout = 30 def _create_client(self): args = {} @@ -287,17 +299,10 @@ class BaseRedisBackend(redis_backend.Red def get_mutex(self, key): if self.distributed_lock: - import redis_lock lock_key = redis_backend.u('_lock_{0}').format(key) log.debug('Trying to acquire Redis lock for key %s', lock_key) - lock = redis_lock.Lock( - redis_client=self.client, - name=lock_key, - expire=self.lock_timeout, - auto_renewal=False, - strict=True, - ) - return lock + return get_mutex_lock(self.client, lock_key, self._lock_timeout, + auto_renewal=self._lock_auto_renewal) else: return None @@ -310,3 +315,40 @@ class RedisPickleBackend(PickleSerialize class RedisMsgPackBackend(MsgPackSerializer, BaseRedisBackend): key_prefix = 'redis_msgpack_backend' pass + + +def get_mutex_lock(client, lock_key, lock_timeout, auto_renewal=False): + import redis_lock + + class _RedisLockWrapper(object): + """LockWrapper for redis_lock""" + + @classmethod + def get_lock(cls): + return redis_lock.Lock( + redis_client=client, + name=lock_key, + expire=lock_timeout, + auto_renewal=auto_renewal, + strict=True, + ) + + def __init__(self): + self.lock = self.get_lock() + + def acquire(self, wait=True): + try: + return self.lock.acquire(wait) + except redis_lock.AlreadyAcquired: + return False + except redis_lock.AlreadyStarted: + # refresh thread exists, but it also means we acquired the lock + return True + + def release(self): + try: + self.lock.release() + except redis_lock.NotAcquired: + pass + + return _RedisLockWrapper() diff --git a/rhodecode/lib/rc_cache/utils.py b/rhodecode/lib/rc_cache/utils.py --- a/rhodecode/lib/rc_cache/utils.py +++ b/rhodecode/lib/rc_cache/utils.py @@ -261,12 +261,15 @@ def get_or_create_region(region_name, re return region_obj -def clear_cache_namespace(cache_region, cache_namespace_uid): +def clear_cache_namespace(cache_region, cache_namespace_uid, invalidate=False): region = get_or_create_region(cache_region, cache_namespace_uid) cache_keys = region.backend.list_keys(prefix=cache_namespace_uid) num_delete_keys = len(cache_keys) - if num_delete_keys: - region.delete_multi(cache_keys) + if invalidate: + region.invalidate(hard=False) + else: + if num_delete_keys: + region.delete_multi(cache_keys) return num_delete_keys diff --git a/rhodecode/model/pull_request.py b/rhodecode/model/pull_request.py --- a/rhodecode/model/pull_request.py +++ b/rhodecode/model/pull_request.py @@ -345,14 +345,14 @@ class PullRequestModel(BaseModel): if only_created: q = q.filter(PullRequest.pull_request_state == PullRequest.STATE_CREATED) - if order_by: - order_map = { - 'name_raw': PullRequest.pull_request_id, - 'id': PullRequest.pull_request_id, - 'title': PullRequest.title, - 'updated_on_raw': PullRequest.updated_on, - 'target_repo': PullRequest.target_repo_id - } + order_map = { + 'name_raw': PullRequest.pull_request_id, + 'id': PullRequest.pull_request_id, + 'title': PullRequest.title, + 'updated_on_raw': PullRequest.updated_on, + 'target_repo': PullRequest.target_repo_id + } + if order_by and order_by in order_map: if order_dir == 'asc': q = q.order_by(order_map[order_by].asc()) else: @@ -499,13 +499,13 @@ class PullRequestModel(BaseModel): pull_request_alias.description.ilike(like_expression), )) - if order_by: - order_map = { - 'name_raw': pull_request_alias.pull_request_id, - 'title': pull_request_alias.title, - 'updated_on_raw': pull_request_alias.updated_on, - 'target_repo': pull_request_alias.target_repo_id - } + order_map = { + 'name_raw': pull_request_alias.pull_request_id, + 'title': pull_request_alias.title, + 'updated_on_raw': pull_request_alias.updated_on, + 'target_repo': pull_request_alias.target_repo_id + } + if order_by and order_by in order_map: if order_dir == 'asc': q = q.order_by(order_map[order_by].asc()) else: @@ -585,13 +585,14 @@ class PullRequestModel(BaseModel): PullRequest.title.ilike(like_expression), PullRequest.description.ilike(like_expression), )) - if order_by: - order_map = { - 'name_raw': PullRequest.pull_request_id, - 'title': PullRequest.title, - 'updated_on_raw': PullRequest.updated_on, - 'target_repo': PullRequest.target_repo_id - } + + order_map = { + 'name_raw': PullRequest.pull_request_id, + 'title': PullRequest.title, + 'updated_on_raw': PullRequest.updated_on, + 'target_repo': PullRequest.target_repo_id + } + if order_by and order_by in order_map: if order_dir == 'asc': q = q.order_by(order_map[order_by].asc()) else: @@ -665,13 +666,13 @@ class PullRequestModel(BaseModel): pull_request_alias.description.ilike(like_expression), )) - if order_by: - order_map = { - 'name_raw': pull_request_alias.pull_request_id, - 'title': pull_request_alias.title, - 'updated_on_raw': pull_request_alias.updated_on, - 'target_repo': pull_request_alias.target_repo_id - } + order_map = { + 'name_raw': pull_request_alias.pull_request_id, + 'title': pull_request_alias.title, + 'updated_on_raw': pull_request_alias.updated_on, + 'target_repo': pull_request_alias.target_repo_id + } + if order_by and order_by in order_map: if order_dir == 'asc': q = q.order_by(order_map[order_by].asc()) else: diff --git a/rhodecode/model/repo.py b/rhodecode/model/repo.py --- a/rhodecode/model/repo.py +++ b/rhodecode/model/repo.py @@ -125,13 +125,15 @@ class RepoModel(BaseModel): :param repo_name: :return: repo object if matched else None """ - + _repo_id = None try: _repo_id = self._extract_id_from_repo_name(repo_name) if _repo_id: return self.get(_repo_id) except Exception: log.exception('Failed to extract repo_name from URL') + if _repo_id: + Session().rollback() return None diff --git a/rhodecode/model/scm.py b/rhodecode/model/scm.py --- a/rhodecode/model/scm.py +++ b/rhodecode/model/scm.py @@ -285,7 +285,8 @@ class ScmModel(BaseModel): repo.update_commit_cache(config=config, cs_cache=None) if delete: cache_namespace_uid = 'cache_repo.{}'.format(repo_id) - rc_cache.clear_cache_namespace('cache_repo', cache_namespace_uid) + rc_cache.clear_cache_namespace( + 'cache_repo', cache_namespace_uid, invalidate=True) def toggle_following_repo(self, follow_repo_id, user_id): diff --git a/rhodecode/public/js/rhodecode/routes.js b/rhodecode/public/js/rhodecode/routes.js --- a/rhodecode/public/js/rhodecode/routes.js +++ b/rhodecode/public/js/rhodecode/routes.js @@ -289,7 +289,7 @@ function registerRCRoutes() { pyroutes.register('repo_commit_comment_create', '/%(repo_name)s/changeset/%(commit_id)s/comment/create', ['repo_name', 'commit_id']); pyroutes.register('repo_commit_comment_delete', '/%(repo_name)s/changeset/%(commit_id)s/comment/%(comment_id)s/delete', ['repo_name', 'commit_id', 'comment_id']); pyroutes.register('repo_commit_comment_edit', '/%(repo_name)s/changeset/%(commit_id)s/comment/%(comment_id)s/edit', ['repo_name', 'commit_id', 'comment_id']); - pyroutes.register('repo_commit_comment_history_view', '/%(repo_name)s/changeset/%(commit_id)s/comment/%(comment_history_id)s/history_view', ['repo_name', 'commit_id', 'comment_history_id']); + pyroutes.register('repo_commit_comment_history_view', '/%(repo_name)s/changeset/%(commit_id)s/comment/%(comment_id)s/history_view/%(comment_history_id)s', ['repo_name', 'commit_id', 'comment_id', 'comment_history_id']); pyroutes.register('repo_commit_comment_preview', '/%(repo_name)s/changeset/%(commit_id)s/comment/preview', ['repo_name', 'commit_id']); pyroutes.register('repo_commit_data', '/%(repo_name)s/changeset-data/%(commit_id)s', ['repo_name', 'commit_id']); pyroutes.register('repo_commit_download', '/%(repo_name)s/changeset-download/%(commit_id)s', ['repo_name', 'commit_id']); diff --git a/rhodecode/public/js/src/rhodecode/comments.js b/rhodecode/public/js/src/rhodecode/comments.js --- a/rhodecode/public/js/src/rhodecode/comments.js +++ b/rhodecode/public/js/src/rhodecode/comments.js @@ -572,7 +572,8 @@ var CommentsController = function() { 'repo_commit_comment_history_view', { 'repo_name': templateContext.repo_name, - 'commit_id': comment_id, + 'commit_id': null, // We don't need to check the commit data here... + 'comment_id': comment_id, 'comment_history_id': comment_history_id, } );