Daniel Axtens [Fri, 5 Jul 2019 05:21:26 +0000 (15:21 +1000)]
docs: Add a release note for CVE-2019-13122
Signed-off-by: Daniel Axtens <dja@axtens.net>
(cherry picked from commit f48179f6368982fdeb7f2dfb515f1972d86b0991) Signed-off-by: Daniel Axtens <dja@axtens.net>
filters: Escape State names when generating selector HTML
States with names containing special characters are not correctly escaped
when generating the select list. Use escape() to fix this.
Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
(cherry picked from commit b3fa0c402e060622a5ed539a465d2fa98b1d2e13) Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Fri, 5 Jul 2019 01:30:28 +0000 (11:30 +1000)]
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.
This change required a migration which was not included in 2.0.0. It's
not possible to backport the migration but since this change only hides
a warning in the versions of Django supported here, it's easy to just
revert this.
Signed-off-by: Stephen Finucane <stephen@that.guru> Closes: #192
Stable-Only
This change required a migration which was not included in 2.0.0. It's
not possible to backport the migration but since this change only hides
a warning in the versions of Django supported here, it's easy to just
revert this.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stable-Only
This change required a migration which was not included in 2.0.0. It's
not possible to backport the migration but since this change wasn't
absolutely necessary, it's easy to just revert this.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stable-Only
Stephen Finucane [Wed, 13 Jun 2018 21:53:22 +0000 (22:53 +0100)]
Don't discard values from 'archive' filter
The pagination functionality used in the 'patchwork.view.generic_list'
generates the filter querystring from scratch. To do this, it calls the
'patchwork.filters.Filters.params' function, which in turn calls the
'patchwork.filters.Filter.key' function for each filter. If any of these
'key' functions return None, the relevant filter is not included in the
querystring. This ensures we don't end up with a load of filters like
the below:
There is one exception to this rule, however: ArchiveFilter. This is a
little unusual in that it is active by default, excluding patches that
are "archived" from the list. As a result, the 'key' function should
return None for this active state, not for the disabled state. This has
been the case up until commit d848f046 which falsely equated 'is False'
with 'is None'. This small typo resulted in the filter being ignored
when generating pagination links and essentially broke pagination for
some use cases.
Fix this up. We could probably simplify this thing greatly by not
recalculating filters for pagination at least or, better yet, by using
django-filter here too. That is a change for another day though.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reported-by: John McNamara <john.mcnamara@intel.com> Reported-by: Eli Schwartz <eschwartz93@gmail.com> Fixes: d848f046 ("trivial: Don't shadow built-ins") Closes: #184
(cherry picked from commit 24c4bef32e1df3da59695372c686af58f8846208)
views: Raise 404 if downloading non-existent dependencies
If a patch was processed by Patchwork before series support was added,
it will not have a series associated with it. As a result, it is not
possible to extract the dependencies for that patch from the series and
a 404 should be raised. This was not previously handled correctly.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net> Reported-by: John McNamara <john.mcnamara@intel.com> Fixes: e2dfd490 ("views: Add 'series' parameter to '/mbox' endpoint") Closes: #189
(cherry picked from commit c975e20d3b5ba503df5fa1ea9f9a7490f3f223ae)
Ali Alnubani [Wed, 6 Jun 2018 13:11:54 +0000 (15:11 +0200)]
parsemail.sh: don't set the python version
This is to fix using the wrong python version when inside a virtualenv.
Signed-off-by: Ali Alnubani <alialnu@mellanox.com> Signed-off-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit 951a58e4a6f71b91beb345a6608dc4d9fb231337)
Stephen Finucane [Wed, 30 May 2018 10:57:59 +0000 (11:57 +0100)]
sql: Update 'grant-all.mysql' script with missing tables
These were all introduced in 2.0 and while the postgreSQL script was
fixed in commit 234bc7c3, the MySQL one was not. This suggests either
(a) no one is using this or (b) people are carrying local changes but
for now we just resolve the issues.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net>
(cherry picked from commit 58160097f9571b808302226f02932c1bb0b68c57)
Daniel Axtens [Mon, 19 Feb 2018 14:34:15 +0000 (01:34 +1100)]
parser: don't fail on multiple SeriesReferences
Parallel parsing would occasonally fail with:
patchwork.models.MultipleObjectsReturned: get() returned more than one SeriesReference -- it returned 2!
I think these are happening if you have different processes parsing
e.g. 1/3 and 2/3 simultaneously: both will have a reference to 1/3,
in the case of 1 it will be the msgid, in the case of 2 it will be
in References. So when we come to parse 3/3, .get() finds 2 and
throws the exception.
This does not fix the creation of multiple series references; it
just causes them to be ignored. We still have serious race conditions
with series creation, but I don't yet have clear answers for them.
With this patch, they will at least not stop patches from being
processed - they'll just lead to wonky series, which we already have.
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Sun, 18 Feb 2018 01:57:03 +0000 (12:57 +1100)]
parser: use Patch.objects.create instead of save()
Attempts to do parallel parsing with MySQL threw the following errors:
_mysql_exceptions.OperationalError: (1213, 'Deadlock found when trying to get lock; try restarting transaction')
Looking at the code, it was thrown when we created a patch like this:
patch = Patch(...)
patch.save()
The SQL statements that were being generated were weird:
UPDATE "patchwork_patch" SET ...
INSERT INTO "patchwork_patch" (...) VALUES (...)
As far as I can tell, the update could never work, because it was
trying to update a patch that didn't exist yet. My hypothesis is
that Django somehow didn't quite 'get' that because of the backend
complexity of the Patch model, so it tried to do an update, failed,
and then tried an insert.
Change the code to use Patch.objects.create, which makes the UPDATEs
and the weird MySQL errors go away.
Also move it up a bit earlier in the process so that if things go wrong
later at least we've committed the patch to the db.
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Sun, 18 Feb 2018 08:06:23 +0000 (19:06 +1100)]
parser: avoid an unnecessary UPDATE of Person
Analysis of SQL statements showed that when parsing an email, the row
for the Person who sent the email was always getting updated. This is
because the test for updating it only checks if the incoming mail has
*a* name attached to the email address, and not if it has a new name.
Django is not smart enough to figure that out, and so unconditionally
UPDATEs the model when asked to save.
Give it a hand - only update the model and save it if the new name is
in fact different.
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Sat, 17 Feb 2018 01:54:51 +0000 (12:54 +1100)]
parser: close a TOCTTOU bug on Person creation
find_author looks up a person by email, and if they do not exist,
creates a Person model, which may be saved later if the message
contains something valuable.
Multiple simultaneous processes can race here: both can do the SELECT,
find there is no Person, and create the model. One will succeed in
saving, the other will get an IntegrityError.
Reduce the window by making find_author into get_or_create_author, and
plumb that through. (Remove a test that specifically required find_author
to *not* create).
More importantly, cover the case where we lose the race, by using
get_or_create which handles the race case, catching the IntegrityError
internally and fetching the winning Person model.
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
[dja: post review cleanup of now-unused import] Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Mon, 19 Feb 2018 14:33:59 +0000 (01:33 +1100)]
parser: Handle even more exotically broken headers
An archive of the Ubuntu kernel team mailing list contains a
fascinating email that causes the following parse error:
email.errors.HeaderParseError: header value appears to contain an embedded header:
'4Mf^tnii7k\\_EnR5aobBm6Di[DZ9@AX1wJ"okBdX-UoJ>:SRn]c6DDU"qUIwfs98vF>...
The broken bit seem related to a UTF-8 quoted-printable encoded
section and to be from an internal attempt to break it over multiple
lines: here's a snippet from the error message:
'\n\t=?utf-8?q?Tnf?=\n'
but interesting the header itself does not contain the new lines, so
clearly something quite weird is happening behind the scenes!
This only throws on header.encode(): it actually makes it through
sanitise_header and into find_headers before throwing the assertion.
So, try to encode in sanitize_header as a final step.
Also, fix a hilarious* python bug that this exposes: whitespace-only
headers cause an index error!
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru> Signed-off-by: Daniel Axtens <dja@axtens.net>
After changing submission via admin interface, CRLF newlines are
suddenly present in the body. Replace them back to '\n'.
The issue was found after modifying submission via admin interface
using Python 2 and downloading the respective mbox file (git choked on
downloaded patch because of malformed line endings). Python 3's mail
module uses '\n' internally so the problem doesn't manifest there, but
the content received by Django/JS is still saved in the database with
CRLF line endings which shouldn't be there.
Signed-off-by: Veronika Kabatova <vkabatov@redhat.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit 7f6685a2a0b12c08c39f9de84684de8d033c57f1)
Daniel Axtens [Thu, 25 Jan 2018 01:25:25 +0000 (12:25 +1100)]
tags: be a bit more permissive in what we render to a message
Currently we render a tag from a comment into a message if it is
'^(whatever)-by: .*'
We found a patch that had a UTF-8 non-breaking space after the colon,
and this was breaking the regex. So just remove the requirement for
a space entirely.
Closes: #124 Signed-off-by: Daniel Axtens <dja@axtens.net>
parser: Fix parsing of pull request emails with CRLF line endings on Python 2
When using Python 2, an incoming email that uses CRLF line endings won't
have the line endings converted by the email parser. This means that the
regex to detect pull request emails will fail to match.
Clean up the line endings when we extract the email body to fix this. (On
Python 3, the email parser policy fixes this for us at the initial email
parsing stage.)
Add a test pull request mbox with CRLF line endings to ensure we don't
regress.
Closes: #148 ("Parsing pull request emails broken on Python 2") Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
[dja: fix up CRLF line endings munged by git-send-email
renumber test
re-order lines to put line-ending munging before other processing
verify CRLF in file
skip test if py3 for speed] Signed-off-by: Daniel Axtens <dja@axtens.net>
Daniel Axtens [Thu, 25 Jan 2018 02:43:11 +0000 (13:43 +1100)]
pagination: Fix quirks
There are a couple of pages where the clickable list of pages
would include missing or duplicate pages.
Write a test that ensures:
- you always have a link to the next/prev numbered page
- there are no duplicate page numbers
Fiddle with the pagination algorithm to get it to pass - required
tweaking a display parameter and a couple of comparison operators,
so all pretty minor.
Now, if there are 10 pages, the displayed page numbers for a given
page are as follows:
Closes: #102 Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit bd1613c1d4f915ffd76880afbf4746521f28e831)
Stephen Finucane [Wed, 17 Jan 2018 19:39:36 +0000 (19:39 +0000)]
parser: Handle 'git-request-pull' mails from Git 2.14.3
Make the regex case insensitive to catch both 'git' and 'Git'.
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: #159 Reviewed-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
(cherry picked from commit d254d1e36cd2acf830e5977920ca2ec6370fbd69)
Tom Rini [Mon, 19 Dec 2016 21:54:17 +0000 (16:54 -0500)]
post-receive.hook: Handle failure to find patch number
When pwclient info -h fails to come up with the number for the change in
question it will exit with a non-zero exit code. This failure will
propagate upwards and exit the script there. Make our call to
get_patch_id or in true so that our script here will see an empty id and
we continue on with the list.
Signed-off-by: Tom Rini <trini@konsulko.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit 0743dd0af9842c7ded4873892085a3f505062440)
Andrew Donnellan [Tue, 19 Dec 2017 05:41:27 +0000 (16:41 +1100)]
views: Don't render token section of user profile if REST API disabled
In profile.html, if settings.ENABLE_REST_API == False, trying to render a
link to the generate_token page will raise a NoReverseMatch exception, so
we shouldn't render that. In any case, if the REST API is disabled, we
really shouldn't render the API token section of the page at all.
Only render the API token and generation link if settings.ENABLE_REST_API
is True.
Reported-by: Tomas Novotny <tomas@novotny.cz> Closes: #138 ("NoReverseMatch exception on user login with disabled REST API") Fixes: 85c8f369204a ("views: Provide a way to view, (re)generate tokens") Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit 14034e8a44a497d32f56f31a0fdc4473336d92af)
Daniel Axtens [Thu, 28 Sep 2017 14:12:24 +0000 (00:12 +1000)]
xmlrpc/patch_list: only fetch required fields
OzLabs noticed *massive* slowdowns in queries like this one:
SELECT "patchwork_submission"."id", "patchwork_submission"."msgid",
"patchwork_submission"."date", "patchwork_submission"."headers",
"patchwork_submission"."submitter_id",
"patchwork_submission"."content", "patchwork_submission"."project_id",
"patchwork_submission"."name", "patchwork_patch"."submission_ptr_id",
"patchwork_patch"."diff", "patchwork_patch"."commit_ref",
"patchwork_patch"."pull_url", "patchwork_patch"."delegate_id",
"patchwork_patch"."state_id", "patchwork_patch"."archived",
"patchwork_patch"."hash" FROM "patchwork_patch" INNER JOIN
"patchwork_submission" ON ("patchwork_patch"."submission_ptr_id" =
"patchwork_submission"."id") WHERE
("patchwork_submission"."project_id" = 2 AND
"patchwork_patch"."state_id" = 1) ORDER BY
"patchwork_submission"."date" ASC
These appear to be a result of pwclient list operations. We *do not*
need content/headers/diff in this case - so do not fetch them as it
is incredibly expensive - queries in excess of 50s have been observed.
This should go to stable/2.0.
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Reported-by: Michael Ellerman <mpe@ellerman.id.au> Cc: Jeremy Kerr <jk@ozlabs.org> Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit 76c3dae262590b8fcddc3d24a89b9b3717e3a9dc)
Stephen Finucane [Thu, 31 Aug 2017 08:48:51 +0000 (09:48 +0100)]
REST: Filter projects by 'linkname', not 'name'
Based on a report on the OVS mailing list, it appears that projects are
being filtered on the 'Project.name' field instead of the
'Project.linkname' field. Correct this and add regression tests to
prevent it happening again.
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 08b2115 ("REST: Allow filtering by both project ID and linkname")
Closes-bug: #117 ("Projects are filtered on the wrong field") Cc: Daniel Axtens <dja@axtens.net>
[dja: drop mangling of value] Signed-off-by: Daniel Axtens <dja@axtens.net>
(cherry picked from commit 174de19d41d76f8387027e789e5975fc078f2d08)
Andrew Donnellan [Thu, 31 Aug 2017 04:31:24 +0000 (14:31 +1000)]
views: Fix "Add to bundle" dropdown on patch list view
The "Add to bundle" option in the patch list view requires the user's
list of bundles to be added to the context. Before PatchworkRequestContext
was removed, this was done in a context processor. When that was
refactored out, the bundles list was re-added in the patch detail view,
but not in the patch list view.
Add the bundles list in the patch list view to rectify this.
Reported-by: David Miller <davem@davemloft.net> Reported-by: Michael Ellerman <mpe@ellerman.id.au> Reported-by: Paul Mackerras <paulus@ozlabs.org> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Closes #116 ("Add to existing bundle drop down has disappeared from bottom of patch list page") Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit b62c11755c21f2114a6f71aeb21a8c1e18617751)
Jeremy Kerr [Mon, 28 Aug 2017 11:39:18 +0000 (19:39 +0800)]
lib/sql: fix permissions for v2.0.0 on postgres
Some tables are no longer present, and others that are used by the web
interface and mail parser need access permissions added.
This change was required to get patchwork going on patchwork.ozlabs.org;
there may be other permissions required, that we haven't hit yet. So,
some review would be good here.
Also: it's unlikely that we need DELETE for the mail parser, but I'm not
confident enough to remove that at the moment.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org> Reviewed-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit 234bc7c316c555a432f678a75b25095c643d7ba1)
Jeremy Kerr [Mon, 28 Aug 2017 11:39:17 +0000 (19:39 +0800)]
tests: Run FuzzTest within a transaction
Currently, the FuzzTests fail for me with:
/backends/base/base.py", line 428, in validate_no_broken_transaction
"An error occurred in the current transaction. You can't "
TransactionManagementError: An error occurred in the current transaction. You can't execute queries until the end of the 'atomic' block.
- because the SQL inserts can fail, during an active transaction (the
first failure I see is attempting to insert \0 chars in
codec-null.mbox); this causes the setup for the next test case to fail.
Instead, run each test in its own transaction.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org> Reviewed-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit e505f1fd26224fc7dcadf43146bc1408a470fe15)
Daniel Axtens [Mon, 28 Aug 2017 04:11:27 +0000 (14:11 +1000)]
Handle EmptyPage exceptions
If a user asks for a page beyond the range of pages, an EmptyPage
exception is thrown. Catch this and clamp the page number
appropriately.
Reported-by: Jeremy Kerr <jk@ozlabs.org> Signed-off-by: Daniel Axtens <dja@axtens.net> Tested-by: Jeremy Kerr <jk@ozlabs.org> Reviewed-by: Stephen Finucane <stephen@that.guru>
(cherry picked from commit 0901378a0aa32fbd9c0e6a937219199dab759ce0)
Andrew Donnellan [Sun, 20 Aug 2017 14:40:10 +0000 (00:40 +1000)]
models: Fix invocation of refresh_tag_counts() for Comments
In Comment.save() and Comment.delete(), we always call
Submission.refresh_tag_counts(), which is an empty stub, rather than
calling Patch.refresh_tag_counts() if the Submission is a Patch.
As such, tag counts are never updated on incoming comments.
Delete Submission.refresh_tag_counts(), as it's useless, and in
Comment.save()/delete(), invoke Patch.refresh_tag_counts() directly when
the submission is a Patch.
Reported-by: David Demelier <markand@malikania.fr> Fixes: 86172ccc161b ("models: Split Patch into two models")
Closes-bug: #111 ("A/R/T not updated on comments") Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Tue, 11 Jul 2017 06:30:48 +0000 (16:30 +1000)]
cron: fix deletion of unactivated accounts
There is a test in expire_notifications() that tries to check if
the user's last login matches the date joined. (I think the login
date is not set until a post-activation login.) This does not work:
on patchwork.ozlabs.org there are 10k users that have never been
deleted.
Drop the date test: it should be sufficient that a user is not
active and their confirmation is not pending.
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Fri, 7 Jul 2017 14:24:46 +0000 (00:24 +1000)]
parser: fix parsing of messages with empty subjects
The fuzz fixups made the test too strict ("if not subject" rather than
"if subject is None") in an attempt to catch broken subject headers.
This broke parsing of messages with an empty subject.
Fix it and add a test.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Sat, 1 Jul 2017 04:28:43 +0000 (14:28 +1000)]
parser: limit emails and names to 255 chars
Also picked up with afl-fuzz.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Wed, 28 Jun 2017 07:48:48 +0000 (17:48 +1000)]
parser: better date parsing
It turns out that there is a lot that can go wrong in parsing a
date. OverflowError, ValueError and OSError have all been observed.
If these go wrong, substitute the current datetime.
Signed-off-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Daniel Axtens [Wed, 28 Jun 2017 07:48:47 +0000 (17:48 +1000)]
parser: deal with headers entirely failing to parse
It turns out that the attempts in clean_header() to convert
headers to strings are not guaranteed to work: you can end up with,
for example, a base64 decoding error which makes it impossible
to determine any header content.
In this case, sanitise_header() should return None, and thus
clean_header() should return None. We then need to plumb that
through.
Signed-off-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Daniel Axtens [Wed, 28 Jun 2017 07:48:46 +0000 (17:48 +1000)]
parser: catch failures in decoding headers
Headers can fail to decode:
- if a part cannot be encoded as ascii
- if the coding hint names a codec that doesn't exist
- if there's a null byte in the codec name
Catch these errors.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Wed, 28 Jun 2017 07:48:44 +0000 (17:48 +1000)]
parser: don't assume headers are strings
In python3, mail.get() can return either a string, or an
email.header.Header type.
clean_header() is designed to clean headers into strings,
so make sure we use that everywhere.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Wed, 28 Jun 2017 07:48:43 +0000 (17:48 +1000)]
parser: fix charset 'guessing' algorithm
The charset guessing algorithm doesn't work if it has to guess
multiple charsets, as it overwrites the payload with None.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Wed, 28 Jun 2017 06:55:09 +0000 (16:55 +1000)]
Remove ResourceWarnings under Py3
This is just a matter of correctly closing files we open.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Aaron Conole [Wed, 14 Jun 2017 18:14:23 +0000 (14:14 -0400)]
events-api: allow filtering by date
This commit allows users of the REST API to query for events based on
the date field. This will allow utility writers to select a smaller
subset of events when polling.
Signed-off-by: Aaron Conole <aconole@bytheb.org> Reviewed-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Wed, 28 Jun 2017 19:54:03 +0000 (20:54 +0100)]
parser: Trivial fix of test docstring
Based on the 'References' and 'In-Reply-To' headers of the used mbox,
the docstring for the 'test_reply_nocover_noversion' test is incorrect.
Fix this.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Sean Farley <sean@farley.io>
Stephen Finucane [Wed, 28 Jun 2017 19:53:58 +0000 (20:53 +0100)]
parser: Support single-patch "series"
There are merits to supporting single-patch series, not least the
ability to provide two consistent interfaces that show _all_ patches in
the '/patches' endpoint and the '/series' endpoint.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Sean Farley <sean@farley.io>
Stephen Finucane [Sun, 25 Jun 2017 20:55:54 +0000 (21:55 +0100)]
parser: Use a series even if the version differs
Currently, once we've found a series for a new submission, we check to
ensure that the version of that series matches that of the submission,
and we discard the series if not. The original intention for this was
given in commit 'c21b30525', where this was added:
There are some things you probably shouldn't do on public mailing
lists, but which people do anyway.
- [PATCH 1/2] test: Add some lorem ipsum
- [PATCH 2/2] test: Convert to Markdown
- [PATCH v2 1/2] test: Add some lorem ipsum
- [PATCH v2 2/2] test: Convert to Markdown
We should correctly parse these...
This is unnecessary for two reasons:
- If the series is using proper references, then the mechanism that
we use to prevent this issue with unversioned follow ups (also added
in that patch) will work here: namely, we don't add submissions if an
submission with a given number already exists in that series.
- If the series is not using references, we already have a check for
version.
Seeing as this check is actually causing issues for legitimate typos, we
should just remove this check. Do so, adding a check to ensure we don't
regress.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Closes-bug: #105
Token authentication is generally viewed as a more secure option for API
authentication than storing a username and password.
Django REST Framework gives us a TokenAuthentication class and an authtoken
app that we can use to generate random tokens and authenticate to API
endpoints. Enable this support and add some tests to validate correct
behavior.
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Stephen Finucane <stephen@that.guru>
We provide our own, much smaller implementation of this currently.
However, we want to be able to implement slightly different variants of
this elsewhere and using an existing library helps avoid reinventing the
wheel and lets us use already battle-tested code.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Stephen Finucane [Tue, 30 May 2017 20:33:58 +0000 (21:33 +0100)]
models: Centralize generation of filenames
Move filename generation to a mixin. This allows us to reuse the code
for other items like cover letters. Some unncessary 'strip' calls are
removed as their unnecessary.
This allows us to change the file extension for diffs to 'diff', which
is a little more accurate.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Tue, 23 May 2017 14:08:13 +0000 (15:08 +0100)]
parser: Handle multiple reference headers
It's possible to duplicate message headers multiple times. One common
case is the 'Received' header, but it appears that multiple
'In-Reply-To' and 'References' headers are also a thing.
Handle these cases through the use of the 'Message.get_all' function,
which returns all matching headers, instead of the 'Message.get'
function previously used.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Wed, 24 May 2017 05:54:55 +0000 (06:54 +0100)]
parser: Strip whitespace from references
Some mail, particularly those generated with older versions of
git-send-email or written by hand, include some extra whitespace in the
'References' and 'In-Reply-To' lines. Ensure we always strip this,
preventing mismatches between this and 'Message-ID', which is already
stripped of whitespace, when looking up SeriesReference's.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Tue, 23 May 2017 11:41:58 +0000 (12:41 +0100)]
parser: Extend series heuristics to include other metadata
Not every series will include the reference headers necessary to do
proper series-ification, particularly those generated without the help
of 'git-send-email' or similar.
Make life a little easier for these folks by attempting to match on
other heuristics of the series: submitter, version, number of patches,
project (mailing list) and date. The last of these is particularly
important to prevent duplicate series getting munged together.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Andrew Donnellan [Thu, 25 May 2017 08:05:10 +0000 (18:05 +1000)]
docs/api: change POST to PATCH in REST API parameters example
api/rest.rst gives an example of how to POST parameters to the PatchDetail
view at api/patches/<patch_id>. However, the endpoint in question doesn't
support POST - you need to use PUT or PATCH. Change it to PATCH.
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Andrew Donnellan [Thu, 25 May 2017 07:38:04 +0000 (17:38 +1000)]
bundle: Fix use of basic auth for bundle mboxes
Commit 0b4f508a8438 ("views: Allow use of basic auth for bundle mboxes")
added support for using Django REST Framework's BasicAuthentication to
authenticate when accessing the bundle-mbox view.
To check the user's credentials, we call
BasicAuthentication.authenticate(), however, we don't check whether
the returned user is actually the bundle owner. This means that any user
can access any private bundle if they authenticate using basic
authentication.
Additionally, if invalid credentials are provided via a basic
authentication header, BasicAuthentication.authenticate() will throw an
AuthenticationFailed exception. We currently don't catch this, resulting in
an exception page being displayed rather than a 404.
Add a new helper, rest_auth(), that takes a request and returns a user.
Call this in bundle_mbox() and save the result into request.user before we
check whether request.user is actually the bundle owner.
Found by code inspection.
Fixes: 0b4f508a8438 ("views: Allow use of basic auth for bundle mboxes") Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Stephen Finucane <stephen@that.guru>
Andrew Donnellan [Thu, 25 May 2017 05:36:18 +0000 (15:36 +1000)]
docker: increase database connection timeout
When starting the Docker environment, if the web container can't see the
database immediately, it waits 5 seconds, tries again, then waits 15
seconds more to account for first-time start-ups where it takes a bit
longer for the database to be initialised.
Some of us, unfortunately, have slow computers with slow mechanical hard
drives which take just a bit longer. Increase the second timeout from 15
seconds to 60 seconds, testing every 5 seconds.
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Acked-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Robin Jarry [Tue, 23 May 2017 13:24:07 +0000 (15:24 +0200)]
pwclient: Force xmlrpc client to return unicode strings
On python 2, the reference implementation of the XML-RPC unmarshaller
decodes strings to unicode with the selected encoding (utf-8 by default)
but it tries to re-encode the unicode strings to ascii bytes before
returning the values. If it fails, it leaves the value as unicode.
Monkey-patch the internal xmlrpclib._stringify() function only on python
2 to force it to preserve unicode strings. This allows to have similar
behaviour in both python 2 and python 3.
Signed-off-by: Robin Jarry <robin.jarry@6wind.com> Signed-off-by: Stephen Finucane <stephen@that.guru>