celery: celery-run should only initialize app and sqlalchemy after workers have been forked
If app and SqlAlchemy were initialized before launching celery, the forked workers would inherit the database connection ... and that doesn't work.
This could be handled by disposing the engine after forking the worker or before each task ... but it remains unnecessary and wrong to initialize the engine early when it isn't used, and then fork it.
celery: drop tracking of task_id - we use ignore_result=True and will never get anything back
There is thus no need for configuration of celery.result_backend .
The alternative would be to fix it. That could give better error reporting from failing repo creations, but would require quite a bit of additional changes before it actually works reliably.
celery: use explicit task names - avoid automatic naming with "kallithea.lib.celerylib." prefix
We wrap async functions in a local f_async wrapper, defined in kallithea/lib/celerylib/__init__.py . For a function Foo.X, even though we changed the wrapper's __name__ to X, the tasks would be named kallithea.lib.celerylib.X , without using the actual module name of X for namespacing.
Drop modifying __name__, and just specify the name explicitly, without trying to namespace it.
api: fix repo group permission check for repo creation
hg.create.repository is only controlling whether all users have write access at top level. For other repo locations, check for write permission to the repo group.
Note: This also covers "repo creation" by forking or by moving another repo.
model: simplify how get_commits_stats task group on author
Avoid using the caching h.person . We want to get rid of the model dependency on helpers.
The stats are persisted, and any temporary incorrectness in the long term cached h.person will thus remain forever. It is thus arguably better to avoid using it in this place.
get_commits_stats is also a long running task, so speed is not *that* critical. And generally, processing commits in order will have a lot of the same committers, so a local cache will have a good hit rate.
(Alternatively, h.person could perhaps be in user model ... but that's not how it is now.)
lib: move some template filter functions from utils2 to webutils
While quite Kallithea specific, we prefer to have these functions in webutils where they soon can be exposed when templates don't need the whole helpers module.
The original comment in b153a51b1d3b was "to get the pylons_app import" ... which seems like a bad workaround for not having installed the app properly before running.
Passing a whole User wouldn't work if actually using celery and thus serializing the parameters. The test scenario was thus different from actual runtime.
tests: fix create_fork to actually pass a repo id as fork_parent_id
Passing a whole repo wouldn't work if actually using celery and thus serializing the parameters. The test scenario was thus different from actual runtime.
notifications: only render body with mentions if there is a body
Notifications for registering a user would pass body=None. It is unfortunate to pass None all the way to render_w_mentions - it seems fair enough that this function expects a str.
There are some odd characters (like \r and \n) that the Kallithea UI doesn't allow in filenames in repos. Kallithea (through the routes module) will fail to generate URLs when browsing Files. That is a known limitation with minimal real-world impact, non-trivial to work around or fix.
There are very few relevant use cases for tracking files with odd filenames. \t is valid but is hard to render in a meaningful way in the UI. And ASCII characters like \ and " are not usable on Windows and should just be avoided.
Kallithea would parse Git diffs with odd characers incorrectly or fail, even before hitting the known limitation. With this change, Kallithea will parse diffs with odd filenames correctly (and then hit the limitation).
Git will quote odd filenames and escape the odd characters when emitting diffs. (Mercurial does by design not allow \r and \n , and Mercurial will thus never have to quote file names in diffs.)
Quotes are already handled (and ignored). With this change, Kallithea will handle \ unescaping of \\ and \", the usual letters like \r and \n and \t, and octal numbers like \033 (for ESC) .
Filenames with \ and " will work perfectly (when not on Windows).
Filenames with \t and ESC will work fine, but without helpful display in the UI.
Filenames with \r and \n will still make the UI fail when trying to generate URLs.
Thanks to stypr of Flatt Security for raising this.
diff: improved handling of Git diffs with " quoting
Kallithea would intentionally and explicitly fail with an ugly exception when trying to parse Git diffs with quoted filenames.
Improve this by parsing quotes ... and ignore them, as long as they are matching. The content inside the quotes might be \-escaped ... but that could potentially also be the case without quoting. We will fix that later.
Adding some test cases that before would have failed to parse and raised an exception.
Thanks to stypr of Flatt Security for raising this.
front-end: use 'bin' path for node commands instead of '.bin'
license-checker is using relative paths for importing other modules - that worked fine when .bin/license-checker was a symlink, but not on filesystems without symlinks support:
git: make sure _check_url only accept the protocols accepted by is_valid_repo_uri
Avoid unnecessary flexibility, ambiguity, and complexity.
The file protocol was never used. But when cloning existing managed repos, is_valid_repo_url would be skipped and _check_url would be called with absolute paths.
repo_groups: fix select of parent group when adding repo group
h.select was passed a list of repo groups where group_id was integer, but parent_group in the request was a string - thus no match.
Do as in repos controller create_repository (and in error handling): leave it to htmlfill to patch up the generated HTML using defaults ... but make sure we always have a default.
repos: extra HTML escaping of repo and repo group names shown in DataTables
These names will already have been "slugged" and can thus not contain anything that can be used for any attack. But let's be explicitly safe and escape them anyway.
raw_name without escaping would cause XSS *if* it was possible to create unsafe repo names.
just_name must be escaped in order to make search work correctly - for example if searching for '<' ... *if* it was possible for names to contain that.
model: remove unused 'subject' parameter of NotificationModel.create()
The subject of emails is determined with EmailNotificationModel._subj_map, based on the notification type. The 'subject' parameter passed to NotificationModel.create is completely unused.
Remove this parameter and update its callers, removing code that is now no longer used.
model/comment: extract notification-related code into a separate method
Preparation for grouping with _get_notification_data next, and keeping clear separation between creating the comment itself, and creating the notification.
Problem introduced in 9a0c41175e66: When iterating the headers dict and setting "msg[key] = value", it wasn't replacing the header but performing add_header so we sometimes ended up with two From headers.
It is also a general problem that while the headers dict only can contain each key once, it can contain entries that only differ in casing and thus will fold to the same message header, making it possible to end up adding duplicate headers.
"msg.replace_header(key, value)" is not a simple solution to the problem: it will raise KeyError if no such previous key exists.
Now, make the problem more clear by explicitly using add_header.
Avoid the duplication problem by deleting the key (no matter which casing) before invoking add_header. Delete promises that "No exception is raised if the named field isn’t present in the headers".
front-end: use 'bin' path for node commands instead of '.bin'
license-checker is using relative paths for importing other modueles - that worked fine when .bin/license-checker was a symlink, but not on filesystems without symlinks support: