Re: [PATCH] models: Validate Project.linkname does not contain forward slash

2020-09-26 Thread Stephen Finucane 
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] models: Validate Project.linkname does not contain forward slash

2020-09-08 Thread 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?

I can also just bite the bullet and get precommit working, but it does seem
aggressive to fail tests on auto-generated files.

> > 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:"

Sure, that's way simpler, I'll submit another patch. Thanks for the feedback!

-- Thomas
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Re: [PATCH] models: Validate Project.linkname does not contain forward slash

2020-09-08 Thread Andrew Donnellan 

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?




  .../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)



--
Andrew Donnellan  OzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

[PATCH] models: Validate Project.linkname does not contain forward slash

2020-09-06 Thread Thomas Bracht Laumann Jespersen 
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.

 .../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:
+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)
-- 
2.26.2

___
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork