# HG changeset patch # User Sylvain Thénault # Date 1258024519 -3600 # Node ID 0c0c051863cb416d39d293bf5b500eceecb1e5db # Parent b54c8d664dd64a0baf16b6aaf4c7b8ca14676000 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 diff -r b54c8d664dd6 -r 0c0c051863cb entity.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 diff -r b54c8d664dd6 -r 0c0c051863cb rqlrewrite.py --- 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: diff -r b54c8d664dd6 -r 0c0c051863cb schema.py --- 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): diff -r b54c8d664dd6 -r 0c0c051863cb test/unittest_entity.py --- 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') diff -r b54c8d664dd6 -r 0c0c051863cb test/unittest_rqlrewrite.py --- 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()