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
--- 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()