From a51b6912bd28489a784edbd8bd1f7e11437ab7e1 Mon Sep 17 00:00:00 2001
From: Vo Minh Thu <vmt@openerp.com>
Date: Wed, 20 Mar 2013 14:22:38 +0100
Subject: [PATCH] [FIX] safe_eval: better exception handling:

- the re-raising of exceptions ignored the "legal" exceptions
(the one used to early abort RPC calls and generate pop-ups)
- re-raising the exception was attempting to re-use the original
exception type but it does not always take only one argument
so it was breaking.

lp bug: https://launchpad.net/bugs/1146256 fixed

bzr revid: vmt@openerp.com-20130320132238-qzo3jptww59ndlch
---
 openerp/addons/base/test/base_test.yml        |  2 +-
 .../tests/addons/test_exceptions/models.py    | 30 +++++++++++++
 openerp/tests/addons/test_exceptions/view.xml | 42 ++++++++++++++++++-
 openerp/tools/safe_eval.py                    | 14 ++++++-
 4 files changed, 84 insertions(+), 4 deletions(-)

diff --git a/openerp/addons/base/test/base_test.yml b/openerp/addons/base/test/base_test.yml
index b7f7e0058b1..baf8aa88815 100644
--- a/openerp/addons/base/test/base_test.yml
+++ b/openerp/addons/base/test/base_test.yml
@@ -47,7 +47,7 @@
             with mute_logger('openerp.tools.safe_eval'):
                 safe_eval('open("/etc/passwd","r")')
             assert False, "safe_eval should not allow calling open() builtin"
-        except NameError:
+        except ValueError:
             pass
 
 -
diff --git a/openerp/tests/addons/test_exceptions/models.py b/openerp/tests/addons/test_exceptions/models.py
index 7a91d147256..a22c6ea7d77 100644
--- a/openerp/tests/addons/test_exceptions/models.py
+++ b/openerp/tests/addons/test_exceptions/models.py
@@ -35,4 +35,34 @@ class m(openerp.osv.osv.Model):
     def generate_undefined(self, cr, uid, ids, context=None):
         self.surely_undefined_symbol
 
+
+    def generate_except_osv_safe_eval(self, cr, uid, ids, context=None):
+        self.generate_safe_eval(cr, uid, ids, self.generate_except_osv, context)
+
+    def generate_except_orm_safe_eval(self, cr, uid, ids, context=None):
+        self.generate_safe_eval(cr, uid, ids, self.generate_except_orm, context)
+
+    def generate_warning_safe_eval(self, cr, uid, ids, context=None):
+        self.generate_safe_eval(cr, uid, ids, self.generate_warning, context)
+
+    def generate_redirect_warning_safe_eval(self, cr, uid, ids, context=None):
+        self.generate_safe_eval(cr, uid, ids, self.generate_redirect_warning, context)
+
+    def generate_access_denied_safe_eval(self, cr, uid, ids, context=None):
+        self.generate_safe_eval(cr, uid, ids, self.generate_access_denied, context)
+
+    def generate_access_error_safe_eval(self, cr, uid, ids, context=None):
+        self.generate_safe_eval(cr, uid, ids, self.generate_access_error, context)
+
+    def generate_exc_access_denied_safe_eval(self, cr, uid, ids, context=None):
+        self.generate_safe_eval(cr, uid, ids, self.generate_exc_access_denied, context)
+
+    def generate_undefined_safe_eval(self, cr, uid, ids, context=None):
+        self.generate_safe_eval(cr, uid, ids, self.generate_undefined, context)
+
+
+    def generate_safe_eval(self, cr, uid, ids, f, context):
+        globals_dict = { 'generate': lambda *args: f(cr, uid, ids, context) }
+        openerp.tools.safe_eval.safe_eval("generate()", mode='exec', globals_dict=globals_dict)
+
 # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:
diff --git a/openerp/tests/addons/test_exceptions/view.xml b/openerp/tests/addons/test_exceptions/view.xml
index 0c8e373a278..4d8e820d456 100644
--- a/openerp/tests/addons/test_exceptions/view.xml
+++ b/openerp/tests/addons/test_exceptions/view.xml
@@ -7,9 +7,11 @@
             <field name="model">test.exceptions.model</field>
             <field name="arch" type="xml">
                 <form string="Test exceptions">
-                      <label string="Each button generates a specific exception on the server. The text on the right is the expected representation of the exception when displayed on the client."/>
+                    <label string="Each button generates a specific exception on the server. The text on the right is the expected representation of the exception when displayed on the client. Button marked with a '*' use safe_eval()."/>
+                    <separator string="" colspan="8"/>
+                    <group colspan="8" col="8">
+                    <group colspan="4" col="8">
                       <group colspan="8" col="8">
-                          <separator string="" colspan="8"/>
                           <button name="generate_except_osv" string="except_osv" type="object" icon="gtk-ok" colspan="1"/>
                           <label string="Warning-description"/>
                       </group>
@@ -41,6 +43,42 @@
                           <button name="generate_undefined" string="Undefined" type="object" icon="gtk-ok" colspan="1"/>
                           <label string="Server error-traceback"/>
                       </group>
+                    </group>
+                    <group colspan="4" col="8">
+                      <group colspan="8" col="8">
+                          <button name="generate_except_osv_safe_eval" string="except_osv*" type="object" icon="gtk-ok" colspan="1"/>
+                          <label string="Warning-description"/>
+                      </group>
+                      <group colspan="8" col="8">
+                          <button name="generate_except_orm_safe_eval" string="except_orm*" type="object" icon="gtk-ok" colspan="1"/>
+                          <label string="Warning-description"/>
+                      </group>
+                      <group colspan="8" col="8">
+                          <button name="generate_warning_safe_eval" string="Warning*" type="object" icon="gtk-ok" colspan="1"/>
+                          <label string="Warning-description"/>
+                      </group>
+                      <group colspan="8" col="8">
+                          <button name="generate_redirect_warning_safe_eval" string="RedirectWarning*" type="object" icon="gtk-ok" colspan="1"/>
+                          <label string="Warning-description-redirection button"/>
+                      </group>
+                      <group colspan="8" col="8">
+                          <button name="generate_access_denied_safe_eval" string="AccessDenied*" type="object" icon="gtk-ok" colspan="1"/>
+                          <label string="Access denied-traceback"/>
+                      </group>
+                      <group colspan="8" col="8">
+                          <button name="generate_access_error_safe_eval" string="AccessError*" type="object" icon="gtk-ok" colspan="1"/>
+                          <label string="Access rights error-description"/>
+                      </group>
+                      <group colspan="8" col="8">
+                          <button name="generate_exc_access_denied_safe_eval" string="Exc AccessDenied*" type="object" icon="gtk-ok" colspan="1"/>
+                          <label string="Access denied-traceback"/>
+                      </group>
+                      <group colspan="8" col="8">
+                          <button name="generate_undefined_safe_eval" string="Undefined*" type="object" icon="gtk-ok" colspan="1"/>
+                          <label string="Server error-traceback"/>
+                      </group>
+                    </group>
+                    </group>
                 </form>
            </field>
         </record>
diff --git a/openerp/tools/safe_eval.py b/openerp/tools/safe_eval.py
index 49244d92e81..dda6b935060 100644
--- a/openerp/tools/safe_eval.py
+++ b/openerp/tools/safe_eval.py
@@ -37,6 +37,8 @@ import logging
 
 from .misc import ustr
 
+import openerp
+
 __all__ = ['test_expr', 'safe_eval', 'const_eval']
 
 # The time module is usually already provided in the safe_eval environment
@@ -242,9 +244,19 @@ def safe_eval(expr, globals_dict=None, locals_dict=None, mode="eval", nocopy=Fal
     c = test_expr(expr, _SAFE_OPCODES, mode=mode)
     try:
         return eval(c, globals_dict, locals_dict)
+    except openerp.osv.orm.except_orm:
+        raise
+    except openerp.exceptions.Warning:
+        raise
+    except openerp.exceptions.RedirectWarning:
+        raise
+    except openerp.exceptions.AccessDenied:
+        raise
+    except openerp.exceptions.AccessError:
+        raise
     except Exception, e:
         import sys
         exc_info = sys.exc_info()
-        raise exc_info[0], '"%s" while evaluating\n%r' % (ustr(e), expr), exc_info[2]
+        raise ValueError, '"%s" while evaluating\n%r' % (ustr(e), expr), exc_info[2]
 
 # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: