Daniel Axtens [Thu, 24 Oct 2019 03:17:04 +0000 (14:17 +1100)]
travis, tox: only do coverage testing in py27
Currently Travis calls `tox -e coverage` unconditionally. However,
the environment has py27 basepython, so all the runs only generate
py27 coverage!
Rather than try to untangle that, just run the coverage when run
in a py27 travis environment. This makes things faster for no
loss of coverage. It means that codecov has nothing to submit for
the py3x environments, but that's no real loss: it would otherwise
submit lots of duplicate data.
We could try to improve coverage by running coverage for 27 and 3x,
but given that 27 is going away, don't stress at this point.
Daniel Axtens [Wed, 23 Oct 2019 22:12:50 +0000 (09:12 +1100)]
travis: fix codecov
In e017f69376da ("travis: run pep8/flake8 tests"), codecov
was removed from the install step, on the basis that tox-travis
would pull it in automatically.
This, it turns out, isn't entirely true: it is pulled in to the
tox environment, but the data is actually sent in an after_success
step. That is outside the tox environment, as it should be - if it
were part of the tox environment, running `tox -e coverage` on a
developer laptop would try to send data to the web. But, as codecov
now isn't present outside the tox environment, we see in the logs:
$ codecov
codecov: command not found
We don't get any reporting of success/failure in the after_success
step, so we didn't notice.
Restore the installation in the travis environment.
Fixes: e017f69376da ("travis: run pep8/flake8 tests") Signed-off-by: Daniel Axtens <dja@axtens.net>
Andrew Donnellan [Wed, 16 Oct 2019 04:23:07 +0000 (15:23 +1100)]
parser: Unmangle From: headers that have been mangled for DMARC purposes
To avoid triggering spam filters due to failed signature validation, many
mailing lists mangle the From header to change the From address to be the
address of the list, typically where the sender's domain has a strict DMARC
policy enabled.
In this case, we should try to unmangle the From header.
Add support for using the X-Original-From or Reply-To headers, as used by
Google Groups and Mailman respectively, to unmangle the From header when
necessary and associate the patch with the correct submitter based on the
unmangled email address.
When downloading mboxes, rewrite the From header using the unmangled
address, and preserve the original header as X-Patchwork-Original-From in
case someone needs it for some reason. The original From header will still
be stored in the database and exposed via the API, as we want to keep
messages as close to the original received format as possible.
Closes: #64 ("Incorrect submitter when using googlegroups") Reported-by: Alexandre Belloni <alexandre.belloni@bootlin.com> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com> Tested-by: Daniel Axtens <dja@axtens.net> # mailman only
[dja: add release note] Signed-off-by: Daniel Axtens <dja@axtens.net>
Jeremy Cline [Wed, 9 Oct 2019 19:03:45 +0000 (15:03 -0400)]
Use secrets and fall back to random.SystemRandom for keys
The random module uses the Mersenne Twister pseudorandom number
generator and is not a cryptographically secure random number
generator[0]. The secrets[1] module is intended for generating
cryptographically strong random numbers, so recommend using that to
generate the secret key. It's new in Python 3, so if it's unavailable
fall back to using the ``os.urandom()`` backed implementation of random.
NOTE(stephenfin): Modified to include change to 'config.yaml'. Also
renamed reno to just stick with hyphens for filenames.
Stephen Finucane [Tue, 15 Oct 2019 12:21:41 +0000 (13:21 +0100)]
docker: Rely on caching
It seems less likely that tox and tox-pyenv will change than our
requirements. Split up the 'RUN' steps so we don't have to reinstall the
former every time the latter change.
Signed-off-by: Stephen Finucane <stephen@that.guru>
The usual sub-resources (mbox, raw) hang off those URLs.
The old style URLs (/patch/NNN/*, /cover/NNN/*) redirect appropriately.
I haven't attempted to do anything meaningful with series, and I
have dropped any attempt to provide a generic message-id lookup
or search functionality. One step at a time.
Our database still stores message ids as with angle brackets; we
just work around that rather than trying to migrate. That too can
come later if we think the pain is justified.
Partially-closes: #106 Reported-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
Reported-by-but-I-don't-want-to-spam: Linus Torvalds <torvalds@linux-foundation.org> Reported-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Sat, 21 Sep 2019 18:06:14 +0000 (19:06 +0100)]
Fix issue with delegation of patch via REST API
There have been reports of people being unable to delegate patches to
themselves, despite being a maintainer or the project to which the patch
is associated.
The issue is a result of how we do a check for whether the user is a
maintainer of the patch's project [1]. This check is checking if a given
'User.id' is in the list of items referenced by
'Project.maintainer_project'. However, 'Project.maintainer_project' is a
backref to 'UserProfile.maintainer_projects'. This means we're comparing
'User.id' and 'UserProfile.id'. Boo.
This wasn't seen in testing since we've had a post-save callback [2] for some
time that ensures we always create a 'UserProfile' object whenever we create a
'User' object. This also means we won't have an issue on deployments initially
deployed after that post-save callback was added, a 'User' with id=N will
always have a corresponding 'UserProfile' with id=N. However, that's not true
for older deployments such as the ozlabs.org one.
Daniel Axtens [Wed, 18 Sep 2019 06:17:28 +0000 (16:17 +1000)]
parsearchive, mail: use repr() to get a human readable exception
Currently if we have particular types of error in mail parsing
in parsearchive or parsemail, we print exc.message, which doesn't
always work:
Traceback (most recent call last):
File ".../patchwork/management/commands/parsearchive.py", line 90, in handle
obj = parse_mail(msg, options['list_id'])
File ".../patchwork/parser.py", line 961, in parse_mail
raise ValueError("Missing 'Message-Id' header")
ValueError: Missing 'Message-Id' header
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "manage.py", line 11, in <module>
execute_from_command_line(sys.argv)
File ".../django/core/management/__init__.py", line 381, in execute_from_command_line
utility.execute()
File ".../django/core/management/__init__.py", line 375, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File ".../django/core/management/base.py", line 323, in run_from_argv
self.execute(*args, **cmd_options)
File ".../django/core/management/base.py", line 364, in execute
output = self.handle(*args, **options)
File ".../patchwork/management/commands/parsearchive.py", line 100, in handle
logger.warning('Invalid mail: %s', exc.message)
AttributeError: 'ValueError' object has no attribute 'message'
repr(exc) will work. Use it.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
In commit ab0c443691, we switched from using commit ranges to fixed
ranges. This was a good idea in so far as it ensures we're providing an
application with dependencies that are guaranteed to work. However,
Patchwork as a project isn't active enough to warrant the continued busy
work effort necessary to keep bumping these versions and it's probably
about time to abandon the experiment. However, rather than switching
back to version ranges, use the compatible releases feature introduced
in PEP 440 [1]. This gives us most of the benefits of ranges but with a
nicer syntax.
This is slightly slower to initially configure but requires less hacking
to get the same environment and should be a lot more maintainable (just
a simple modification to change the Python version).
Signed-off-by: Stephen Finucane <stephen@that.guru>
This is failing since the update to Xenial [1] with the following
warning:
$ sudo -u postgres psql -c "grant ALL on DATABASE postgres to travis WITH GRANT OPTION;"
could not change directory to "/home/travis/build/getpatchwork/patchwork": Permission denied
psql: could not connect to server: No such file or directory
Is the server running locally and accepting
connections on Unix domain socket "/var/run/postgresql/.s.PGSQL.5433"?
Bump the distro version to xenial, as this is the first version to
include Python 3.7 support. Bionic is also available but it doesn't
support Python 3.5 [1].
In addition, skip a test that was valid on Python 3.4 - 3.6 but does not
appear to be an issue for Python 3.7.
Ali Alnubani [Wed, 4 Sep 2019 13:40:31 +0000 (13:40 +0000)]
sql: Fix table lists
The patch adds missing commas in the table lists where missing, and
removes where unnecessary. This fixes errors such as the following when
feeding the script to psql:
psql:lib/sql/grant-all.postgres.sql:37: ERROR: syntax error at or near "patchwork_emailconfirmation"
LINE 19: patchwork_emailconfirmation,
...
Signed-off-by: Ali Alnubani <alialnu@mellanox.com> Reviewed-by: Stephen Finucane <stephen@that.guru> Fixes: ca0e79d4db34 ("sql: Sort 'grant-all' scripts alphabetically")
migrations: Correct 'unique_together' order in '0015'
This was resulting in exceptions like the following when used with MySQL
8.0:
Traceback (most recent call last):
File "../patchwork/manage.py", line 11, in <module>
execute_from_command_line(sys.argv)
...
File "../.tox/py27-django111/lib/python2.7/site-packages/django/db/backends/mysql/schema.py", line 88, in _delete_composed_index
return super(DatabaseSchemaEditor, self)._delete_composed_index(model, fields, *args)
File "../.tox/py27-django111/lib/python2.7/site-packages/django/db/backends/base/schema.py", line 394, in _delete_composed_index
", ".join(columns),
ValueError: Found wrong number (0) of constraints for patchwork_seriespatch(series_id, number)
This error was being raised by the following lines in the 0033
migration:
It appears that this is because of a mismatch between the order of
fields in a 'unique_together' constraint [1]. Correct the order in the
original migration and see the issue disappear.
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: d67d859f40f ("models: Add 'Series' model")
As was designed, starting the interpreter would cause the State model
and its entries to be evaluated. This was an issue if, for example, the
model had been modified and you were attempting to apply the migration.
Traceback (most recent call last):
File "manage.py", line 11, in <module>
execute_from_command_line(sys.argv)
...
File "/usr/local/lib/python2.7/dist-packages/django/forms/models.py", line 1199, in _set_queryset
self.widget.choices = self.choices
File "/home/patchwork/patchwork/patchwork/forms.py", line 157, in _get_choices
super(OptionalModelChoiceField, self)._get_choices())
File "/usr/local/lib/python2.7/dist-packages/django/forms/models.py", line 1143, in __len__
return (len(self.queryset) + (1 if self.field.empty_label is not None else 0))
File "/usr/local/lib/python2.7/dist-packages/django/db/models/query.py", line 232, in __len__
self._fetch_all()
File "/usr/local/lib/python2.7/dist-packages/django/db/models/query.py", line 1118, in _fetch_all
self._result_cache = list(self._iterable_class(self))
File "/usr/local/lib/python2.7/dist-packages/django/db/models/query.py", line 53, in __iter__
results = compiler.execute_sql(chunked_fetch=self.chunked_fetch)
File "/usr/local/lib/python2.7/dist-packages/django/db/models/sql/compiler.py", line 899, in execute_sql
raise original_exception
django.db.utils.OperationalError: (1054, "Unknown column 'patchwork_state.slug' in 'field list'")
Resolve this by moving the evaluation into '__init__', meaning it will
only occur when a new form is created.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Michael Ellerman [Tue, 27 Aug 2019 06:13:13 +0000 (16:13 +1000)]
models: Add commit_url_format to Project
Add a new field to Project, commit_url_format, which specifies a
format string that can be used to generate a link to a particular
commit for a project.
This is used in the display of a patch, to render the patch's commit
as a clickable link back to the commit on the SCM website.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Wed, 21 Aug 2019 05:15:44 +0000 (15:15 +1000)]
mbox: do not copy Content-Type into exported mbox
Daniel reports a patch + comment combination that breaks in
git am. The patch reports a Content-Type of US-ASCII, while
the comment adds a Ack with UTF-8 characters. The exported
mbox contains both the original Content-Type, and a UTF-8
Content-Type that we set. However, because the US-ASCII one
occurs later, git am honours it instead of ours, and chokes
on the UTF-8 characters.
Strip out any subsequent Content-Type:s. We normalise things
to UTF-8 and should not allow it to be overridden.
Add a test for this, based on the original report.
Reported-by: Daniel Borkmann <daniel@iogearbox.net> Signed-off-by: Daniel Axtens <dja@axtens.net>
docker: *Actually* don't require rebuilding if unnecessary
Because we were using 'set -e', we were erroring out as soon as
something - a diff in this case - failed. Temporarily disable it for
this one check.
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 0b5b4e8c ("docker: Don't require rebuilding if unnecessary") Cc: Daniel Axtens <dja@axtens.net> Signed-off-by: Daniel Axtens <dja@axtens.net>
Mete Polat [Mon, 15 Jul 2019 14:50:28 +0000 (16:50 +0200)]
Add 'dumparchive' management command
Introduces a new management command which can export all patches in a
project as one mbox file. Export of multiple projects is supported.
Additionally allows to compress the output.
Signed-off-by: Mete Polat <metepolat2000@gmail.com> Signed-off-by: Stephen Finucane <stephen@that.guru>
- Remove some newlines between terms and definitions that were causing
the latter to be rendered as blockquotes instead
- Order list of settings alphabetically
- Update URLs to use latest version of Django we support
Signed-off-by: Stephen Finucane <stephen@that.guru>
templatetags: Do not mark output of msgid tag as safe
The msgid template tag exists to remove angle brackets from either side of
the Message-ID header.
It also marks its output as safe, meaning it does not get autoescaped by
Django templating.
Its output is not safe. A maliciously crafted email can include HTML tags
inside the Message-ID header, and as long as the angle brackets are not at
the start and end of the header, we will quite happily render them.
Rather than using mark_safe(), use escape() to explicitly escape the
Message-ID.
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Sun, 21 Oct 2018 11:05:58 +0000 (12:05 +0100)]
Remove pwclient
Let's start managing this via a separate project, which will allow the
client to evolve separately from the server. No redirect is added for
the old '/pwclient' URL as it seems wiser to return a HTTP 404 error
code.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Commit 753e4572d updated the parser to consider additional header lines
when deciding where a patch message ends and the diff begins. However,
these additional lines were not captured meaning these patches didn't
have a diff associated with them and they therefore weren't patches in
the Patchwork sense of the term. Correct this and add a test.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Petr Vorel [Sun, 5 May 2019 20:20:05 +0000 (22:20 +0200)]
parser: Add missing extended header lines
Patchwork didn't recognise some patches due missing some extended header
lines (e.g. "old mode" and "new mode" for renaming file mode, see [1]).
Thus adding all modes from git doc [2].
Stephen Finucane [Tue, 14 May 2019 14:56:53 +0000 (15:56 +0100)]
docs: Document backport criteria
Explain why we don't want to be in the business of backport certain
patches, in the long run. It took me a while to put this into words but
I was helped by a similar discussion ongoing in the OpenStack community
at the moment [1].
In July 2018, we received a report of OzLabs patchwork mangling
emails that have subjects containing words with internal commas,
like "Insert DT binding for foo,bar" (#197).
Stephen took a look and came up with the comment this reverts. Quoting
the commit message:
RFC2822 states that long headers can be wrapped using CRLF followed by
WSP [1]. For example:
Subject: Foo bar,
baz
Should be parsed as:
Foo bar,baz
As it turns out, this is not the case. Journey with me to
section 2.2.3 of RFC 2822:
2.2.3. Long Header Fields
Each header field is logically a single line of characters comprising
the field name, the colon, and the field body. For convenience
however, and to deal with the 998/78 character limitations per line,
the field body portion of a header field can be split into a multiple
line representation; this is called "folding". The general rule is
that wherever this standard allows for folding white space (not
simply WSP characters), a CRLF may be inserted before any WSP. For
example, the header field:
Subject: This is a test
can be represented as:
Subject: This
is a test
So the issue with the example in the reverted commit is that there is no
folding white space in "bar,baz", so it's not valid to split it.
These are valid:
Subject: Foo bar,baz
Subject: Foo
bar,baz
but splitting "bar,baz" into "bar,\n baz" is not valid.
What then is correct unfolding behaviour? Quoting the RFC again:
The process of moving from this folded multiple-line representation
of a header field to its single line representation is called
"unfolding". Unfolding is accomplished by simply removing any CRLF
that is immediately followed by WSP. Each header field should be
treated in its unfolded form for further syntactic and semantic
evaluation.
In other words, the unfolding rule requires you to strip the CRLF, but
it does not permit you to strip the WSP. Indeed, if "bar,\n baz" is
received, the correct unfolding is "bar, baz".
If you do strip the WSP, you end up mashing words together, such as in
https://patchwork.ozlabs.org/patch/1097852/
So revert the commit, restoring original behaviour, but keep a corrected
version of the test.
This presents a big question though: how did Rob's email up with a
mangled subject line?
To answer this question, you end up having to learn about OzLabs
Patchwork and how it differs from Patchwork the project.
OzLabs Patchwork (patchwork.ozlabs.org) is an installation of Patchwork.
Part of what makes it so useful for so many projects is a little
intervening layer that can massage some mail to make it end up in the
right project. Email that lands in the device tree project is an example
of email that goes through this process. I only learned about this
today and I haven't looked in any detail at precisely what is done to
the mail. The script is not part of the Patchwork project.
This intervening filter is a Python script that runs - and this is an
important detail - in Python 2.7.
Ignoring all the details, the filter basically operates in a pipe
between the mail program and patchwork's parsemail, like
(mail from system) | filter.py | parsemail
At it's very simplest, filter.py acts as follows:
import email
import sys
mail = email.parse_from_file(sys.stdin)
sys.stdout.write(mail.as_string())
Fascinatingly, if you take Rob's email from #197 and put it through this
process, you can see that it is getting mangled:
You can see that python27 has incorrectly wrapped the header, breaking
where there is not a foldable space. Python3 does not have this issue.
To summarise:
- part of the magic of OzLabs PW is a filter to make sure mail gets to
the right place. This isn't part of the Patchwork project and so is
usually invisible to patchwork developers.
- the filter is written in python27. The email module in py27 has a bug
that incorrectly breaks subjects around commas within words.
- patchwork correctly unfolds those broken subjects with a space after
the comma.
- the extra space was interpreted as a bug in patchwork, leading to a
misinterpretation of the spec to strip out the whitespace that was
believed to be in error.
- that broke other wrapped subjects.
To solve this, revert the commit and I'll work with jk to get the filter
script into py3 compatibility. (Given that py27 sunsets in ~7mo, trying
to fix it is not worth it.)
Closes: #273 Signed-off-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Stephen Finucane <stephen@that.guru>
[stephenfin: Use a new release note instead of editing the original one]
Russell Currey [Wed, 1 May 2019 06:27:18 +0000 (16:27 +1000)]
docs: Mention Postgres for Docker development install
Might as well since it's there, and it gives some clue to anyone trying
to use Docker on non-x86. I figured it was best to leave this out of
the README since it's incredibly niche.
Signed-off-by: Russell Currey <ruscur@russell.cc> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Russell Currey [Wed, 1 May 2019 06:27:17 +0000 (16:27 +1000)]
docker: Use Ubuntu ports repositories on non-x86 architectures
This should allow Patchwork to run "out of the box" in Docker on any
architecture with a) an Ubuntu port and b) support in the Postgres
multiarch Docker image, which includes at least arm64 and ppc64le.
It's a little gross hacking the Dockerfile like this, but I'm not sure
there's a more elegant way to do it. Unfortunately it doesn't seem like
there's any way to do conditional COPY, and anything in RUN is plain
/bin/sh, so that's why it looks like it does.
Tested on ppc64le and on x86_64.
Signed-off-by: Russell Currey <ruscur@russell.cc> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Russell Currey [Wed, 1 May 2019 04:35:53 +0000 (14:35 +1000)]
docker: Install libpq-dev to fix psycopg2-binary build
psycopg2-binary fails if pg_config isn't installed, which is provided by
libpq-dev.
This seems strange to me since psycopg2-binary suggests that
you use psycopg2-binary instead (of itself) if you don't want to build
psycopg2 so you wouldn't need pg_config, which is very confusing.
It's possible that psycopg2-binary only needs to compile itself on
non-x86 platforms, since I hit this on ppc64le.
Anyway, it works when this is added.
Signed-off-by: Russell Currey <ruscur@russell.cc> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Russell Currey [Wed, 1 May 2019 03:16:43 +0000 (13:16 +1000)]
README: add .env file to installation instructions
Creating the .env file is mentioned in the installation documentation
but not in the README, so following only the steps mentioned there will
fail. Add this and add a `cd patchwork` in there for good measure so
you could straight up copy paste the steps.
Signed-off-by: Russell Currey <ruscur@russell.cc> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Mon, 29 Apr 2019 15:33:16 +0000 (01:33 +1000)]
REST: Handle regular form data requests for checks
08d1459a4a40 ("Add REST API validation using OpenAPI schema") moved
all API requests to JSON blobs rather than form data.
dc48fbce99ef ("REST: Handle JSON requests") attempted to change the
check serialiser to handle this. However, because both a JSON dict
and a QueryDict satisfy isinstance(data, dict), everything was handled
as JSON and the old style requests were broken.
Found in the process of debugging issues from the OzLabs PW & Snowpatch
crew - I'm not sure if they actually hit this one, but kudos to them
anyway as we wouldn't have found it without them.
Fixes: dc48fbce99ef ("REST: Handle JSON requests") Signed-off-by: Daniel Axtens <dja@axtens.net>
Jeremy Kerr [Sat, 27 Apr 2019 11:12:16 +0000 (19:12 +0800)]
notifications: fix notification expiry when no user is associated
It's possible that an EmailConfirmation object will have no associated
user (eg, for email opt-out, which does not require a user object). In
this case, we will see a NULL value for EmailConfirmation.user_id.
However, having a NULL value appear in a SQL 'IN' clause will match
every value. This means that once one of these null-user
EmailConfirmations is present, we will never expire any non-active user
accounts.
This change adds a filter for a valid user_id when we query for active
EmailConfirmation objects. This means we'll have a valid values set to
use in the pending_confs set.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
[dja: fix pep8 issue] Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Fri, 15 Mar 2019 06:27:40 +0000 (17:27 +1100)]
Fix YAML loader warning
In my tests I'm seeing:
/home/patchwork/patchwork/patchwork/tests/api/validator.py:229:
YAMLLoadWarning: calling yaml.load() without Loader=... is deprecated,
as the default Loader is unsafe. Please read https://msg.pyyaml.org/load
for full details.
Fix this by using the safe loader in the tests.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
projects / thirdparty / patchwork.git / log
