views: Raise 404 if downloading non-existent dependencies

Message ID 20180607130447.25186-1-stephen@that.guru
State Accepted
Headers show
Series
  • views: Raise 404 if downloading non-existent dependencies
Related show

Commit Message

Stephen Finucane June 7, 2018, 1:04 p.m.
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>
Reported-by: John McNamara <john.mcnamara@intel.com>
Fixes: e2dfd490 ("views: Add 'series' parameter to '/mbox' endpoint")
Closes: #189
---
NOTE: This needs to be backported to stable/2.0 once merged to master as
it's an issue there too.
---
 patchwork/tests/test_mboxviews.py             | 24 +++++++++++++++++++
 patchwork/views/patch.py                      |  5 ++++
 .../notes/issue-189-fe4024f33e1b5203.yaml     |  7 ++++++
 3 files changed, 36 insertions(+)
 create mode 100644 releasenotes/notes/issue-189-fe4024f33e1b5203.yaml

Comments

Daniel Axtens June 13, 2018, 2:17 a.m. | #1
I've briefly read through the patch, and it LGTM.

Stephen Finucane <stephen@that.guru> writes:

> 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>
> Reported-by: John McNamara <john.mcnamara@intel.com>
> Fixes: e2dfd490 ("views: Add 'series' parameter to '/mbox' endpoint")
> Closes: #189
> ---
> NOTE: This needs to be backported to stable/2.0 once merged to master as
> it's an issue there too.
> ---
>  patchwork/tests/test_mboxviews.py             | 24 +++++++++++++++++++
>  patchwork/views/patch.py                      |  5 ++++
>  .../notes/issue-189-fe4024f33e1b5203.yaml     |  7 ++++++
>  3 files changed, 36 insertions(+)
>  create mode 100644 releasenotes/notes/issue-189-fe4024f33e1b5203.yaml
>
> diff --git a/patchwork/tests/test_mboxviews.py b/patchwork/tests/test_mboxviews.py
> index 2d6cdc30..8eb3581a 100644
> --- a/patchwork/tests/test_mboxviews.py
> +++ b/patchwork/tests/test_mboxviews.py
> @@ -31,6 +31,7 @@ from patchwork.tests.utils import create_comment
>  from patchwork.tests.utils import create_patch
>  from patchwork.tests.utils import create_project
>  from patchwork.tests.utils import create_person
> +from patchwork.tests.utils import create_series_patch
>  from patchwork.tests.utils import create_user
>  
>  
> @@ -206,3 +207,26 @@ class MboxCommentPostcriptUnchangedTest(TestCase):
>  
>          self.assertContains(response, content)
>          self.assertNotContains(response, content + '\n')
> +
> +
> +class MboxSeriesDependencies(TestCase):
> +
> +    def test_patch_with_dependencies(self):
> +        patch_a = create_series_patch()
> +        patch_b = create_series_patch(series=patch_a.series)
> +
> +        response = self.client.get('%s?series=*' % reverse(
> +            'patch-mbox', args=[patch_b.patch.id]))
> +
> +        self.assertContains(response, patch_a.patch.content)
> +        self.assertContains(response, patch_b.patch.content)
> +
> +    def test_legacy_patch(self):
> +        """Validate a patch with non-existent dependencies raises a 404."""
> +        # we're explicitly creating a patch without a series
> +        patch = create_patch()
> +
> +        response = self.client.get('%s?series=*' % reverse(
> +            'patch-mbox', args=[patch.id]))
> +
> +        self.assertEqual(response.status_code, 404)
> diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
> index eccb5441..cbd4ec39 100644
> --- a/patchwork/views/patch.py
> +++ b/patchwork/views/patch.py
> @@ -139,6 +139,11 @@ def patch_mbox(request, patch_id):
>  
>      response = HttpResponse(content_type='text/plain')
>      if series_id:
> +        if not patch.series.count():
> +            raise Http404('Patch does not have an associated series. This is '
> +                          'because the patch was processed with an older '
> +                          'version of Patchwork. It is not possible to '
> +                          'provide dependencies for this patch.')
>          response.write(series_patch_to_mbox(patch, series_id))
>      else:
>          response.write(patch_to_mbox(patch))
> diff --git a/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml b/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml
> new file mode 100644
> index 00000000..ce9fe93c
> --- /dev/null
> +++ b/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml
> @@ -0,0 +1,7 @@
> +---
> +fixes:
> +  - |
> +    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. This was not
> +    previously handled correctly. A 404 is now raised if this occurs.
> -- 
> 2.17.1
>
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
Stephen Finucane June 13, 2018, 9:33 a.m. | #2
On Wed, 2018-06-13 at 12:17 +1000, Daniel Axtens wrote:
> I've briefly read through the patch, and it LGTM.

Thanks. I've applied this now. Got two more patches to resolve
usability concerns and I think we're good to cut 2.1.

Stephen 

> Stephen Finucane <stephen@that.guru> writes:
> 
> > 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>
> > Reported-by: John McNamara <john.mcnamara@intel.com>
> > Fixes: e2dfd490 ("views: Add 'series' parameter to '/mbox'
> > endpoint")
> > Closes: #189
> > ---
> > NOTE: This needs to be backported to stable/2.0 once merged to
> > master as
> > it's an issue there too.
> > ---
> >  patchwork/tests/test_mboxviews.py             | 24
> > +++++++++++++++++++
> >  patchwork/views/patch.py                      |  5 ++++
> >  .../notes/issue-189-fe4024f33e1b5203.yaml     |  7 ++++++
> >  3 files changed, 36 insertions(+)
> >  create mode 100644 releasenotes/notes/issue-189-
> > fe4024f33e1b5203.yaml
> > 
> > diff --git a/patchwork/tests/test_mboxviews.py
> > b/patchwork/tests/test_mboxviews.py
> > index 2d6cdc30..8eb3581a 100644
> > --- a/patchwork/tests/test_mboxviews.py
> > +++ b/patchwork/tests/test_mboxviews.py
> > @@ -31,6 +31,7 @@ from patchwork.tests.utils import create_comment
> >  from patchwork.tests.utils import create_patch
> >  from patchwork.tests.utils import create_project
> >  from patchwork.tests.utils import create_person
> > +from patchwork.tests.utils import create_series_patch
> >  from patchwork.tests.utils import create_user
> >  
> >  
> > @@ -206,3 +207,26 @@ class
> > MboxCommentPostcriptUnchangedTest(TestCase):
> >  
> >          self.assertContains(response, content)
> >          self.assertNotContains(response, content + '\n')
> > +
> > +
> > +class MboxSeriesDependencies(TestCase):
> > +
> > +    def test_patch_with_dependencies(self):
> > +        patch_a = create_series_patch()
> > +        patch_b = create_series_patch(series=patch_a.series)
> > +
> > +        response = self.client.get('%s?series=*' % reverse(
> > +            'patch-mbox', args=[patch_b.patch.id]))
> > +
> > +        self.assertContains(response, patch_a.patch.content)
> > +        self.assertContains(response, patch_b.patch.content)
> > +
> > +    def test_legacy_patch(self):
> > +        """Validate a patch with non-existent dependencies raises
> > a 404."""
> > +        # we're explicitly creating a patch without a series
> > +        patch = create_patch()
> > +
> > +        response = self.client.get('%s?series=*' % reverse(
> > +            'patch-mbox', args=[patch.id]))
> > +
> > +        self.assertEqual(response.status_code, 404)
> > diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
> > index eccb5441..cbd4ec39 100644
> > --- a/patchwork/views/patch.py
> > +++ b/patchwork/views/patch.py
> > @@ -139,6 +139,11 @@ def patch_mbox(request, patch_id):
> >  
> >      response = HttpResponse(content_type='text/plain')
> >      if series_id:
> > +        if not patch.series.count():
> > +            raise Http404('Patch does not have an associated
> > series. This is '
> > +                          'because the patch was processed with an
> > older '
> > +                          'version of Patchwork. It is not
> > possible to '
> > +                          'provide dependencies for this patch.')
> >          response.write(series_patch_to_mbox(patch, series_id))
> >      else:
> >          response.write(patch_to_mbox(patch))
> > diff --git a/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml
> > b/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml
> > new file mode 100644
> > index 00000000..ce9fe93c
> > --- /dev/null
> > +++ b/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml
> > @@ -0,0 +1,7 @@
> > +---
> > +fixes:
> > +  - |
> > +    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.
> > This was not
> > +    previously handled correctly. A 404 is now raised if this
> > occurs.
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > Patchwork mailing list
> > Patchwork@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/patchwork

Patch

diff --git a/patchwork/tests/test_mboxviews.py b/patchwork/tests/test_mboxviews.py
index 2d6cdc30..8eb3581a 100644
--- a/patchwork/tests/test_mboxviews.py
+++ b/patchwork/tests/test_mboxviews.py
@@ -31,6 +31,7 @@  from patchwork.tests.utils import create_comment
 from patchwork.tests.utils import create_patch
 from patchwork.tests.utils import create_project
 from patchwork.tests.utils import create_person
+from patchwork.tests.utils import create_series_patch
 from patchwork.tests.utils import create_user
 
 
@@ -206,3 +207,26 @@  class MboxCommentPostcriptUnchangedTest(TestCase):
 
         self.assertContains(response, content)
         self.assertNotContains(response, content + '\n')
+
+
+class MboxSeriesDependencies(TestCase):
+
+    def test_patch_with_dependencies(self):
+        patch_a = create_series_patch()
+        patch_b = create_series_patch(series=patch_a.series)
+
+        response = self.client.get('%s?series=*' % reverse(
+            'patch-mbox', args=[patch_b.patch.id]))
+
+        self.assertContains(response, patch_a.patch.content)
+        self.assertContains(response, patch_b.patch.content)
+
+    def test_legacy_patch(self):
+        """Validate a patch with non-existent dependencies raises a 404."""
+        # we're explicitly creating a patch without a series
+        patch = create_patch()
+
+        response = self.client.get('%s?series=*' % reverse(
+            'patch-mbox', args=[patch.id]))
+
+        self.assertEqual(response.status_code, 404)
diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
index eccb5441..cbd4ec39 100644
--- a/patchwork/views/patch.py
+++ b/patchwork/views/patch.py
@@ -139,6 +139,11 @@  def patch_mbox(request, patch_id):
 
     response = HttpResponse(content_type='text/plain')
     if series_id:
+        if not patch.series.count():
+            raise Http404('Patch does not have an associated series. This is '
+                          'because the patch was processed with an older '
+                          'version of Patchwork. It is not possible to '
+                          'provide dependencies for this patch.')
         response.write(series_patch_to_mbox(patch, series_id))
     else:
         response.write(patch_to_mbox(patch))
diff --git a/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml b/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml
new file mode 100644
index 00000000..ce9fe93c
--- /dev/null
+++ b/releasenotes/notes/issue-189-fe4024f33e1b5203.yaml
@@ -0,0 +1,7 @@ 
+---
+fixes:
+  - |
+    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. This was not
+    previously handled correctly. A 404 is now raised if this occurs.