[PATCH] REST: don't 500 if we get a non-int ID in /comment/ views

Daniel Axtens dja at axtens.net
Sat Aug 21 01:32:58 AEST 2021


The Patch and Cover comment views passed unsanitised patch_id/pk
down to the query set filter, which attempts to convert them to
integers, fails, and propagates a ValueError up to us.

Rather than propagating it to the user as a 500, catch it explicitly
and return a 404. Ideally we'd like to return a 400 and some explanation,
but I can't see an easy way to bend Django or DRF to my will here.

Signed-off-by: Daniel Axtens <dja at axtens.net>

---

Will need some tweaking for 2.2, but conceptually simple.

Stephen, would be interested if you can come up with a better way
to propagate a meaningful error out to the user!
---
 patchwork/api/comment.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py
index 0c578b44f1cb..4fa48ff73e49 100644
--- a/patchwork/api/comment.py
+++ b/patchwork/api/comment.py
@@ -86,6 +86,13 @@ class CoverCommentList(ListAPIView):
     lookup_url_kwarg = 'pk'
 
     def get_queryset(self):
+        # ideally we'd want to raise a 400 here with some explanation, but
+        # it's not clear how to. DRF throws away any 404 message we provide
+        try:
+            int(self.kwargs['pk'])
+        except ValueError:
+            raise Http404
+
         if not Cover.objects.filter(pk=self.kwargs['pk']).exists():
             raise Http404
 
@@ -105,6 +112,13 @@ class PatchCommentList(ListAPIView):
     lookup_url_kwarg = 'patch_id'
 
     def get_queryset(self):
+        # ideally we'd want to raise a 400 here with some explanation, but
+        # it's not clear how to. DRF throws away any 404 message we provide
+        try:
+            int(self.kwargs['patch_id'])
+        except ValueError:
+            raise Http404
+
         if not Patch.objects.filter(id=self.kwargs['patch_id']).exists():
             raise Http404
 
-- 
2.30.2



More information about the Patchwork mailing list