From 9f09857397c6681bc511e2ffd31ef190c7bb51d5 Mon Sep 17 00:00:00 2001
From: rdb <git@rdb.name>
Date: Sat, 12 Dec 2020 14:59:18 +0100
Subject: [PATCH] collide: Error checking for
 CollisionPolygon::setup/verify_points()

---
 panda/src/collide/collisionPolygon_ext.cxx | 40 +++++++++++++++-------
 panda/src/collide/collisionPolygon_ext.h   |  2 +-
 tests/collide/test_collision_polygon.py    | 10 ++++++
 3 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/panda/src/collide/collisionPolygon_ext.cxx b/panda/src/collide/collisionPolygon_ext.cxx
index 398db4fa7b..1ecb68ef76 100644
--- a/panda/src/collide/collisionPolygon_ext.cxx
+++ b/panda/src/collide/collisionPolygon_ext.cxx
@@ -29,7 +29,11 @@ extern struct Dtool_PyTypedObject Dtool_LPoint3f;
  */
 bool Extension<CollisionPolygon>::
 verify_points(PyObject *points) {
-  const pvector<LPoint3> vec = convert_points(points);
+  pvector<LPoint3> vec;
+  if (!convert_points(vec, points)) {
+    return false;
+  }
+
   const LPoint3 *verts_begin = &vec[0];
   const LPoint3 *verts_end = verts_begin + vec.size();
 
@@ -42,7 +46,16 @@ verify_points(PyObject *points) {
  */
 void Extension<CollisionPolygon>::
 setup_points(PyObject *points) {
-  const pvector<LPoint3> vec = convert_points(points);
+  pvector<LPoint3> vec;
+  if (!convert_points(vec, points)) {
+    return;
+  }
+
+  if (vec.size() < 3) {
+    PyErr_SetString(PyExc_ValueError, "expected at least 3 points");
+    return;
+  }
+
   const LPoint3 *verts_begin = &vec[0];
   const LPoint3 *verts_end = verts_begin + vec.size();
 
@@ -52,13 +65,11 @@ setup_points(PyObject *points) {
 /**
  * Converts a Python sequence to a list of LPoint3 objects.
  */
-pvector<LPoint3> Extension<CollisionPolygon>::
-convert_points(PyObject *points) {
-  pvector<LPoint3> vec;
+bool Extension<CollisionPolygon>::
+convert_points(pvector<LPoint3> &vec, PyObject *points) {
   PyObject *seq = PySequence_Fast(points, "function expects a sequence");
-
   if (!seq) {
-    return vec;
+    return false;
   }
 
   PyObject **items = PySequence_Fast_ITEMS(seq);
@@ -69,18 +80,23 @@ convert_points(PyObject *points) {
 
   for (Py_ssize_t i = 0; i < len; ++i) {
 #ifdef STDFLOAT_DOUBLE
-    if (ptr = DtoolInstance_UPCAST(items[i], Dtool_LPoint3d)) {
+    if (DtoolInstance_Check(itemts[i]) &&
+        (ptr = DtoolInstance_UPCAST(items[i], Dtool_LPoint3d))) {
 #else
-    if (ptr = DtoolInstance_UPCAST(items[i], Dtool_LPoint3f)) {
+    if (DtoolInstance_Check(items[i]) &&
+        (ptr = DtoolInstance_UPCAST(items[i], Dtool_LPoint3f))) {
 #endif
       vec.push_back(*(LPoint3 *)ptr);
-    } else {
-      collide_cat.warning() << "Argument must be of LPoint3 type.\n";
+    }
+    else {
+      Dtool_Raise_TypeError("Argument must be of LPoint3 type.");
+      Py_DECREF(seq);
+      return false;
     }
   }
 
   Py_DECREF(seq);
-  return vec;
+  return true;
 }
 
 #endif
diff --git a/panda/src/collide/collisionPolygon_ext.h b/panda/src/collide/collisionPolygon_ext.h
index ed2043066d..ef61c6b4f8 100644
--- a/panda/src/collide/collisionPolygon_ext.h
+++ b/panda/src/collide/collisionPolygon_ext.h
@@ -35,7 +35,7 @@ public:
   void setup_points(PyObject *points);
 
 private:
-  static pvector<LPoint3> convert_points(PyObject *points);
+  static bool convert_points(pvector<LPoint3> &vec, PyObject *points);
 };
 
 #endif  // HAVE_PYTHON
diff --git a/tests/collide/test_collision_polygon.py b/tests/collide/test_collision_polygon.py
index 0d1c884709..a2acaf0a6b 100644
--- a/tests/collide/test_collision_polygon.py
+++ b/tests/collide/test_collision_polygon.py
@@ -1,4 +1,5 @@
 from panda3d import core
+import pytest
 
 
 def test_collision_polygon_verify_not_enough_points():
@@ -27,6 +28,9 @@ def test_collision_polygon_verify_points():
     assert core.CollisionPolygon.verify_points([core.LPoint3(3, -8, -7), core.LPoint3(9, 10, 8), core.LPoint3(7, 0, 10), core.LPoint3(-6, -2, 3)])
     assert core.CollisionPolygon.verify_points([core.LPoint3(-1, -3, -5), core.LPoint3(10, 3, -10), core.LPoint3(-10, 10, -4), core.LPoint3(0, 1, -4), core.LPoint3(-9, -2, 0)])
 
+    with pytest.raises(TypeError):
+        core.CollisionPolygon.verify_points([core.LPoint3(0, 0, 0), None])
+
 
 def test_collision_polygon_setup_points():
     # Create empty collision polygon
@@ -43,3 +47,9 @@ def test_collision_polygon_setup_points():
         polygon.setup_points(points)
         assert polygon.is_valid()
         assert polygon.get_num_points() == len(points)
+
+    with pytest.raises(TypeError):
+        polygon.setup_points([core.LPoint3(0, 0, 0), None, 1])
+
+    with pytest.raises(ValueError):
+        polygon.setup_points([core.LPoint3(0, 0, 0)])