Patchwork saves patches, comments etc with UTC timezone and reports
this time when opening the patch details. However, internally generated
processes such as events are reported with the instance's local time.
There's nothing wrong with that and making PW timezone-aware would add
useless complexity, but in a world-wide collaboration a lot of confusion
may arise as the timezone is not reported at all. Instance's local time
might be very different from the local time of CI integrating with PW,
which is different from the local time of person dealing with it etc.
Use UTC everywhere by default instead of UTC for sumbissions and local
timezone for internally generated events (which is not reported).
Signed-off-by: Veronika Kabatova <vkabatov@redhat.com>
[dja:
- squash 2 patches: https://patchwork.ozlabs.org/patch/876744/
https://patchwork.ozlabs.org/patch/877815/
- minor changes to both patches - rejig order of migrations and
adjust wording: "happened sooner" -> "happened earlier"] Tested-by: Daniel Axtens <dja@axtens.net> Signed-off-by: Daniel Axtens <dja@axtens.net>
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>
Sometimes, multiple projects reside at the same mailing list. So far,
Patchwork only allowed a single project per mailing list, which made it
impossible for these projects to use Patchwork (unless they did some
dirty hacks).
Add a new property `subject_match` to projects and implement filtering
on (list_id, subject_match) match instead of solely list_id. Instance
admin can specify a regex on a per-project basis when the project is
created.
Signed-off-by: Veronika Kabatova <vkabatov@redhat.com> Signed-off-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Sat, 24 Feb 2018 01:22:54 +0000 (12:22 +1100)]
tools: drop vagrant
It served us well, but it's now outdated (Trusty, Python 3.4, etc)
There is no indication that anyone uses it or keeps it up to date.
Signed-off-by: Daniel Axtens <dja@axtens.net> Acked-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Daniel Axtens [Thu, 15 Feb 2018 01:04:53 +0000 (12:04 +1100)]
Don't aim to have patchwork send email
Getting things like SPF and DKIM right for this would be nigh-on
impossible, plus it would mean sysadmins would have to let patchwork
send email, which is a risk to the reputation of their systems.
Let's just aim for being a read-only representation of the mailing
list for now.
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Daniel Axtens <dja@axtens.net>
This resolves a deprecation warning that's recently been raised:
UserWarning: The psycopg2 wheel package will be renamed from release
2.8; in order to keep installing from binary please use "pip install
psycopg2-binary" instead. For details see:
<http://initd.org/psycopg/docs/install.html#binary-install-from-pypi>.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Signed-off-by: Stephen Finucane <stephen@that.guru> Fixes: 8585ea5af ("models: Use 'base_manager_name'") Fixes: ed7328fdb ("models: Series plural name is Series")
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>
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:17 +0000 (13:43 +1100)]
Fix Python3.6 depreciation warnings
We see on Travis and on tox:
/home/travis/build/daxtens/patchwork/patchwork/tests/test_list.py:79: DeprecationWarning: invalid escape sequence \d
id_re = re.compile('<tr id="patch_row:(\d+)"')
/home/travis/build/daxtens/patchwork/patchwork/tests/test_mail_settings.py:248: DeprecationWarning: invalid escape sequence \s
form_re_template = ('<form\s+[^>]*action="%(url)s"[^>]*>'
/home/travis/build/daxtens/patchwork/patchwork/tests/test_parser.py:616: DeprecationWarning: invalid escape sequence \
'\ No newline at end of file'))
All of these are easy to fix: just mark them as raw strings.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Daniel Axtens [Thu, 25 Jan 2018 02:43:16 +0000 (13:43 +1100)]
docker: add Python3.6 to image
This is needed to support py3.6-django111 in tox - otherwise tox
fails.
Python3.6 is not in Xenial, so this - irritatingly - requires pulling
in either a PPA, or the package from Artful. PPAs are icky, so pull it
in from Artful.
We can either add an Artful repo (like we do for Trusty to pull in
python3.4), or move the image to Artful and add in the Xenial repo
to get python3.5. It's more efficient to move the entire image to
Artful and pull in 3.5 from Xenial - otherwise most packages get
downloaded from Artful anyway. It does mean we're going to need to
move again to Bionic in a few months, but we'll just have to deal.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
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:
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>
docs/development: Fix tox invocation for listing targets
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Reviewed-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Sun, 10 Dec 2017 17:30:32 +0000 (17:30 +0000)]
Modify sections used in release notes
Add an 'api' section and remove 'security' and 'issues'. The former will
helps us group REST API related changes, while the latter are not
currently used and are unlikely to ever be used.
Signed-off-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Fri, 24 Nov 2017 19:37:42 +0000 (19:37 +0000)]
Remove support for Django 1.6, 1.7
These versions are massively outdated and the only reason for keeping
them was to allow installation on RHEL 7 using the version provided via
EPEL. No one's actually using this so just kill it.
This also allows us to remove support for django-filter 0.11, which was
only retained for use with these older versions of Django.
Signed-off-by: Stephen Finucane <stephen@that.guru> Reviewed-by: Daniel Axtens <dja@axtens.net>
Andrew Donnellan [Tue, 19 Dec 2017 05:32:03 +0000 (16:32 +1100)]
models, templatetags: Make tag count column in patch list optional per tag
Add a field, show_column, to the Tag model to determine whether the tag
gets a tag count column in the patch list view. This allows the creation of
tags that will be collated when generating mboxes but won't take up space
in the patch list.
show_column will default to True to maintain the current behaviour by
default.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au> Closes: #142 ("Ability to add tags that don't also have a column in the UI") Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Stephen Finucane <stephen@that.guru>
Andrew Donnellan [Wed, 20 Dec 2017 06:34:24 +0000 (17:34 +1100)]
templates: Add click-to-copy patch ID ("mpe mode") to patch detail page
Similar to what we already do on the patch list page, display the patch ID
on the patch detail page and make it a click-to-copy button.
Closes: #115 ("Show copy-able patch ID ("mpe mode") on patch detail page") Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Signed-off-by: Stephen Finucane <stephen@that.guru>
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>
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>
Daniel Axtens [Thu, 23 Nov 2017 23:54:27 +0000 (10:54 +1100)]
postgres: Fix version to 9.6
Always version your dependencies, people.
Postgres 10 has been released, the data format is not backwards
compatible with Postgres 9.6. I don't think too many people have
moved to 10 yet, so test with 9.6 for now.
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Fri, 19 May 2017 14:53:25 +0000 (15:53 +0100)]
tox: Add Django 1.11
Add support for the latest release of Django, 1.11. This is the next LTS
release (1.8 being the last one), so it's particularly important that we
maintain support for this going forward.
While neither the latest releases of django-rest-framework nor that of
django-filter explicitly support Django 1.11, it appears that they are
functional [1][2]. We can bump these packages separately when new
versions are released.
Stephen Finucane [Fri, 19 May 2017 14:53:22 +0000 (15:53 +0100)]
REST: Allow for mutability of request.POST
Using Django 1.11 yields the following error for the 'patchwork.tests
.test_rest_api.TestCheckAPI.test_create' test:
AttributeError: This QueryDict instance is immutable
This occurs due to our modification of data to allow users to create
instances using a slugified state instead of the underlying integer
value, e.g. 'success' instead of 1.
Resolve this by unsetting the immutability of that queryset. As
suggested in the linked SO answer, there is limited side effects to
doing this.
Signed-off-by: Stephen Finucane <stephen@that.guru> Tested-by: Daniel Axtens <dja@axtens.net>
Stephen Finucane [Fri, 19 May 2017 14:54:08 +0000 (15:54 +0100)]
tox: Remove '--liveserver' parameter
This parameter is no longer supported in Django 1.11 [1]. Remove this
and instead set the 'DJANGO_LIVE_TEST_SERVER_ADDRESS' environment
variable, which will do the same thing for Django < 1.11 and be ignored
by Django >= 1.11.
Daniel Axtens [Fri, 20 Oct 2017 05:36:55 +0000 (16:36 +1100)]
Support testing with PostgreSQL
This allows us to easily test against PostgreSQL using the same
tooling we normally use. This is helpful in (for example) shaking
out the test failures that were observed on ozlabs.org
To use it:
docker-compose -f docker-compose-pg.yml <usual argument>
(You may find in necessary to do a 'docker-compose down' first,
depending on what state the system is in and what command you're
running.)
Signed-off-by: Daniel Axtens <dja@axtens.net> Reviewed-by: Stephen Finucane <stephen@that.guru>
Stephen Finucane [Fri, 20 Oct 2017 05:36:53 +0000 (16:36 +1100)]
REST: Specify default ordering fields
This hides warnings likes the following:
UnorderedObjectListWarning: Pagination may yield inconsistent
results with an unordered object_list: <QuerySet [<User:
test_user_0>]>
Signed-off-by: Stephen Finucane <stephen@that.guru>
[dja: fix merge conflict in patch.py] Signed-off-by: Daniel Axtens <dja@axtens.net> Acked-by: Stephen Finucane <stephen@that.guru>
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>
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>
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>
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>
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>
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>
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>