Re: [PATCH v2] models: Validate Project.linkname does not contain forward slash
> Thanks! This is one of those things you just assume people are doing so > you never bother enforcing it :) In hindsight, the 'Project.linkname' > field is supposed to be a slug - that is, restricted to letters, > numbers, underscores and hyphens. You'd use 'Project.name' if you > wanted more details. Does it make sense to enforce that, as opposed to > simply excluding slashes? If so, I suspect we should use the built-in > 'validate_unicode_slug' validator [1]? > > Thoughts/objections? I think that's even better, if that's the original intention. I'll submit a new patch. Thanks for getting back on this :-) -- Thomas ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: Regression in the patches API
On Wed, 2020-06-03 at 18:00 +0200, Ralf Ramsauer wrote:
> Hi,
>
> it seems that the /patches-API is broken under certain conditions. The
> API call throws an exception if the following conditions are met:
> - The user is logged in a browser
> - In the browser, the user requests /api/patches//
> - The user is maintainer of the project related to
>
> This fault can easily be reproduced with a vanilla patchwork instance:
> 1. ./manage.py createsuperuser # create 'admin' superuser
> 2. Login in a browser as admin
> 3. Create some new project
> 4. ./manage.py parsearchive list.mbox # import some patches
> 6. In the browser, navigate to /admin/auth/user/1/change
>and add the user as maintainer of the project
> 7. In the browser, navigate to /api/patches/1
> 8. Exception is thrown
>
> The exception is not thrown, if step 6 is either skipped or reverted.
> Please find the backtrace attached.
Thanks for bringing this up. I spent some time looking at this today.
The reason this is specific to being a project maintainer is that the
editable fields in the browseable API only appear for project
maintainers or superusers. It appears the 'patch.relation' field is
breaking this because we're modifying the output from this:
'relations': {
'patches': [
... list of patches ...
]
},
to this:
'relations': [
... list of patches ...
],
I guess the main issue is that we're going from an object (the relation
through model) thing to a list of objects.
I haven't yet figured out how to preserve the list of input fields
while retaining this style of output. We might need to drop the former
if I can't figure that out.
Stephen
> Thanks
> Ralf
> ___
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] pwclient: Add check-get subcommand
On Thu, 2020-09-10 at 17:11 -0700, Olof Johansson wrote: > Provide a subcommand to query the checks ran against a specific patch_id. > > Also allows formatting of the output, to make it easier to query whether > a specific patch has seen any tests (and what their status is). > > Signed-off-by: Olof Johansson Thanks. I added a release note and some minor style fixups and then applied this. It will be part of the imminent 1.3.0 release :) Cheers, Stephen ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH 3/5] settings: Add context processor django.template.context_processors.request
On Fri, 2020-08-28 at 00:14 +1000, Andrew Donnellan wrote: > Django 3.1 adds a new admin sidebar feature that requires the > django.template.context_processors.request context processor to be enabled > in the settings. > > Signed-off-by: Andrew Donnellan Compatible with Django 3.0 also. LGTM. Reviewed-by: Stephen Finucane ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH 5/5] Add support for Django 3.1
On Fri, 2020-08-28 at 00:14 +1000, Andrew Donnellan wrote: > Signed-off-by: Andrew Donnellan Reviewed-by: Stephen Finucane And that wraps up the series. Thanks (!), and apologies for not getting to it sooner. Stephen ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH 4/5] requirements: Update django-filter
On Fri, 2020-08-28 at 00:14 +1000, Andrew Donnellan wrote: > Update django-filter dependency to a version that's compatible with Django > 3.1. > > Signed-off-by: Andrew Donnellan Reviewed-by: Stephen Finucane ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH 2/5] urls: Update url pattern functions
On Fri, 2020-08-28 at 00:14 +1000, Andrew Donnellan wrote: > Django 3.1 deprecates django.conf.urls.url() as an alias for > django.urls.re_path(). Also switch to using django.urls.include() rather > than django.conf.urls.include(). > > Signed-off-by: Andrew Donnellan Reviewed-by: Stephen Finucane PS: I messed with the formatting of this a little to avoid the hanging indents before applying it. ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH 1/5] templates: Replace ifequal and ifnotequal with if
On Fri, 2020-08-28 at 00:14 +1000, Andrew Donnellan wrote: > Django 3.1 deprecates the ifequal and ifnotequal tags, for removal in 4.0. > Replace all occurrences of ifequal and ifnotequal with if. > > Signed-off-by: Andrew Donnellan Looks sensible. Reviewed-by: Stephen Finucane and applied. ___ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH] models: Validate Project.linkname does not contain forward slash
On Tue, 2020-09-08 at 21:31 +1000, Andrew Donnellan wrote:
> On 6/9/20 10:55 pm, Thomas Bracht Laumann Jespersen wrote:
> > I started by creating a project that contained a forward slash
> > (importing patches from https://lists.sr.ht/~sircmpwn/sr.ht-dev/) and
> > it fails to render the "projects" main page.
> >
> > The specific error reads:
> >
> > NoReverseMatch at /
> >
> > Reverse for 'patch-list' with keyword arguments
> > '{'project_id': 'foo/bar'}' not found. 1 pattern(s) tried:
> > ['project/(?P[^/]+)/list/$']
> >
> > which appears to explicitly disallow forward slashes.
> >
> > So I think it makes sense to validate that project linkname doesn't
> > contain forward slahes.
> >
> > Signed-off-by: Thomas Bracht Laumann Jespersen
> > ---
> >
> > I hard a hard time satisfying flake8, so I ended up disabling the pre-commit
> > hook. I also looked over the documentation for contributors and figure that
> > if
> > a release note is required, just let me know then I'll add it.
>
> TIL we have precommit hooks. I've been contributing to patchwork for a
> few years now...
>
> In any case, it also causes the test suite to fail its flake8 check.
> Perhaps we need to exclude the migrations directory?
Running 'pre-commit run -a' before you commit will do the trick also.
The 'patchwork.migrations directory was previously excluded, but we re-
added it for validation recently enough as quite a few of the
migrations in there were hand-written and could do with the validation.
Stephen
> > .../0044_add_project_linkname_validation.py | 19 +++
> > patchwork/models.py | 10 +-
> > 2 files changed, 28 insertions(+), 1 deletion(-)
> > create mode 100644
> > patchwork/migrations/0044_add_project_linkname_validation.py
> >
> > diff --git a/patchwork/migrations/0044_add_project_linkname_validation.py
> > b/patchwork/migrations/0044_add_project_linkname_validation.py
> > new file mode 100644
> > index 000..2fff7df
> > --- /dev/null
> > +++ b/patchwork/migrations/0044_add_project_linkname_validation.py
> > @@ -0,0 +1,19 @@
> > +# Generated by Django 3.0.10 on 2020-09-06 22:47
> > +
> > +from django.db import migrations, models
> > +import patchwork.models
> > +
> > +
> > +class Migration(migrations.Migration):
> > +
> > +dependencies = [
> > +('patchwork', '0043_merge_patch_submission'),
> > +]
> > +
> > +operations = [
> > +migrations.AlterField(
> > +model_name='project',
> > +name='linkname',
> > +field=models.CharField(max_length=255, unique=True,
> > validators=[patchwork.models.validate_project_linkname]),
> > +),
> > +]
> > diff --git a/patchwork/models.py b/patchwork/models.py
> > index 77ab924..9027219 100644
> > --- a/patchwork/models.py
> > +++ b/patchwork/models.py
> > @@ -31,6 +31,13 @@ def validate_regex_compiles(regex_string):
> > raise ValidationError('Invalid regular expression entered!')
> >
> >
> > +def validate_project_linkname(linkname):
> > +if re.fullmatch(r'[^/]+', linkname) is None:
>
> This could just be "if '/' in linkname:"
>
> > +raise ValidationError(
> > +'Invalid project linkname: Value cannot contain forward slash
> > (/)'
> > +)
> > +
> > +
> > class Person(models.Model):
> > # properties
> >
> > @@ -56,7 +63,8 @@ class Person(models.Model):
> > class Project(models.Model):
> > # properties
> >
> > -linkname = models.CharField(max_length=255, unique=True)
> > +linkname = models.CharField(max_length=255, unique=True,
> > +validators=[validate_project_linkname])
> > name = models.CharField(max_length=255, unique=True)
> > listid = models.CharField(max_length=255)
> > listemail = models.CharField(max_length=200)
> >
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
Re: [PATCH v2] models: Validate Project.linkname does not contain forward slash
On Tue, 2020-09-08 at 16:06 +0200, Thomas Bracht Laumann Jespersen wrote:
> I started by creating a project that contained a forward slash
> (importing patches from https://lists.sr.ht/~sircmpwn/sr.ht-dev/) and
> it fails to render the "projects" main page.
>
> The specific error reads:
>
> NoReverseMatch at /
>
> Reverse for 'patch-list' with keyword arguments
> '{'project_id': 'foo/bar'}' not found. 1 pattern(s) tried:
> ['project/(?P[^/]+)/list/$']
>
> which appears to explicitly disallow forward slashes.
>
> So I think it makes sense to validate that project linkname doesn't
> contain forward slahes.
>
> Signed-off-by: Thomas Bracht Laumann Jespersen
Thanks! This is one of those things you just assume people are doing so
you never bother enforcing it :) In hindsight, the 'Project.linkname'
field is supposed to be a slug - that is, restricted to letters,
numbers, underscores and hyphens. You'd use 'Project.name' if you
wanted more details. Does it make sense to enforce that, as opposed to
simply excluding slashes? If so, I suspect we should use the built-in
'validate_unicode_slug' validator [1]?
Thoughts/objections?
Stephen
[1] https://docs.djangoproject.com/en/3.1/ref/validators/#validate-unicode-slug
> ---
>
> I simplified the check for '/' as suggested, and got the pre-commit hook to
> work. I changed the formatting in the migration to satisfy the max line length
> check.
>
> .../0044_add_project_linkname_validation.py | 23 +++
> patchwork/models.py | 10 +++-
> 2 files changed, 32 insertions(+), 1 deletion(-)
> create mode 100644
> patchwork/migrations/0044_add_project_linkname_validation.py
>
> diff --git a/patchwork/migrations/0044_add_project_linkname_validation.py
> b/patchwork/migrations/0044_add_project_linkname_validation.py
> new file mode 100644
> index 000..8add430
> --- /dev/null
> +++ b/patchwork/migrations/0044_add_project_linkname_validation.py
> @@ -0,0 +1,23 @@
> +# Generated by Django 3.0.10 on 2020-09-06 22:47
> +
> +from django.db import migrations, models
> +import patchwork.models
> +
> +
> +class Migration(migrations.Migration):
> +
> +dependencies = [
> +('patchwork', '0043_merge_patch_submission'),
> +]
> +
> +operations = [
> +migrations.AlterField(
> +model_name='project',
> +name='linkname',
> +field=models.CharField(
> +max_length=255,
> +unique=True,
> +validators=[patchwork.models.validate_project_linkname]
> +),
> +),
> +]
> diff --git a/patchwork/models.py b/patchwork/models.py
> index 77ab924..64d5a10 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -31,6 +31,13 @@ def validate_regex_compiles(regex_string):
> raise ValidationError('Invalid regular expression entered!')
>
>
> +def validate_project_linkname(linkname):
> +if '/' in linkname:
> +raise ValidationError(
> +'Invalid project linkname: Value cannot contain forward slash
> (/)'
> +)
> +
> +
> class Person(models.Model):
> # properties
>
> @@ -56,7 +63,8 @@ class Person(models.Model):
> class Project(models.Model):
> # properties
>
> -linkname = models.CharField(max_length=255, unique=True)
> +linkname = models.CharField(max_length=255, unique=True,
> +validators=[validate_project_linkname])
> name = models.CharField(max_length=255, unique=True)
> listid = models.CharField(max_length=255)
> listemail = models.CharField(max_length=200)
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
