controllers: move controllers base class from lib/base to controllers

TG quickstart put it in lib/base.py , but it fits better on the controllers
layer as a base there.

The contributing docs were a bit ahead of time ... but with a typo.

lib: consistently import helpers the same way

Make it easier to grep for any remaining potential layering-violating use of
helpers.

lib: move webhelpers2 and friends to webutils

Gives less of the unfortunate use of helpers - especially in low level libs.

lib: consistently use webutils.url instead of h.url

Templates are still the valid use case for accessing through h.

Simplify mocking of url() function.

diffs: remove unused argument enable_comments and class no-comment

enable_comments was only used to set/not-set the 'no-comment' CSS class.
While this class was emitted, no CSS rule nor any JavaScript logic was
actually using it. Last real usage of that class was removed with commit
e87baa8f1c5bd2488aefc23b95c0db3a04bc8431.

Cleanup the code by not emitting 'no-comment' and remove the
'enable_comments' flag.

lib: fix bad references to utils3

A problem introduced in when rebasing 5e46f73f0d1c after renaming the temporary
utils3 name to webutils.

routing: separate url handling from routing - move it to webutils

This is a helper method relying on the thread local tg.request. We didn't have
a good place to put it. Now we do.

This (re)moves unfortunate dependencies to the routing module (which almost is
a controller).

vcs: move changeset diff from controller to vcs

Remove unfortunate model dependency on controller ... and put the VCS details
where they belong, with less VCS specific knowledge in the controllers.

isort: upgrade to 5.1.2

The changes to non-top-level imports seem nice.

diff: drop per file ignore-whitespace and context - it didn't work and had conceptual issue (Issue #344)

Diffs are currently generated at the low level as one big diff between two vcs
resisions, provided global values for diff context size and flag for ignoring
whitespace. All files use the same flags. There is no way to actually compute
the full diff using these use per file flags, and no simple and efficient way
to add it.

The best option is thus to drop the failed attempt at making it per file, and
just rely on the simple global flags in the URL.

The links for changing whitespace and context is sometimes shown for the whole
"page", and sometimes next to the diff for one file. For now, keep showing the
link in these places, but make sure it navigates back to the FID of the section
where the link was clicked.

The implementation is completely rewritten and moved to a more appropriate
location in helpers.

With a more clean implementation, we also consistently use the simple getters
to extract values from the URL.

vcs: drop the superfluous and leaky hgcompat "layer"

Explicit is better. And gives less pyflakes noise.

py3: add b'' annotations in some places where they will be needed later

Mostly entirely trivial adding of b prefix that is a ignored for py2 ... and
also a bit of related trivial reformatting/refactorings.

vcs: always return bytes from node.content

We will rather have the unicode conversions explicit.

Note: Py3 bytes doesn't have .startswith - replace that with a regexp.

vcs: refactor run_git_command to just return stdout as unicode string

This will be convenient for py3.

hg: bump minimum version to 5.1

We will soon move to Python 3 which only will support 5.1 or later.

Remove old hacks and tech debt.

Also avoids future warning:
DeprecationWarning: inspect.getargspec() is deprecated since Python 3.0, use inspect.signature() or inspect.getfullargspec()

auth: restore anonymous repository access

Dominik Ruf found that aa25ef34ebab introduced a regression in anonymous access
to repositories ... if that is enabled.

The refactoring was too strict when it missed that not all repo permission
checks require a logged in user. Read access can be granted to the default user
... but not write or admin.

Instead of the commands used in aa25ef34ebab, the following commands are used
to consistently also allow the default user in all decorators where we only need
repo read access:

# Introduce explicit allow_default_user=True - that was the default before aa25ef34ebab
sed -i 's/@LoginRequired()/@LoginRequired(allow_default_user=True)/g' `hg mani`
sed -i 's/@LoginRequired(\(..*\))/@LoginRequired(\1, allow_default_user=True)/g' `hg mani`
# The primary case: Replace @NotAnonymous with removal of allow_default_user=True
perl -0pi -e 's/\@LoginRequired\((?:(.*), )?allow_default_user=True\)\n\s*\@NotAnonymous\(\)/\@LoginRequired(\1)/g' `hg mani`
# If there is a global permission check, no anonymous is ever allowed
perl -0pi -e 's/\@LoginRequired\(allow_default_user=True\)(\n\s*\@HasPermission)/\@LoginRequired()\1/g' `hg mani`
# Repo access for write or admin also assume no default user
perl -0pi -e 's/\@LoginRequired\(allow_default_user=True\)(\n\s*\@HasRepoPermissionLevelDecorator\('"'(write|admin)'"'\))/\@LoginRequired()\1/g' `hg mani`

diffs: wrap vcs repo get_diff

Refactor to get a single place for diff error handling outside the vcs lib.

diffs: move as_html and _safe_id from method to a pure function - avoid calling the method as function

as_html was sometimes used in a way where we actually don't want to use the
whole DiffProcessor - we just created a dummy instance and passed custom input
as parameter to the instance method.

Instead, make it the function we apparently want.

Make it clear that as_html not just returns a "diff" but that it is a html diff.

diffs: drop the DiffLimitExceeded container - just make it a flag available as property

Keep it simple.

diffs: inline prepare() into __init__ and make the result available as .parsed

Make it more clear what the DiffProcessor is: Something that works on a raw
diff as input, mainly compute when initialized, and returns an object where the
result is available in different ways.

diffs: cleanup of variable naming around cut_off_limit

A brief summary of this area:

The base controller sets self.cut_off_limit from config and is used for diffs,
unless controllers are given a fulldiff query parameter. In a few cases, these
are passed to templates as c.cut_off_limit or c.fulldiff . Also, if the diff
function returns a LimitedDiffContainer, c.limited_diff is set so the UI can
report the data set is partial.

codingstyle: trivial whitespace fixes

Reported by flake8.

controllers: consistently use c.cs_comments and cs_statuses

c.comments and c.statuses were also used for lists of comments and statuses. To
get more consistency and avoid confusion and conflicts, use different for names
for mappings from changeset hashes.

controllers: rename __before__ to _before in preparation of TurboGears2

__before__ in Pylons is called _before in TurboGears2. We can prepare this
rename already in Pylons-based Kallithea, so that the real TG2 migration
commit just changes the BaseController.

Since TurboGears2 _before can pass extra arguments, we add *args and
**kwargs parameters as well.

tg: minimize future diff by some mocking and replacing some pylons imports with tg

No actual tg dependency yet, just a temporary hack faking tg as an alias for
pylons.

Based on work by Alessandro Molina.

compare: refactor drop-down UI code

Don't rely on fake ref_names in the controller; instead put the code in
the HTML together with the other UI code.

pullrequests: rename "as_form" to something more descriptive

This parameter is (only) used for showing the PR contents preview on the
"New Pull Request" page.

templates: properly escape inline JavaScript values

TLDR: Kallithea has issues with escaping values for use in inline JS.
Despite judicious poking of the code, no actual security vulnerabilities
have been found, just lots of corner-case bugs. This patch fixes those,
and hardens the code against actual security issues.

The long version:

To embed a Python value (typically a 'unicode' plain-text value) in a
larger file, it must be escaped in a context specific manner. Example:

>>> s = u'<script>alert("It\'s a trap!");</script>'

1) Escaped for insertion into HTML element context

>>> print cgi.escape(s)
&lt;script&gt;alert("It's a trap!");&lt;/script&gt;

2) Escaped for insertion into HTML element or attribute context

>>> print h.escape(s)
&lt;script&gt;alert(&#34;It&#39;s a trap!&#34;);&lt;/script&gt;

This is the default Mako escaping, as usually used by Kallithea.

3) Encoded as JSON

>>> print json.dumps(s)
"<script>alert(\"It's a trap!\");</script>"

4) Escaped for insertion into a JavaScript file

>>> print '(' + json.dumps(s) + ')'
("<script>alert(\"It's a trap!\");</script>")

The parentheses are not actually required for strings, but may be needed
to avoid syntax errors if the value is a number or dict (object).

5) Escaped for insertion into a HTML inline <script> element

>>> print h.js(s)
("\x3cscript\x3ealert(\"It's a trap!\");\x3c/script\x3e")

Here, we need to combine JS and HTML escaping, further complicated by
the fact that "<script>" tag contents can either be parsed in XHTML mode
(in which case '<', '>' and '&' must additionally be XML escaped) or
HTML mode (in which case '</script>' must be escaped, but not using HTML
escaping, which is not available in HTML "<script>" tags). Therefore,
the XML special characters (which can only occur in string literals) are
escaped using JavaScript string literal escape sequences.

(This, incidentally, is why modern web security best practices ban all
use of inline JavaScript...)

Unsurprisingly, Kallithea does not do (5) correctly. In most cases,
Kallithea might slap a pair of single quotes around the HTML escaped
Python value. A typical benign example:

$('#child_link').html('${_('No revisions')}');

This works in English, but if a localized version of the string contains
an apostrophe, the result will be broken JavaScript. In the more severe
cases, where the text is user controllable, it leaves the door open to
injections. In this example, the script inserts the string as HTML, so
Mako's implicit HTML escaping makes sense; but in many other cases, HTML
escaping is actually an error, because the value is not used by the
script in an HTML context.

The good news is that the HTML escaping thwarts attempts at XSS, since
it's impossible to inject syntactically valid JavaScript of any useful
complexity. It does allow JavaScript errors and gibberish to appear on
the page, though.

In these cases, the escaping has been fixed to use either the new 'h.js'
helper, which does JavaScript escaping (but not HTML escaping), OR the
new 'h.jshtml' helper (which does both), in those cases where it was
unclear if the value might be used (by the script) in an HTML context.
Some of these can probably be "relaxed" from h.jshtml to h.js later, but
for now, using h.jshtml fixes escaping and doesn't introduce new errors.

In a few places, Kallithea JSON encodes values in the controller, then
inserts the JSON (without any further escaping) into <script> tags. This
is also wrong, and carries actual risk of XSS vulnerabilities. However,
in all cases, security vulnerabilities were narrowly avoided due to other
filtering in Kallithea. (E.g. many special characters are banned from
appearing in usernames.) In these cases, the escaping has been fixed
and moved to the template, making it immediately visible that proper
escaping has been performed.

Mini-FAQ (frequently anticipated questions):

Q: Why do everything in one big, hard to review patch?
Q: Why add escaping in specific case FOO, it doesn't seem needed?

Because the goal here is to have "escape everywhere" as the default
policy, rather than identifying individual bugs and fixing them one
by one by adding escaping where needed. As such, this patch surely
introduces a lot of needless escaping. This is no different from
how Mako/Pylons HTML escape everything by default, even when not
needed: it's errs on the side of needless work, to prevent erring
on the side of skipping required (and security critical) work.

As for reviewability, the most important thing to notice is not where
escaping has been introduced, but any places where it might have been
missed (or where h.jshtml is needed, but h.js is used).

Q: The added escaping is kinda verbose/ugly.

That is not a question, but yes, I agree. Hopefully it'll encourage us
to move away from inline JavaScript altogether. That's a significantly
larger job, though; with luck this patch will keep us safe and secure
until such a time as we can implement the real fix.

Q: Why not use Mako filter syntax ("${val|h.js}")?

Because of long-standing Mako bug #140, preventing use of 'h' in
filters.

Q: Why not work around bug #140, or even use straight "${val|js}"?

Because Mako still applies the default h.escape filter before the
explicitly specified filters.

Q: Where do we go from here?

Longer term, we should stop doing variable expansions in script blocks,
and instead pass data to JS via e.g. data attributes, or asynchronously
using AJAX calls. Once we've done that, we can remove inline JavaScript
altogether in favor of separate script files, and set a strict Content
Security Policy explicitly blocking inline scripting, and thus also the
most common kind of cross-site scripting attack.

cleanup: use list comprehensions

It's often the same number of lines, but avoids introducing a needless
"result" variable, and makes the item expression stand out more clearly.

It's also a tiny bit faster, but the readability gains is what matters.

auth: simplify repository permission checks

In practice, Kallithea has the 'repository.admin' permission imply the
'repository.write' permission, which again implies 'repository.read'.

This codifies/enforces this practice by replacing HasRepoPermissionAny
"perm function" with the new HasRepositoryLevel function, reducing the
risk of errors and saving quite a lot of typing.

compare: remove old Dulwich hack

We no longer support Dulwich 0.9.9, so don't need the workaround.

Turbogears2 migration: replace pylons.url by kallithea.config.routing.url

In preparation for the migration to Turbogears2, introduce a
kallithea.config.routing.url to replace pylons.url. The implementation is
basically the same: wrap around routes.url().

This change involves:
- a number of import statement changes
- fixing some tests in test_libs.py; to avoid duplication, the different
implementations of fake_url were grouped in one place.

This change was first proposed by Alessandro Molina in his initial
port. Following changes were made afterwards:
- move UrlGenerator from kallithea.lib.utils to kallithea.config.routing
- add documentation to UrlGenerator
- kallithea/lib/auth.py used url_for instead of url, for no apparent reason
so this was changed.
- fix libs tests
- rebase onto Pylons-based Kallithea first

Turbogears2 migration: remove some references to Pylons in comments

In order to minimize the diff of the actual Turbogears2 migration, this
commit already removes certain unnecessary references to Pylons from the
Kallithea source base.
Places where the reference to Pylons is important are still kept for now, as
well as references in kallithea/config where many changes are made for
Turbogears2 anyway.

diff: correct handling of links to old filename in renames

There were links to the file at the parent revision ... but if the file had
been renamed, it used the wrong name.

diffs: fold diff_block_simple (used by PR and compare) into diff_block

Change to using the same datamodel and enjoy the reduced amount of code
duplication.

diff: minor cleanups

More consistency and preparing for later changes.

compare: ensure that repositories exist before proceeding

The index method on CompareController did not verify that other_repo
existed, causing a rendering error if it wasn't.

Since neither controller method can proceed if either repository is
non-existent, check existence and load Repository objects in __before__.
Also perform type compatibility check up front while we're at it, remove
redundant repository database lookups, and enable error message i18n.

diff: make sure context parameter is an integer

Prevent Abort in mdiff on malformed URLs.

tests: fix Git on Windows sometimes failing on ' or ` in file:/// URLs

Some Git versions will fail when cloning a repository like:

git clone file:///path/to/strange-file-'`foobar myclone

Since '` (and some other characters) are filtered anyway (in
lib.utils.repo_name_slug) I think this test doesn't need to test those
characters.

compare: close Dulwich internals for Git repositories to avoid leaking open files

Fix failing test_compare.py on Windows by explicitly closing internal
object_store objects.

cleanup: replace redirect with WebOb exceptions

All redirect does is to log "Generating 302 redirect" with logging the actual
location and raise a WebOb HTTPFound exception, and the logging is redundant,
as WebOb exceptions and their status codes are already logged.

Instead, just raise the exception directly, which is both explicit and simpler
(and finally, gets rid of "return redirect" which never really returns).

git: add unicode conversion to make tests pass with dulwich 0.11.2

This makes the test suite pass with both dulwich 0.9.9 and the unsupported
0.11.2.

compare: ignore whitespace around revision string

When manually entering a revision ID, it's possible for spurious
whitespace to occur around it; this should not cause a 404.

cleanup: remove unused imports

Found with pyflakes.

compare: backout 51c4d2e74898 to fix ancestor calculation to empty repository

This case can not just be simplified as I thought it could.

Mercurial 3.4 has
d2de20e1451f 'revset: extend fullreposet to make "null" revision magically appears in set'
Which makes
ancestor(id(0000000000),1)
'correctly' return the null revision as ancestor.

We can however not rely on that when supporting Mercurial 3.3.

git: run external commands as list of strings so we really get correct quoting (Issue #135)

a6dfd14d4b20 from
https://bitbucket.org/conservancy/kallithea/pull-request/17/add-quotes-to-repo-urls-for-git-backend
fixed that issue but did not make it "safe". The vcs git backend still used
command strings but tried to quote them correctly ... but that approach is
almost impossible to get right.

Instead, pass a string list all the way to the subprocess module and let it do
the quoting. This also makes some of the code more simple.

hg: adapt for changes in 3.2 and 3.3

Mercurial 3.2:

- moved localrepo.pull to exchange.pull
- removed __getitem__ from lazysets

compare: workaround unexpected Mercurial behaviour when finding ancestor of null rev

ancestor(id(0000000000000000000000000000000000000000),7) == 7
- that caused weird pull request diffs and lots of grief when (accidentally)
creating a pull request from the null revision.

Add special handling for the case of null revisions.

diff: consistently use a=base=ancestor and b=changes in compare and PR

Some additional fixes to the previous change.

diff: rename template values for org and other for compare and PR

sed -i -e 's,\<\(c\.\(default_\)\?\)org_,\1a_,g' -e 's,\<\(c\.\(default_\)\?\)other_,\1cs_,g' kallithea/controllers/compare.py kallithea/templates/compare/compare_cs.html kallithea/templates/compare/compare_diff.html
sed -i -e 's,\<\(c\.\(default_\)\?\)org_,\1cs_,g' -e 's,\<\(c\.\(default_\)\?\)other_,\1a_,g' kallithea/controllers/pullrequests.py kallithea/templates/pullrequests/pullrequest.html kallithea/templates/pullrequests/pullrequest_show.html kallithea/templates/changeset/diff_block.html

Renaming it differently for compare and PR finally fixes some issues with diffs
and links pointing at wrong revisions and repos - no more whac-a-mole.

pull requests: fix same git repository pull-request issue

Currentry, Kallithea can not create pull-request from same git repository.
Because, ancestor variable is not initialized when create pull-request from same git repository.

This patch find ancestor by `git merge-base` command and initialize it.

compare: introduce .cs_repo as the repo for .cs_changes - sometimes it is org, sometimes other

Influences pull request creation, display and compare.

Correct licensing information in individual files.

The top-level license file is now LICENSE.md.

Also, in various places where there should have been joint copyright holders
listed, a single copyright holder was listed. It does not appear easy to add
a link to a large list of copyright holders in these places, so it simply
refers to the fact that various authors hold copyright.

In future, if an easy method is discovered to link to a list from those
places, we should do so.

Finally, text is added to LICENSE.md to point to where the full list of
copyright holders is, and that Kallithea as a whole is GPLv3'd.

Second step in two-part process to rename directories.
This is the actual directory rename.