[PATCH saem_ref] [ark] Avoid infinite loops by settings a maximum number of iterations limit

Denis Laxalde denis.laxalde at logilab.fr
Wed Feb 28 10:32:36 CET 2018


# HG changeset patch
# User Denis Laxalde <denis.laxalde at logilab.fr>
# Date 1519634988 -3600
#      Mon Feb 26 09:49:48 2018 +0100
# Node ID 69c8586db0b8c985b70d5e8a7d7a7da5e1d57b8d
# Parent  dc88b0ed93f8b907fdf9e7437388af494a55a9df
# Available At http://hg.logilab.org/review/cubes/saem_ref
#              hg pull http://hg.logilab.org/review/cubes/saem_ref -r 69c8586db0b8
[ark] Avoid infinite loops by settings a maximum number of iterations limit

Arbitrarily set this limit to 20, without further usage knowledge.
We don't catch the error on application side for now, waiting for an
actual use case.

diff --git a/cubicweb_saem_ref/ark.py b/cubicweb_saem_ref/ark.py
--- a/cubicweb_saem_ref/ark.py
+++ b/cubicweb_saem_ref/ark.py
@@ -20,6 +20,7 @@ import re
 from six import text_type
 
 
+ARK_MAXIT = 20
 ARK_PREFIX = 'rf'
 ARK_CONTROLCHAR = 'g'
 ARK_NAME_LENGTH = 10
@@ -57,8 +58,8 @@ def generate_ark(cnx, naan):
     Return the generated name.
     """
     cu = cnx.system_sql(
-        'select * from gen_ark(%s, %s, %s, %s);',
-        (naan, ARK_NAME_LENGTH, ARK_PREFIX, ARK_CONTROLCHAR),
+        'select * from gen_ark(%s, %s, %s, %s, %s);',
+        (naan, ARK_NAME_LENGTH, ARK_PREFIX, ARK_CONTROLCHAR, ARK_MAXIT),
     )
     name, = cu.fetchone()
     return name
@@ -71,8 +72,8 @@ def generate_qualified_ark(cnx, naan, na
     Return the generated qualifier.
     """
     cu = cnx.system_sql(
-        'select * from gen_qualified_ark(%s, %s, %s);',
-        (naan, name, ARK_QUALIFIER_LENGTH),
+        'select * from gen_qualified_ark(%s, %s, %s, %s);',
+        (naan, name, ARK_QUALIFIER_LENGTH, ARK_MAXIT),
     )
     qualifier, = cu.fetchone()
     return qualifier
diff --git a/cubicweb_saem_ref/schema/ark.postgres.sql b/cubicweb_saem_ref/schema/ark.postgres.sql
--- a/cubicweb_saem_ref/schema/ark.postgres.sql
+++ b/cubicweb_saem_ref/schema/ark.postgres.sql
@@ -52,12 +52,17 @@ END;
 -- Insert a record in "ark" table ensuring uniqueness constraints are
 -- fulfilled.
 CREATE OR REPLACE
-FUNCTION gen_ark(naan INTEGER, len INTEGER, prefix TEXT, control_char TEXT)
+FUNCTION gen_ark(naan INTEGER, len INTEGER, prefix TEXT, control_char TEXT, maxit INTEGER)
 RETURNS TEXT AS $$
 DECLARE
     ark_name TEXT;
+    nit INTEGER;
 BEGIN
+    nit := 0;
     LOOP
+        ASSERT nit < maxit,
+            'ark generation loop exceeded maximum number of iterations: ' || maxit;
+        nit := nit + 1;
         BEGIN
             INSERT INTO ark
                 VALUES (naan, gen_ark_part(len, prefix, control_char), DEFAULT)
@@ -66,7 +71,6 @@ BEGIN
         EXCEPTION
             WHEN unique_violation THEN
                 -- Continue and try with another "name".
-                RAISE WARNING 'unique_violation';
                 NULL;
         END;
     END LOOP;
@@ -76,17 +80,22 @@ END;
 -- Insert a record in "ark" table from a "naan" value and a "base" identifier
 -- using a qualifier.
 CREATE OR REPLACE
-FUNCTION gen_qualified_ark(naan_ INTEGER, base TEXT, len INTEGER)
+FUNCTION gen_qualified_ark(naan_ INTEGER, base TEXT, len INTEGER, maxit INTEGER)
 RETURNS TEXT AS $$
 DECLARE
     ark_qualifier TEXT;
+    nit INTEGER;
 BEGIN
     PERFORM 1 FROM ark WHERE ark.name = base AND ark.qualifier = '';
     IF NOT FOUND THEN
         RAISE 'no ark record matching "%/%" found', naan_, base
         USING ERRCODE = 'invalid_parameter_value';
     END IF;
+    nit := 0;
     LOOP
+        ASSERT nit < maxit,
+            'ark generation loop exceeded maximum number of iterations: ' || maxit;
+        nit := nit + 1;
         BEGIN
             INSERT INTO ark
                 VALUES (naan_, base, gen_ark_part(len, '', ''))
@@ -95,7 +104,6 @@ BEGIN
         EXCEPTION
             WHEN unique_violation THEN
                 -- Continue and try with another "name".
-                RAISE WARNING 'unique_violation';
                 NULL;
         END;
     END LOOP;
diff --git a/test/test_ark.py b/test/test_ark.py
--- a/test/test_ark.py
+++ b/test/test_ark.py
@@ -18,6 +18,7 @@
 from unittest import TestCase
 
 import psycopg2
+import psycopg2.errorcodes
 
 from cubicweb.devtools import (
     PostgresApptestConfiguration,
@@ -115,8 +116,8 @@ class ArkServiceTC(testlib.CubicWebTC):
         with self.connect() as conn:
             with conn.cursor() as cu:
                 for _ in range(n):
-                    cu.execute("SELECT * from gen_ark(%s, %s, %s, %s);",
-                               (0, 4, 'rf', 'x'))
+                    cu.execute("SELECT * from gen_ark(%s, %s, %s, %s, %s);",
+                               (0, 4, 'rf', 'x', 10))
                     arks.append(cu.fetchone()[0])
                     conn.commit()
                 cu.execute("SELECT * from ark")
@@ -124,6 +125,33 @@ class ArkServiceTC(testlib.CubicWebTC):
         self.assertEqual(len(set(r)), n, len(r))
         self.assertCountEqual([ark for _, ark, _ in r], arks)
 
+    def test_gen_ark_maxit(self):
+        with self.connect() as conn:
+            with conn.cursor() as cu:
+                with self.assertRaises(psycopg2.InternalError) as cm:
+                    cu.execute("SELECT * from gen_ark(%s, %s, %s, %s, %s);",
+                               (0, 4, 'rf', 'x', 0))
+                exc = cm.exception
+                self.assertEqual(psycopg2.errorcodes.lookup(exc.pgcode),
+                                 'ASSERT_FAILURE')
+                self.assertIn('maximum number of iterations: 0',
+                              str(exc))
+
+    def test_gen_qualified_ark_maxit(self):
+        with self.connect() as conn:
+            with conn.cursor() as cu:
+                cu.execute("INSERT INTO ark VALUES (%s, %s, DEFAULT);",
+                           (0, "parent", ))
+                conn.commit()
+                with self.assertRaises(psycopg2.InternalError) as cm:
+                    cu.execute("SELECT * from gen_qualified_ark(%s, %s, %s, %s);",
+                               (0, "parent", 7, 0))
+                exc = cm.exception
+                self.assertEqual(psycopg2.errorcodes.lookup(exc.pgcode),
+                                 'ASSERT_FAILURE')
+                self.assertIn('maximum number of iterations: 0',
+                              str(exc))
+
     def test_qualifier(self):
         def insert(qualifier=None):
             with self.connect() as conn:
@@ -156,7 +184,7 @@ class ArkServiceTC(testlib.CubicWebTC):
         with self.connect() as conn:
             with conn.cursor() as cu:
                 with self.assertRaises(psycopg2.DataError) as cm:
-                    cu.execute("SELECT * FROM gen_qualified_ark(%s, %s, %s);",
+                    cu.execute("SELECT * FROM gen_qualified_ark(%s, %s, %s, 20);",
                                (1, "doesnotexist", 3))
         self.assertIn('no ark record matching "1/doesnotexist" found',
                       str(cm.exception))
@@ -166,7 +194,7 @@ class ArkServiceTC(testlib.CubicWebTC):
             with conn.cursor() as cu:
                 cu.execute("INSERT INTO ark VALUES (%s, %s, DEFAULT);",
                            (12345, "exists", ))
-                cu.execute("SELECT * FROM gen_qualified_ark(%s, %s, %s);",
+                cu.execute("SELECT * FROM gen_qualified_ark(%s, %s, %s, 20);",
                            (12345, "exists", 3))
                 cu.execute("SELECT * FROM ark"
                            " WHERE ark.naan = 12345 AND ark.name = 'exists'"



More information about the saem-devel mailing list