diff --git a/openerp-server b/openerp-server index c86400be6d6..835ff228e9b 100755 --- a/openerp-server +++ b/openerp-server @@ -88,8 +88,11 @@ def setup_pid_file(): def preload_registry(dbname): """ Preload a registry, and start the cron.""" - db, pool = openerp.pooler.get_db_and_pool(dbname, update_module=config['init'] or config['update'], pooljobs=False) - pool.get('ir.cron').restart(db.dbname) + try: + db, pool = openerp.pooler.get_db_and_pool(dbname, update_module=config['init'] or config['update'], pooljobs=False) + pool.get('ir.cron').restart(db.dbname) + except Exception: + logging.exception('Failed to initialize database `%s`.', dbname) def run_test_file(dbname, test_file): """ Preload a registry, possibly run a test file, and start the cron.""" diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index 3e9a0f6fb3e..43fea5c16cf 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -471,4 +471,33 @@ - !python {model: res.country }: | ids = self.search(cr, uid, [('create_date', '<', '2001-01-01 12:00:00')]) + + - + Verify that invalid expressions are refused, even for magic fields +- + !python {model: res.country }: | + try: + self.search(cr, uid, [('does_not_exist', '=', 'foo')]) + raise AssertionError('Invalid fields should not be accepted') + except ValueError: + pass + + try: + self.search(cr, uid, [('create_date', '>>', 'foo')]) + raise AssertionError('Invalid operators should not be accepted') + except ValueError: + pass + + import psycopg2 + try: + cr._default_log_exceptions = False + cr.execute('SAVEPOINT expression_failure_test') + self.search(cr, uid, [('create_date', '=', "1970-01-01'); --")]) + # if the above search gives no error, the operand was not escaped! + cr.execute('RELEASE SAVEPOINT expression_failure_test') + raise AssertionError('Operands should always be SQL escaped') + except psycopg2.DataError: + # Should give: 'DataError: invalid input syntax for type timestamp' or similar + cr.execute('ROLLBACK TO SAVEPOINT expression_failure_test') + diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 272c1945d25..ff2528db73d 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -126,6 +126,7 @@ import logging from openerp.tools import flatten, reverse_enumerate import fields import openerp.modules +from openerp.osv.orm import MAGIC_COLUMNS #.apidoc title: Domain Expressions @@ -413,7 +414,7 @@ class expression(object): # check if the expression is valid if not is_leaf(e): - raise ValueError('Bad domain expression: %r, %r is not a valid term.' % (exp, e)) + raise ValueError("Invalid term %r in domain expression %r" % (e, exp)) # normalize the leaf's operator e = normalize_leaf(*e) @@ -421,41 +422,47 @@ class expression(object): left, operator, right = e working_table = table # The table containing the field (the name provided in the left operand) - fargs = left.split('.', 1) + field_path = left.split('.', 1) # If the field is _inherits'd, search for the working_table, # and extract the field. - if fargs[0] in table._inherit_fields: + if field_path[0] in table._inherit_fields: while True: - field = working_table._columns.get(fargs[0]) + field = working_table._columns.get(field_path[0]) if field: self.__field_tables[i] = working_table break - next_table = working_table.pool.get(working_table._inherit_fields[fargs[0]][0]) + next_table = working_table.pool.get(working_table._inherit_fields[field_path[0]][0]) if next_table not in self.__all_tables: self.__joins.append('%s."%s"=%s."%s"' % (next_table._table, 'id', working_table._table, working_table._inherits[next_table._name])) self.__all_tables.add(next_table) working_table = next_table # Or (try to) directly extract the field. else: - field = working_table._columns.get(fargs[0]) + field = working_table._columns.get(field_path[0]) if not field: if left == 'id' and operator == 'child_of': ids2 = to_ids(right, table) dom = child_of_domain(left, ids2, working_table) self.__exp = self.__exp[:i] + dom + self.__exp[i+1:] + else: + # field could not be found in model columns, it's probably invalid, unless + # it's one of the _log_access special fields + # TODO: make these fields explicitly available in self.columns instead! + if (field_path[0] not in MAGIC_COLUMNS) and (left not in MAGIC_COLUMNS): + raise ValueError("Invalid field %r in domain expression %r" % (left, exp)) continue field_obj = table.pool.get(field._obj) - if len(fargs) > 1: + if len(field_path) > 1: if field._type == 'many2one': - right = field_obj.search(cr, uid, [(fargs[1], operator, right)], context=context) - self.__exp[i] = (fargs[0], 'in', right) + right = field_obj.search(cr, uid, [(field_path[1], operator, right)], context=context) + self.__exp[i] = (field_path[0], 'in', right) # Making search easier when there is a left operand as field.o2m or field.m2m if field._type in ['many2many', 'one2many']: - right = field_obj.search(cr, uid, [(fargs[1], operator, right)], context=context) - right1 = table.search(cr, uid, [(fargs[0], 'in', right)], context=context) + right = field_obj.search(cr, uid, [(field_path[1], operator, right)], context=context) + right1 = table.search(cr, uid, [(field_path[0], 'in', right)], context=context) self.__exp[i] = ('id', 'in', right1) if not isinstance(field, fields.property): @@ -657,6 +664,12 @@ class expression(object): def __leaf_to_sql(self, leaf, table): left, operator, right = leaf + # final sanity checks - should never fail + assert operator in (TERM_OPERATORS + ('inselect',)), \ + "Invalid operator %r in domain term %r" % (operator, leaf) + assert leaf in (TRUE_LEAF, FALSE_LEAF) or left in table._all_columns \ + or left in MAGIC_COLUMNS, "Invalid field %r in domain term %r" % (left, leaf) + if leaf == TRUE_LEAF: query = 'TRUE' params = [] @@ -704,8 +717,8 @@ class expression(object): query = '(%s OR %s."%s" IS NULL)' % (query, table._table, left) elif check_nulls and operator == 'not in': query = '(%s AND %s."%s" IS NOT NULL)' % (query, table._table, left) # needed only for TRUE. - else: # Must not happen. - pass + else: # Must not happen + raise ValueError("Invalid domain term %r" % (leaf,)) elif right == False and (left in table._columns) and table._columns[left]._type=="boolean" and (operator == '='): query = '(%s."%s" IS NULL or %s."%s" = false )' % (table._table, left, table._table, left) @@ -725,15 +738,12 @@ class expression(object): elif (operator == '=?'): if (right is False or right is None): + # '=?' is a short-circuit that makes the term TRUE if right is None or False query = 'TRUE' params = [] - elif left in table._columns: - format = table._columns[left]._symbol_set[0] - query = '(%s."%s" = %s)' % (table._table, left, format) - params = table._columns[left]._symbol_set[1](right) else: - query = "(%s.\"%s\" = '%%s')" % (table._table, left) - params = right + # '=?' behaves like '=' in other cases + query, params = self.__leaf_to_sql((left, '=', right), table) elif left == 'id': query = '%s.id %s %%s' % (table._table, operator) @@ -749,15 +759,11 @@ class expression(object): query = '(unaccent(%s."%s") %s unaccent(%s))' % (table._table, left, sql_operator, format) else: query = '(%s."%s" %s %s)' % (table._table, left, sql_operator, format) - else: - # Ugly case to support columns present in database but not in - # _columns. This will probably be removed in the future, but - # we have to keep it for now. - _logger.warning("The domain term '%s' specify a column not declared in _columns." % ((left, operator, right),)) - if self.has_unaccent and sql_operator in ('ilike', 'not ilike'): - query = "(unaccent(%s.\"%s\") %s unaccent('%s'))" % (table._table, left, sql_operator, right) - else: - query = "(%s.\"%s\" %s '%s')" % (table._table, left, sql_operator, right) + elif left in MAGIC_COLUMNS: + query = "(%s.\"%s\" %s %%s)" % (table._table, left, sql_operator) + params = right + else: # Must not happen + raise ValueError("Invalid field %r in domain term %r" % (left, leaf)) add_null = False if need_wildcard: @@ -771,10 +777,6 @@ class expression(object): add_null = not str_utf8 elif left in table._columns: params = table._columns[left]._symbol_set[1](right) - else: - # Matching else clause for the above else clause, where the - # params are actually already in the query. - params = [] if add_null: query = '(%s OR %s."%s" IS NULL)' % (query, table._table, left) diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index 5f19ddf2378..37dd1010d2f 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -638,6 +638,7 @@ class orm_template(object): _log_create = False CONCURRENCY_CHECK_FIELD = '__last_update' + def log(self, cr, uid, id, message, secondary=False, context=None): if context and context.get('disable_log'): return True @@ -2546,12 +2547,25 @@ class orm_memory(orm_template): for id in ids: self._check_access(uid, id, operation) +# Definition of log access columns, automatically added to models if +# self._log_access is True +LOG_ACCESS_COLUMNS = { + 'create_uid': 'INTEGER REFERENCES res_users ON DELETE SET NULL', + 'create_date': 'TIMESTAMP', + 'write_uid': 'INTEGER REFERENCES res_users ON DELETE SET NULL', + 'write_date': 'TIMESTAMP' +} +# special columns automatically created by the ORM +MAGIC_COLUMNS = ['id'] + LOG_ACCESS_COLUMNS.keys() + \ + ['internal.create_uid', 'internal.date_access'] # for osv_memory only + class orm(orm_template): _sql_constraints = [] _table = None _protected = ['read', 'write', 'create', 'default_get', 'perm_read', 'unlink', 'fields_get', 'fields_view_get', 'search', 'name_get', 'distinct_field_get', 'name_search', 'copy', 'import_data', 'search_count', 'exists'] __logger = logging.getLogger('orm') __schema = logging.getLogger('orm.schema') + def read_group(self, cr, uid, domain, fields, groupby, offset=0, limit=None, context=None, orderby=False): """ Get the list of records in list view grouped by the given ``groupby`` fields @@ -2780,7 +2794,7 @@ class orm(orm_template): # iterate on the database columns to drop the NOT NULL constraints # of fields which were required but have been removed (or will be added by another module) columns = [c for c in self._columns if not (isinstance(self._columns[c], fields.function) and not self._columns[c].store)] - columns += ('id', 'write_uid', 'write_date', 'create_uid', 'create_date') # openerp access columns + columns += MAGIC_COLUMNS cr.execute("SELECT a.attname, a.attnotnull" " FROM pg_class c, pg_attribute a" " WHERE c.relname=%s" @@ -2847,7 +2861,7 @@ class orm(orm_template): column_data = self._select_column_data(cr) for k, f in self._columns.iteritems(): - if k in ('id', 'write_uid', 'write_date', 'create_uid', 'create_date'): + if k in MAGIC_COLUMNS: continue # Don't update custom (also called manual) fields if f.manual and not update_custom_fields: @@ -3149,23 +3163,17 @@ class orm(orm_template): def _add_log_columns(self, cr): - logs = { - 'create_uid': 'INTEGER REFERENCES res_users ON DELETE SET NULL', - 'create_date': 'TIMESTAMP', - 'write_uid': 'INTEGER REFERENCES res_users ON DELETE SET NULL', - 'write_date': 'TIMESTAMP' - } - for k in logs: + for field, field_def in LOG_ACCESS_COLUMNS.iteritems(): cr.execute(""" SELECT c.relname FROM pg_class c, pg_attribute a WHERE c.relname=%s AND a.attname=%s AND c.oid=a.attrelid - """, (self._table, k)) + """, (self._table, field)) if not cr.rowcount: - cr.execute('ALTER TABLE "%s" ADD COLUMN "%s" %s' % (self._table, k, logs[k])) + cr.execute('ALTER TABLE "%s" ADD COLUMN "%s" %s' % (self._table, field, field_def)) cr.commit() self.__schema.debug("Table '%s': added column '%s' with definition=%s", - self._table, k, logs[k]) + self._table, field, field_def) def _select_column_data(self, cr): @@ -4641,7 +4649,7 @@ class orm(orm_template): for f in fields: ftype = fields[f]['type'] - if self._log_access and f in ('create_date', 'create_uid', 'write_date', 'write_uid'): + if self._log_access and f in LOG_ACCESS_COLUMNS: del data[f] if f in default: diff --git a/openerp/sql_db.py b/openerp/sql_db.py index 79606b9101c..8038c39486d 100644 --- a/openerp/sql_db.py +++ b/openerp/sql_db.py @@ -183,6 +183,8 @@ class Cursor(object): self.__caller = False self.__closer = False + self._default_log_exceptions = True + def __del__(self): if not self.__closed: # Oops. 'self' has not been closed explicitly. @@ -199,7 +201,7 @@ class Cursor(object): self._close(True) @check - def execute(self, query, params=None, log_exceptions=True): + def execute(self, query, params=None, log_exceptions=None): if '%d' in query or '%f' in query: self.__logger.warn(query) self.__logger.warn("SQL queries cannot contain %d or %f anymore. " @@ -212,11 +214,11 @@ class Cursor(object): params = params or None res = self._obj.execute(query, params) except psycopg2.ProgrammingError, pe: - if log_exceptions: + if self._default_log_exceptions or log_exceptions: self.__logger.error("Programming error: %s, in query %s", pe, query) raise except Exception: - if log_exceptions: + if self._default_log_exceptions or log_exceptions: self.__logger.exception("bad query: %s", self._obj.query or query) raise