[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:35:08 CET 2018


Le mercredi 28 février 2018 à 10:32 +0100, Denis Laxalde a écrit :
> # 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.

This patch is actually already applied (and released), but I'd welcome
any feedback. Thanks.

> 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.pgco
> de),
> +                                 '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.pgco
> de),
> +                                 '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