Merge pull request #1544 from mgreter/bugfix/protect-sass-value-op-fr…

…om-exceptions Protect `sass_value_op` from thrown C++ exceptions
sass · Sep 9, 2015 · 681b511 · 681b511
2 parents 74e3021 + a172416
commit 681b511
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 42 deletions.
diff --git a/src/sass_context.cpp b/src/sass_context.cpp
@@ -246,7 +246,7 @@ extern "C" {
       c_ctx->source_map_string = 0;
       json_delete(json_err);
     }
-    catch(std::bad_alloc& ba) {
+    catch (std::bad_alloc& ba) {
       std::stringstream msg_stream;
       JsonNode* json_err = json_mkobject();
       msg_stream << "Unable to allocate memory: " << ba.what() << std::endl;

diff --git a/src/sass_values.cpp b/src/sass_values.cpp
@@ -359,51 +359,66 @@ extern "C" {
 
     Sass::Value* rv = 0;
     Memory_Manager<AST_Node> mem;
-    Value* lhs = sass_value_to_ast_node(mem, a);
-    Value* rhs = sass_value_to_ast_node(mem, b);
-
-    // see if it's a relational expression
-    switch(op) {
-      case Sass_OP::EQ:  return sass_make_boolean(Eval::eq(lhs, rhs));
-      case Sass_OP::NEQ: return sass_make_boolean(!Eval::eq(lhs, rhs));
-      case Sass_OP::GT:  return sass_make_boolean(!Eval::lt(lhs, rhs) && !Eval::eq(lhs, rhs));
-      case Sass_OP::GTE: return sass_make_boolean(!Eval::lt(lhs, rhs));
-      case Sass_OP::LT:  return sass_make_boolean(Eval::lt(lhs, rhs));
-      case Sass_OP::LTE: return sass_make_boolean(Eval::lt(lhs, rhs) || Eval::eq(lhs, rhs));
-      default:           break;
-    }
 
-    if (sass_value_is_number(a) && sass_value_is_number(b)) {
-      const Number* l_n = dynamic_cast<const Number*>(lhs);
-      const Number* r_n = dynamic_cast<const Number*>(rhs);
-      rv = Eval::op_numbers(mem, op, *l_n, *r_n);
-    }
-    else if (sass_value_is_number(a) && sass_value_is_color(a)) {
-      const Number* l_n = dynamic_cast<const Number*>(lhs);
-      const Color* r_c = dynamic_cast<const Color*>(rhs);
-      rv = Eval::op_number_color(mem, op, *l_n, *r_c);
-    }
-    else if (sass_value_is_color(a) && sass_value_is_number(b)) {
-      const Color* l_c = dynamic_cast<const Color*>(lhs);
-      const Number* r_n = dynamic_cast<const Number*>(rhs);
-      rv = Eval::op_color_number(mem, op, *l_c, *r_n);
-    }
-    else if (sass_value_is_color(a) && sass_value_is_color(b)) {
-      const Color* l_c = dynamic_cast<const Color*>(lhs);
-      const Color* r_c = dynamic_cast<const Color*>(rhs);
-      rv = Eval::op_colors(mem, op, *l_c, *r_c);
-    }
-    else /* convert other stuff to string and apply operation */ {
-      Value* l_v = dynamic_cast<Value*>(lhs);
-      Value* r_v = dynamic_cast<Value*>(rhs);
-      rv = Eval::op_strings(mem, op, *l_v, *r_v);
+    try {
+
+      Value* lhs = sass_value_to_ast_node(mem, a);
+      Value* rhs = sass_value_to_ast_node(mem, b);
+
+      // see if it's a relational expression
+      switch(op) {
+        case Sass_OP::EQ:  return sass_make_boolean(Eval::eq(lhs, rhs));
+        case Sass_OP::NEQ: return sass_make_boolean(!Eval::eq(lhs, rhs));
+        case Sass_OP::GT:  return sass_make_boolean(!Eval::lt(lhs, rhs) && !Eval::eq(lhs, rhs));
+        case Sass_OP::GTE: return sass_make_boolean(!Eval::lt(lhs, rhs));
+        case Sass_OP::LT:  return sass_make_boolean(Eval::lt(lhs, rhs));
+        case Sass_OP::LTE: return sass_make_boolean(Eval::lt(lhs, rhs) || Eval::eq(lhs, rhs));
+        default:           break;
+      }
+
+      if (sass_value_is_number(a) && sass_value_is_number(b)) {
+        const Number* l_n = dynamic_cast<const Number*>(lhs);
+        const Number* r_n = dynamic_cast<const Number*>(rhs);
+        rv = Eval::op_numbers(mem, op, *l_n, *r_n);
+      }
+      else if (sass_value_is_number(a) && sass_value_is_color(a)) {
+        const Number* l_n = dynamic_cast<const Number*>(lhs);
+        const Color* r_c = dynamic_cast<const Color*>(rhs);
+        rv = Eval::op_number_color(mem, op, *l_n, *r_c);
+      }
+      else if (sass_value_is_color(a) && sass_value_is_number(b)) {
+        const Color* l_c = dynamic_cast<const Color*>(lhs);
+        const Number* r_n = dynamic_cast<const Number*>(rhs);
+        rv = Eval::op_color_number(mem, op, *l_c, *r_n);
+      }
+      else if (sass_value_is_color(a) && sass_value_is_color(b)) {
+        const Color* l_c = dynamic_cast<const Color*>(lhs);
+        const Color* r_c = dynamic_cast<const Color*>(rhs);
+        rv = Eval::op_colors(mem, op, *l_c, *r_c);
+      }
+      else /* convert other stuff to string and apply operation */ {
+        Value* l_v = dynamic_cast<Value*>(lhs);
+        Value* r_v = dynamic_cast<Value*>(rhs);
+        rv = Eval::op_strings(mem, op, *l_v, *r_v);
+      }
+
+      // ToDo: maybe we should should return null value?
+      if (!rv) return sass_make_error("invalid return value");
+
+      // convert result back to ast node
+      return ast_node_to_sass_value(rv);
+
     }
 
-    // ToDo: maybe we should should return null value?
-    if (!rv) return sass_make_error("invalid return value");
+    // simply pass the error message back to the caller for now
+    catch (Error_Invalid& e) { return sass_make_error(e.message.c_str()); }
+    catch (std::bad_alloc& ba) { return sass_make_error("memory exhausted"); }
+    catch (std::exception& e) { return sass_make_error(e.what()); }
+    catch (std::string& e) { return sass_make_error(e.c_str()); }
+    catch (const char* e) { return sass_make_error(e); }
+    catch (...) { return sass_make_error("unknown"); }
 
-    // convert result back to ast node
-    return ast_node_to_sass_value(rv);
+    return 0;
 
   }