diff --git a/common/escaping.cc b/common/escaping.cc index e75fba0af..b7f8b16c4 100644 --- a/common/escaping.cc +++ b/common/escaping.cc @@ -266,7 +266,7 @@ absl::optional unescape(const std::string& s, bool is_bytes) { // Raw string preceded by the 'r|R' prefix. bool is_raw_literal = false; if (value[0] == 'r' || value[0] == 'R') { - value.resize(value.size() - 1); + value = value.substr(1, n - 1); n = value.size(); is_raw_literal = true; } diff --git a/common/escaping_test.cc b/common/escaping_test.cc index 8428632c4..85cdc7f8b 100644 --- a/common/escaping_test.cc +++ b/common/escaping_test.cc @@ -35,6 +35,7 @@ std::vector test_cases = { {R"("\\")", "\\"}, {"'''x''x'''", "x''x"}, {R"("""x""x""")", R"(x""x)"}, + {R"(r"")", ""}, // Octal 303 -> Code point 195 (Ã) // Octal 277 -> Code point 191 (¿) {R"("\303\277")", "ÿ"}, diff --git a/conformance/BUILD b/conformance/BUILD index 518c33907..2a402ceb0 100644 --- a/conformance/BUILD +++ b/conformance/BUILD @@ -38,11 +38,13 @@ cc_binary( "//eval/eval:container_backed_map_impl", "//eval/public:builtin_func_registrar", "//eval/public:cel_expr_builder_factory", + "//parser", "@com_github_grpc_grpc//:grpc++", "@com_google_absl//absl/strings", "@com_google_googleapis//google/api/expr/v1alpha1:checked_cc_proto", "@com_google_googleapis//google/api/expr/v1alpha1:conformance_service_cc_grpc", "@com_google_googleapis//google/api/expr/v1alpha1:syntax_cc_proto", + "@com_google_googleapis//google/rpc:code_cc_proto", "@com_google_protobuf//:protobuf", ], ) @@ -53,8 +55,8 @@ cc_binary( srcs = [":" + driver], args = [ "$(location @com_google_cel_spec//tests/simple:simple_test)", - "--server=$(location @com_google_cel_go//server/main:cel_server)", - "--eval_server=$(location :server)", + "--server=$(location :server)", + "--check_server=$(location @com_google_cel_go//server/main:cel_server)", # Requires container support "--skip_test=basic/namespace/self_eval_container_lookup,self_eval_container_lookup_unchecked", # Requires heteregenous equality spec clarification @@ -83,8 +85,8 @@ sh_test( srcs = ["@com_google_cel_spec//tests:conftest-nofail.sh"], args = [ "$(location @com_google_cel_spec//tests/simple:simple_test)", - "--server=$(location @com_google_cel_go//server/main:cel_server)", - "--eval_server=$(location :server)", + "--server=$(location :server)", + "--check_server=$(location @com_google_cel_go//server/main:cel_server)", ] + ["$(location " + test + ")" for test in DASHBOARD_TESTS], data = [ ":server", diff --git a/conformance/server.cc b/conformance/server.cc index 39031bff0..111765446 100644 --- a/conformance/server.cc +++ b/conformance/server.cc @@ -5,12 +5,14 @@ #include "google/api/expr/v1alpha1/syntax.pb.h" #include "google/api/expr/v1alpha1/value.pb.h" #include "google/protobuf/struct.pb.h" +#include "google/rpc/code.pb.h" #include "grpcpp/grpcpp.h" #include "absl/strings/str_split.h" #include "eval/eval/container_backed_list_impl.h" #include "eval/eval/container_backed_map_impl.h" #include "eval/public/builtin_func_registrar.h" #include "eval/public/cel_expr_builder_factory.h" +#include "parser/parser.h" #include "base/statusor.h" @@ -190,12 +192,25 @@ class ConformanceServiceImpl final Status Parse(grpc::ServerContext* context, const v1alpha1::ParseRequest* request, v1alpha1::ParseResponse* response) override { - return Status(StatusCode::UNIMPLEMENTED, "only Eval is supported"); + if (request->cel_source().empty()) { + return Status(StatusCode::INVALID_ARGUMENT, "No source code."); + } + auto parse_status = parser::Parse(request->cel_source(), ""); + if (!parse_status.ok()) { + auto issue = response->add_issues(); + *issue->mutable_message() = std::string(parse_status.status().message()); + issue->set_code(google::rpc::Code::INVALID_ARGUMENT); + } else { + google::api::expr::v1alpha1::ParsedExpr out; + out = parse_status.ValueOrDie(); + response->mutable_parsed_expr()->CopyFrom(out); + } + return Status::OK; } Status Check(grpc::ServerContext* context, const v1alpha1::CheckRequest* request, v1alpha1::CheckResponse* response) override { - return Status(StatusCode::UNIMPLEMENTED, "only Eval is supported"); + return Status(StatusCode::UNIMPLEMENTED, "Check is not supported"); } Status Eval(grpc::ServerContext* context, const v1alpha1::EvalRequest* request, diff --git a/eval/eval/evaluator_core.cc b/eval/eval/evaluator_core.cc index 60500a397..5e0579aa4 100644 --- a/eval/eval/evaluator_core.cc +++ b/eval/eval/evaluator_core.cc @@ -18,12 +18,12 @@ const ExpressionStep* ExecutionFrame::Next() { } cel_base::StatusOr CelExpressionFlatImpl::Evaluate( - const Activation& activation, google::protobuf::Arena* arena) const { + const BaseActivation& activation, google::protobuf::Arena* arena) const { return Trace(activation, arena, CelEvaluationListener()); } cel_base::StatusOr CelExpressionFlatImpl::Trace( - const Activation& activation, google::protobuf::Arena* arena, + const BaseActivation& activation, google::protobuf::Arena* arena, CelEvaluationListener callback) const { ExecutionFrame frame(&path_, activation, arena, max_iterations_); diff --git a/eval/eval/evaluator_core.h b/eval/eval/evaluator_core.h index 87f2cbf70..50d2d4c0d 100644 --- a/eval/eval/evaluator_core.h +++ b/eval/eval/evaluator_core.h @@ -95,7 +95,7 @@ class ExecutionFrame { // flat is the flattened sequence of execution steps that will be evaluated. // activation provides bindings between parameter names and values. // arena serves as allocation manager during the expression evaluation. - ExecutionFrame(const ExecutionPath* flat, const Activation& activation, + ExecutionFrame(const ExecutionPath* flat, const BaseActivation& activation, google::protobuf::Arena* arena, int max_iterations) : pc_(0UL), execution_path_(flat), @@ -129,7 +129,7 @@ class ExecutionFrame { google::protobuf::Arena* arena() { return arena_; } // Returns reference to Activation - const Activation& activation() const { return activation_; } + const BaseActivation& activation() const { return activation_; } // Returns reference to iter_vars std::map& iter_vars() { return iter_vars_; } @@ -151,7 +151,7 @@ class ExecutionFrame { private: size_t pc_; // pc_ - Program Counter. Current position on execution path. const ExecutionPath* execution_path_; - const Activation& activation_; + const BaseActivation& activation_; ValueStack value_stack_; google::protobuf::Arena* arena_; const int max_iterations_; @@ -173,11 +173,11 @@ class CelExpressionFlatImpl : public CelExpression { : path_(std::move(path)), max_iterations_(max_iterations) {} // Implementation of CelExpression evaluate method. - cel_base::StatusOr Evaluate(const Activation& activation, + cel_base::StatusOr Evaluate(const BaseActivation& activation, google::protobuf::Arena* arena) const override; // Implementation of CelExpression trace method. - cel_base::StatusOr Trace(const Activation& activation, + cel_base::StatusOr Trace(const BaseActivation& activation, google::protobuf::Arena* arena, CelEvaluationListener callback) const override; diff --git a/eval/eval/function_step.cc b/eval/eval/function_step.cc index 55b5355cf..d0afdbc7b 100644 --- a/eval/eval/function_step.cc +++ b/eval/eval/function_step.cc @@ -149,7 +149,7 @@ cel_base::StatusOr LazyFunctionStep::ResolveFunction( CelFunctionDescriptor matcher{name_, receiver_style_, arg_types}; - const Activation& activation = frame->activation(); + const BaseActivation& activation = frame->activation(); for (auto provider : providers_) { auto status = provider->GetFunction(matcher, activation); if (!status.ok()) { diff --git a/eval/public/activation.h b/eval/public/activation.h index 1e8c01f66..7d38f0ba8 100644 --- a/eval/public/activation.h +++ b/eval/public/activation.h @@ -17,10 +17,35 @@ namespace api { namespace expr { namespace runtime { +// Base class for an activation. +class BaseActivation { + public: + BaseActivation() = default; + + // Non-copyable/non-assignable + BaseActivation(const BaseActivation&) = delete; + BaseActivation& operator=(const BaseActivation&) = delete; + + // Return a list of function overloads for the given name. + virtual std::vector FindFunctionOverloads( + absl::string_view) const = 0; + + // Provide the value that is bound to the name, if found. + // arena parameter is provided to support the case when we want to pass the + // ownership of returned object ( Message/List/Map ) to Evaluator. + virtual absl::optional FindValue(absl::string_view, + google::protobuf::Arena*) const = 0; + + // Check whether a select path is unknown. + virtual bool IsPathUnknown(absl::string_view) const = 0; + + virtual ~BaseActivation() {} +}; + // Instance of Activation class is used by evaluator. // It provides binding between references used in expressions // and actual values. -class Activation { +class Activation : public BaseActivation { public: Activation() = default; @@ -28,15 +53,17 @@ class Activation { Activation(const Activation&) = delete; Activation& operator=(const Activation&) = delete; - // Return list of function overloads for the given name. + // BaseActivation std::vector FindFunctionOverloads( - absl::string_view name) const; + absl::string_view name) const override; - // Provide value that is bound to the name, if found. - // arena parameter is provided to support the case when we want to pass the - // ownership of returned object ( Message/List/Map ) to Evaluator. absl::optional FindValue(absl::string_view name, - google::protobuf::Arena* arena) const; + google::protobuf::Arena* arena) const override; + + bool IsPathUnknown(absl::string_view path) const override { + return google::protobuf::util::FieldMaskUtil::IsPathInFieldMask(std::string(path), + unknown_paths_); + } // Insert a function into the activation (ie a lazily bound function). Returns // a status if the name and shape of the function matches another one that has @@ -75,12 +102,6 @@ class Activation { return unknown_paths_; } - // Check whether select path is unknown. - bool IsPathUnknown(absl::string_view path) const { - return google::protobuf::util::FieldMaskUtil::IsPathInFieldMask(std::string(path), - unknown_paths_); - } - private: class ValueEntry { public: diff --git a/eval/public/cel_expression.h b/eval/public/cel_expression.h index 9f740ff69..74815369e 100644 --- a/eval/public/cel_expression.h +++ b/eval/public/cel_expression.h @@ -34,12 +34,12 @@ class CelExpression { // activation contains bindings from parameter names to values // arena parameter specifies Arena object where output result and // internal data will be allocated. - virtual cel_base::StatusOr Evaluate(const Activation& activation, + virtual cel_base::StatusOr Evaluate(const BaseActivation& activation, google::protobuf::Arena* arena) const = 0; // Trace evaluates expression calling the callback on each sub-tree. virtual cel_base::StatusOr Trace( - const Activation& activation, google::protobuf::Arena* arena, + const BaseActivation& activation, google::protobuf::Arena* arena, CelEvaluationListener callback) const = 0; }; diff --git a/eval/public/cel_function_provider.cc b/eval/public/cel_function_provider.cc index f14f4dbb1..42ce62e34 100644 --- a/eval/public/cel_function_provider.cc +++ b/eval/public/cel_function_provider.cc @@ -13,7 +13,7 @@ class ActivationFunctionProviderImpl : public CelFunctionProvider { ActivationFunctionProviderImpl() {} cel_base::StatusOr GetFunction( const CelFunctionDescriptor& descriptor, - const Activation& activation) const override { + const BaseActivation& activation) const override { std::vector overloads = activation.FindFunctionOverloads(descriptor.name()); diff --git a/eval/public/cel_function_provider.h b/eval/public/cel_function_provider.h index a397c11b8..e8537dc39 100644 --- a/eval/public/cel_function_provider.h +++ b/eval/public/cel_function_provider.h @@ -20,7 +20,7 @@ class CelFunctionProvider { // nullptr is interpreted as no funtion overload matches the descriptor. virtual cel_base::StatusOr GetFunction( const CelFunctionDescriptor& descriptor, - const Activation& activation) const = 0; + const BaseActivation& activation) const = 0; virtual ~CelFunctionProvider() {} }; diff --git a/eval/public/cel_function_registry_test.cc b/eval/public/cel_function_registry_test.cc index c4f6b3ec3..2a9297a24 100644 --- a/eval/public/cel_function_registry_test.cc +++ b/eval/public/cel_function_registry_test.cc @@ -23,7 +23,7 @@ class NullLazyFunctionProvider : public virtual CelFunctionProvider { // Just return nullptr indicating no matching function. cel_base::StatusOr GetFunction( const CelFunctionDescriptor& desc, - const Activation& activation) const override { + const BaseActivation& activation) const override { return nullptr; } };