close #511810: bad rql generated when looking for vocabulary for a relation on an entity which doesn't exist (yet) stable
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Thu, 12 Nov 2009 12:15:19 +0100
branchstable
changeset 3826 0c0c051863cb
parent 3825 b54c8d664dd6
child 3827 c7142a4e3470
close #511810: bad rql generated when looking for vocabulary for a relation on an entity which doesn't exist (yet) technical details: * add a graph of linked variables in the RRQLExpression instance * modify rql rewriter to remove relations where an unexistant variable and or a variable linked to an unexistant variable is used. * activate this feature in Entity.unrelated_rql if the entity doesn't exists yet
entity.py
rqlrewrite.py
schema.py
test/unittest_entity.py
test/unittest_rqlrewrite.py
--- a/entity.py	Thu Nov 12 12:08:43 2009 +0100
+++ b/entity.py	Thu Nov 12 12:15:19 2009 +0100
@@ -778,9 +778,13 @@
             rqlexprs = rtype.get_rqlexprs('add')
             rewriter = RQLRewriter(self.req)
             rqlst = self.req.vreg.parse(self.req, rql, args)
+            if not self.has_eid():
+                existant = searchedvar
+            else:
+                existant = None # instead of 'SO', improve perfs
             for select in rqlst.children:
                 rewriter.rewrite(select, [((searchedvar, searchedvar), rqlexprs)],
-                                 select.solutions, args)
+                                 select.solutions, args, existant)
             rql = rqlst.as_string()
         return rql, args
 
--- a/rqlrewrite.py	Thu Nov 12 12:08:43 2009 +0100
+++ b/rqlrewrite.py	Thu Nov 12 12:15:19 2009 +0100
@@ -13,6 +13,7 @@
 from rql import nodes as n, stmts, TypeResolverException
 
 from logilab.common.compat import any
+from logilab.common.graph import has_path
 
 from cubicweb import Unauthorized, typed_eid
 
@@ -134,7 +135,7 @@
         if len(self.select.solutions) < len(self.solutions):
             raise Unsupported()
 
-    def rewrite(self, select, snippets, solutions, kwargs):
+    def rewrite(self, select, snippets, solutions, kwargs, existingvars=None):
         """
         snippets: (varmap, list of rql expression)
                   with varmap a *tuple* (select var, snippet var)
@@ -146,6 +147,7 @@
         self.removing_ambiguity = False
         self.exists_snippet = {}
         self.pending_keys = []
+        self.existingvars = existingvars
         # we have to annotate the rqlst before inserting snippets, even though
         # we'll have to redo it latter
         self.annotate(select)
@@ -213,6 +215,14 @@
 
     def insert_snippet(self, varmap, snippetrqlst, parent=None):
         new = snippetrqlst.where.accept(self)
+        existing = self.existingvars
+        self.existingvars = None
+        try:
+            return self._insert_snippet(varmap, parent, new)
+        finally:
+            self.existingvars = existing
+
+    def _insert_snippet(self, varmap, parent, new):
         if new is not None:
             if self.varinfo.get('stinfo', {}).get('optrelations'):
                 assert parent is None
@@ -478,8 +488,26 @@
     def visit_exists(self, node):
         return self._visit_unary(node, n.Exists)
 
+    def keep_var(self, varname):
+        if varname in 'SO':
+            return varname in self.existingvars
+        if varname == 'U':
+            return True
+        vargraph = self.current_expr.vargraph
+        for existingvar in self.existingvars:
+            #path = has_path(vargraph, varname, existingvar)
+            if has_path(vargraph, varname, existingvar):
+                return True
+        # no path from this variable to an existing variable
+        return False
+
     def visit_relation(self, node):
         lhs, rhs = node.get_variable_parts()
+        # remove relations where an unexistant variable and or a variable linked
+        # to an unexistant variable is used.
+        if self.existingvars:
+            if not self.keep_var(lhs.name):
+                return
         if node.r_type in ('has_add_permission', 'has_update_permission',
                            'has_delete_permission', 'has_read_permission'):
             assert lhs.name == 'U'
@@ -488,6 +516,8 @@
             self.pending_keys.append( (key, action) )
             return
         if isinstance(rhs, n.VariableRef):
+            if self.existingvars and not self.keep_var(rhs.name):
+                return
             if lhs.name in self.revvarmap and rhs.name != 'U':
                 orel = self._may_be_shared_with(node, 'object', lhs.name)
                 if orel is not None:
--- a/schema.py	Thu Nov 12 12:08:43 2009 +0100
+++ b/schema.py	Thu Nov 12 12:15:19 2009 +0100
@@ -812,6 +812,17 @@
                 raise Exception('unable to guess selection variables')
             mainvars = ','.join(mainvars)
         RQLExpression.__init__(self, expression, mainvars, eid)
+        self.vargraph = {}
+        for relation in self.rqlst.get_nodes(nodes.Relation):
+            try:
+                rhsvarname = relation.children[1].children[0].variable.name
+                lhsvarname = relation.children[0].name
+            except AttributeError:
+                pass
+            else:
+                self.vargraph.setdefault(lhsvarname, []).append(rhsvarname)
+                self.vargraph.setdefault(rhsvarname, []).append(lhsvarname)
+                #self.vargraph[(lhsvarname, rhsvarname)] = relation.r_type
 
     @property
     def full_rql(self):
--- a/test/unittest_entity.py	Thu Nov 12 12:08:43 2009 +0100
+++ b/test/unittest_entity.py	Thu Nov 12 12:15:19 2009 +0100
@@ -239,6 +239,14 @@
         #rql = email.unrelated_rql('use_email', 'Person', 'object')[0]
         #self.assertEquals(rql, '')
 
+    def test_unrelated_rql_security_nonexistant(self):
+        self.login('anon')
+        email = self.vreg['etypes'].etype_class('EmailAddress')(self.request())
+        rql = email.unrelated_rql('use_email', 'CWUser', 'object')[0]
+        self.assertEquals(rql, 'Any S,AA,AB,AC,AD ORDERBY AA '
+                          'WHERE S is CWUser, S login AA, S firstname AB, S surname AC, S modification_date AD, '
+                          'A eid %(B)s, EXISTS(S identity A, NOT A in_group C, C name "guests", C is CWGroup)')
+
     def test_unrelated_base(self):
         p = self.add_entity('Personne', nom=u'di mascio', prenom=u'adrien')
         e = self.add_entity('Tag', name=u'x')
--- a/test/unittest_rqlrewrite.py	Thu Nov 12 12:08:43 2009 +0100
+++ b/test/unittest_rqlrewrite.py	Thu Nov 12 12:15:19 2009 +0100
@@ -11,6 +11,7 @@
 from rql import parse, nodes, RQLHelper
 
 from cubicweb import Unauthorized
+from cubicweb.schema import RRQLExpression
 from cubicweb.rqlrewrite import RQLRewriter
 from cubicweb.devtools import repotest, TestServerConfiguration
 
@@ -32,7 +33,7 @@
     return {1: 'CWUser',
             2: 'Card'}[eid]
 
-def rewrite(rqlst, snippets_map, kwargs):
+def rewrite(rqlst, snippets_map, kwargs, existingvars=None):
     class FakeVReg:
         schema = schema
         @staticmethod
@@ -47,12 +48,15 @@
                 rqlhelper.simplify(rqlst, needcopy)
     rewriter = RQLRewriter(mock_object(vreg=FakeVReg, user=(mock_object(eid=1))))
     for v, snippets in snippets_map.items():
-        snippets_map[v] = [mock_object(snippet_rqlst=parse('Any X WHERE '+snippet).children[0],
-                                       expression='Any X WHERE '+snippet)
+        snippets_map[v] = [isinstance(snippet, basestring)
+                           and mock_object(snippet_rqlst=parse('Any X WHERE '+snippet).children[0],
+                                           expression='Any X WHERE '+snippet)
+                           or snippet
                            for snippet in snippets]
     rqlhelper.compute_solutions(rqlst.children[0], {'eid': eid_func_map}, kwargs=kwargs)
     solutions = rqlst.children[0].solutions
-    rewriter.rewrite(rqlst.children[0], snippets_map.items(), solutions, kwargs)
+    rewriter.rewrite(rqlst.children[0], snippets_map.items(), solutions, kwargs,
+                     existingvars)
     test_vrefs(rqlst.children[0])
     return rewriter.rewritten
 
@@ -241,6 +245,61 @@
                              u"Any X,C WHERE X? documented_by C, C is Card WITH X BEING (Any X WHERE X concerne A, X is Affaire)")
 
 
+    def test_rrqlexpr_nonexistant_subject_1(self):
+        constraint = RRQLExpression('S owned_by U')
+        rqlst = parse('Card C')
+        rewrite(rqlst, {('C', 'S'): (constraint,)}, {}, 'SU')
+        self.failUnlessEqual(rqlst.as_string(),
+                             u"Any C WHERE C is Card, A eid %(B)s, EXISTS(C owned_by A)")
+        rqlst = parse('Card C')
+        rewrite(rqlst, {('C', 'S'): (constraint,)}, {}, 'OU')
+        self.failUnlessEqual(rqlst.as_string(),
+                             u"Any C WHERE C is Card")
+        rqlst = parse('Card C')
+        rewrite(rqlst, {('C', 'S'): (constraint,)}, {}, 'SOU')
+        self.failUnlessEqual(rqlst.as_string(),
+                             u"Any C WHERE C is Card, A eid %(B)s, EXISTS(C owned_by A)")
+
+    def test_rrqlexpr_nonexistant_subject_2(self):
+        constraint = RRQLExpression('S owned_by U, O owned_by U, O is Card')
+        rqlst = parse('Card C')
+        rewrite(rqlst, {('C', 'S'): (constraint,)}, {}, 'SU')
+        self.failUnlessEqual(rqlst.as_string(),
+                             'Any C WHERE C is Card, A eid %(B)s, EXISTS(C owned_by A)')
+        rqlst = parse('Card C')
+        rewrite(rqlst, {('C', 'S'): (constraint,)}, {}, 'OU')
+        self.failUnlessEqual(rqlst.as_string(),
+                             'Any C WHERE C is Card, B eid %(D)s, EXISTS(A owned_by B, A is Card)')
+        rqlst = parse('Card C')
+        rewrite(rqlst, {('C', 'S'): (constraint,)}, {}, 'SOU')
+        self.failUnlessEqual(rqlst.as_string(),
+                             'Any C WHERE C is Card, A eid %(B)s, EXISTS(C owned_by A, D owned_by A, D is Card)')
+
+    def test_rrqlexpr_nonexistant_subject_3(self):
+        constraint = RRQLExpression('U in_group G, G name "users"')
+        rqlst = parse('Card C')
+        rewrite(rqlst, {('C', 'S'): (constraint,)}, {}, 'SU')
+        self.failUnlessEqual(rqlst.as_string(),
+                             u'Any C WHERE C is Card, A eid %(B)s, EXISTS(A in_group D, D name "users", D is CWGroup)')
+
+    def test_rrqlexpr_nonexistant_subject_4(self):
+        constraint = RRQLExpression('U in_group G, G name "users", S owned_by U')
+        rqlst = parse('Card C')
+        rewrite(rqlst, {('C', 'S'): (constraint,)}, {}, 'SU')
+        self.failUnlessEqual(rqlst.as_string(),
+                             u'Any C WHERE C is Card, A eid %(B)s, EXISTS(A in_group D, D name "users", C owned_by A, D is CWGroup)')
+        rqlst = parse('Card C')
+        rewrite(rqlst, {('C', 'S'): (constraint,)}, {}, 'OU')
+        self.failUnlessEqual(rqlst.as_string(),
+                             u'Any C WHERE C is Card, A eid %(B)s, EXISTS(A in_group D, D name "users", D is CWGroup)')
+
+    def test_rrqlexpr_nonexistant_subject_5(self):
+        constraint = RRQLExpression('S owned_by Z, O owned_by Z, O is Card')
+        rqlst = parse('Card C')
+        rewrite(rqlst, {('C', 'S'): (constraint,)}, {}, 'S')
+        self.failUnlessEqual(rqlst.as_string(),
+                             u"Any C WHERE C is Card, EXISTS(C owned_by A, A is CWUser)")
+
 
 if __name__ == '__main__':
     unittest_main()