diff --git a/.bazelrc b/.bazelrc index d472694..f826dd3 100644 --- a/.bazelrc +++ b/.bazelrc @@ -1,6 +1,10 @@ build --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 build --cxxopt=-fsized-deallocation +# Ensure we use the protobuf compiler from deps +build --proto_compiler=@com_google_protobuf//:protoc +build --proto_toolchain_for_cc=@com_google_protobuf//:cc_toolchain + # Enable matchers in googletest build --define absl=1 diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 98d7ac2..ae03ee8 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -14,7 +14,7 @@ jobs: test: strategy: matrix: - os: [ubuntu-latest, macos-latest] + os: [ubuntu-24.04, macos-latest] name: Unit tests runs-on: ${{ matrix.os }} steps: diff --git a/.github/workflows/conformance.yaml b/.github/workflows/conformance.yaml index 4bc5fd9..272dd0f 100644 --- a/.github/workflows/conformance.yaml +++ b/.github/workflows/conformance.yaml @@ -13,7 +13,7 @@ permissions: jobs: conformance: name: Conformance Testing - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 steps: - name: Setup Cache uses: actions/cache@v3 diff --git a/Makefile b/Makefile index 0a6ebb0..92aa713 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,7 @@ COPYRIGHT_YEARS := 2023 LICENSE_IGNORE := -e internal/testdata/ LICENSE_HEADER_VERSION := 0294fdbe1ce8649ebaf5e87e8cdd588e33730bbb # NOTE: Keep this version in sync with the version in `/bazel/deps.bzl`. -PROTOVALIDATE_VERSION ?= v0.5.6 +PROTOVALIDATE_VERSION ?= v0.7.1 # Set to use a different compiler. For example, `GO=go1.18rc1 make test`. GO ?= go diff --git a/WORKSPACE b/WORKSPACE index fcba728..e15b00f 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -4,6 +4,10 @@ load("//bazel:deps.bzl", "protovalidate_cc_dependencies") protovalidate_cc_dependencies() +load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps") + +protobuf_deps() + load("@rules_buf//buf:repositories.bzl", "rules_buf_dependencies", "rules_buf_toolchains") rules_buf_dependencies() @@ -23,7 +27,7 @@ switched_rules_by_language( cc = True, ) -load("@com_github_protocolbuffers_protobuf//:protobuf_deps.bzl", "protobuf_deps") +load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps") protobuf_deps() diff --git a/bazel/cel_cpp.patch b/bazel/cel_cpp.patch index 1803fc1..ff81545 100644 --- a/bazel/cel_cpp.patch +++ b/bazel/cel_cpp.patch @@ -1,5 +1,35 @@ +diff --git a/base/memory_manager.cc b/base/memory_manager.cc +index 1b7b3550..13b4ff97 100644 +--- a/base/memory_manager.cc ++++ b/base/memory_manager.cc +@@ -45,6 +45,7 @@ + #include "absl/base/config.h" + #include "absl/base/dynamic_annotations.h" + #include "absl/base/macros.h" ++#include "absl/base/optimization.h" + #include "absl/base/thread_annotations.h" + #include "absl/numeric/bits.h" + #include "absl/synchronization/mutex.h" +@@ -234,7 +235,7 @@ class GlobalMemoryManager final : public MemoryManager { + void* Allocate(size_t size, size_t align) override { + static_cast(size); + static_cast(align); +- ABSL_INTERNAL_UNREACHABLE; ++ ABSL_UNREACHABLE(); + return nullptr; + } + +@@ -242,7 +243,7 @@ class GlobalMemoryManager final : public MemoryManager { + void OwnDestructor(void* pointer, void (*destructor)(void*)) override { + static_cast(pointer); + static_cast(destructor); +- ABSL_INTERNAL_UNREACHABLE; ++ ABSL_UNREACHABLE(); + } + }; + diff --git a/eval/eval/container_access_step.cc b/eval/eval/container_access_step.cc -index 39c2507..d7962a2 100644 +index 39c2507d..d7962a29 100644 --- a/eval/eval/container_access_step.cc +++ b/eval/eval/container_access_step.cc @@ -11,6 +11,8 @@ @@ -8,9 +38,9 @@ index 39c2507..d7962a2 100644 #include "eval/public/unknown_attribute_set.h" +#include "eval/public/structs/legacy_type_adapter.h" +#include "eval/public/structs/legacy_type_info_apis.h" - + namespace google::api::expr::runtime { - + @@ -34,6 +36,8 @@ class ContainerAccessStep : public ExpressionStepBase { ExecutionFrame* frame) const; CelValue LookupInList(const CelList* cel_list, const CelValue& key, @@ -18,12 +48,12 @@ index 39c2507..d7962a2 100644 + CelValue LookupInMessage(const CelValue::MessageWrapper& msg, const CelValue& key, + ExecutionFrame* frame) const; }; - + inline CelValue ContainerAccessStep::LookupInMap(const CelMap* cel_map, @@ -109,6 +113,26 @@ inline CelValue ContainerAccessStep::LookupInList(const CelList* cel_list, CelValue::TypeName(key.type()))); } - + +CelValue ContainerAccessStep::LookupInMessage(const CelValue::MessageWrapper& msg, const CelValue& key, + ExecutionFrame* frame) const { + if (!key.IsString()) { diff --git a/bazel/deps.bzl b/bazel/deps.bzl index c889320..5b6780a 100644 --- a/bazel/deps.bzl +++ b/bazel/deps.bzl @@ -16,6 +16,15 @@ load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe") load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") _dependencies = { + # This is needed due to an unresolved issue with protobuf v27+. + # https://github.com/protocolbuffers/protobuf/issues/17200 + "rules_python": { + "sha256": "0a8003b044294d7840ac7d9d73eef05d6ceb682d7516781a4ec62eeb34702578", + "strip_prefix": "rules_python-0.24.0", + "urls": [ + "https://github.com/bazelbuild/rules_python/releases/download/0.24.0/rules_python-0.24.0.tar.gz", + ], + }, "bazel_skylib": { "sha256": "74d544d96f4a5bb630d465ca8bbcfe231e3594e5aae57e1edbf17a6eb3ca2506", "urls": [ @@ -23,12 +32,12 @@ _dependencies = { "https://mirror.bazel.build/github.com/bazelbuild/bazel-skylib/releases/download/1.3.0/bazel-skylib-1.3.0.tar.gz", ], }, - "com_github_protocolbuffers_protobuf": { - "sha256": "75be42bd736f4df6d702a0e4e4d30de9ee40eac024c4b845d17ae4cc831fe4ae", - "strip_prefix": "protobuf-21.7", + "com_google_protobuf": { + "sha256": "e4ff2aeb767da6f4f52485c2e72468960ddfe5262483879ef6ad552e52757a77", + "strip_prefix": "protobuf-27.2", "urls": [ - "https://mirror.bazel.build/github.com/protocolbuffers/protobuf/archive/v21.7.tar.gz", - "https://github.com/protocolbuffers/protobuf/archive/v21.7.tar.gz", + "https://mirror.bazel.build/github.com/protocolbuffers/protobuf/archive/v27.2.tar.gz", + "https://github.com/protocolbuffers/protobuf/archive/v27.2.tar.gz", ], }, "rules_proto": { @@ -56,10 +65,10 @@ _dependencies = { }, # NOTE: Keep Version in sync with `/Makefile`. "com_github_bufbuild_protovalidate": { - "sha256": "a6fd142c780c82104198138d609bace9b1b145c99e265aa33de1f651e90047d8", - "strip_prefix": "protovalidate-0.5.6", + "sha256": "ccb3952c38397d2cb53fe841af66b05fc012dd17fa754cbe35d9abb547cdf92d", + "strip_prefix": "protovalidate-0.7.1", "urls": [ - "https://github.com/bufbuild/protovalidate/archive/v0.5.6.tar.gz", + "https://github.com/bufbuild/protovalidate/archive/v0.7.1.tar.gz", ], }, } diff --git a/buf/validate/internal/constraints.cc b/buf/validate/internal/constraints.cc index 51b96ea..75ef886 100644 --- a/buf/validate/internal/constraints.cc +++ b/buf/validate/internal/constraints.cc @@ -27,9 +27,70 @@ #include "eval/public/structs/cel_proto_wrapper.h" #include "google/protobuf/any.pb.h" #include "google/protobuf/descriptor.pb.h" +#include "google/protobuf/dynamic_message.h" +#include "google/protobuf/util/message_differencer.h" namespace buf::validate::internal { namespace cel = google::api::expr; +namespace { + +bool isEmptyItem(cel::runtime::CelValue item) { + switch (item.type()) { + case ::cel::Kind::kBool: + return !item.BoolOrDie(); + case ::cel::Kind::kInt: + return item.Int64OrDie() == 0; + case ::cel::Kind::kUint: + return item.Uint64OrDie() == 0; + case ::cel::Kind::kDouble: + return item.DoubleOrDie() == 0; + case ::cel::Kind::kString: + return item.StringOrDie().value().empty(); + case ::cel::Kind::kBytes: + return item.BytesOrDie().value().empty(); + default: + return false; + } +} + +bool isDefaultItem(cel::runtime::CelValue item, const google::protobuf::FieldDescriptor *field) { + using google::protobuf::FieldDescriptor; + using google::protobuf::DynamicMessageFactory; + using google::protobuf::util::MessageDifferencer; + switch (field->cpp_type()) { + case FieldDescriptor::CPPTYPE_INT32: + return item.IsInt64() && item.Int64OrDie() == field->default_value_int32(); + case FieldDescriptor::CPPTYPE_INT64: + return item.IsInt64() && item.Int64OrDie() == field->default_value_int64(); + case FieldDescriptor::CPPTYPE_UINT32: + return item.IsUint64() && item.Uint64OrDie() == field->default_value_uint32(); + case FieldDescriptor::CPPTYPE_UINT64: + return item.IsUint64() && item.Uint64OrDie() == field->default_value_uint64(); + case FieldDescriptor::CPPTYPE_DOUBLE: + return item.IsDouble() && item.DoubleOrDie() == field->default_value_double(); + case FieldDescriptor::CPPTYPE_FLOAT: + return item.IsDouble() && item.DoubleOrDie() == field->default_value_float(); + case FieldDescriptor::CPPTYPE_BOOL: + return item.IsBool() && item.BoolOrDie() == field->default_value_bool(); + case FieldDescriptor::CPPTYPE_ENUM: + return item.IsInt64() && item.Int64OrDie() == field->default_value_enum()->number(); + case FieldDescriptor::CPPTYPE_STRING: + return item.IsString() && item.StringOrDie().value() == field->default_value_string(); + case FieldDescriptor::CPPTYPE_MESSAGE: + if (item.IsMessage()) { + DynamicMessageFactory dmf; + const auto *message = item.MessageOrDie(); + auto* empty = dmf.GetPrototype(message->GetDescriptor())->New(); + return MessageDifferencer::Equals(*message, *empty); + } + break; + default: + break; + } + return false; +} + +} absl::StatusOr> NewConstraintBuilder(google::protobuf::Arena* arena) { @@ -121,6 +182,10 @@ absl::Status FieldConstraintRules::Validate( if (!status.ok()) { return status; } + + if (ignoreDefault_ && isDefaultItem(result, field_)) { + return absl::OkStatus(); + } } activation.InsertValue("this", result); return ValidateCel(ctx, field_->name(), activation); @@ -143,25 +208,6 @@ absl::Status EnumConstraintRules::Validate( return absl::OkStatus(); } -bool isEmptyItem(cel::runtime::CelValue item) { - switch (item.type()) { - case ::cel::Kind::kBool: - return !item.BoolOrDie(); - case ::cel::Kind::kInt: - return item.Int64OrDie() == 0; - case ::cel::Kind::kUint: - return item.Uint64OrDie() == 0; - case ::cel::Kind::kDouble: - return item.DoubleOrDie() == 0; - case ::cel::Kind::kString: - return item.StringOrDie().value() == ""; - case ::cel::Kind::kBytes: - return item.BytesOrDie().value() == ""; - default: - return false; - } -} - absl::Status RepeatedConstraintRules::Validate( ConstraintContext& ctx, const google::protobuf::Message& message) const { auto status = Base::Validate(ctx, message); @@ -176,6 +222,9 @@ absl::Status RepeatedConstraintRules::Validate( if (itemRules_->getIgnoreEmpty() && isEmptyItem(item)) { continue; } + if (itemRules_->getIgnoreDefault() && isDefaultItem(item, field_)) { + continue; + } cel::runtime::Activation activation; activation.InsertValue("this", item); int pos = ctx.violations.violations_size(); diff --git a/buf/validate/internal/constraints.h b/buf/validate/internal/constraints.h index b68cc77..b960f6b 100644 --- a/buf/validate/internal/constraints.h +++ b/buf/validate/internal/constraints.h @@ -40,7 +40,13 @@ class FieldConstraintRules : public CelConstraintRules { const FieldConstraints& field, const AnyRules* anyRules = nullptr) : field_(desc), - ignoreEmpty_(field.ignore_empty() || desc->has_presence()), + mapEntryField_(desc->containing_type()->options().map_entry()), + ignoreEmpty_(field.ignore() == IGNORE_IF_DEFAULT_VALUE || + field.ignore() == IGNORE_IF_UNPOPULATED || + field.ignore_empty() || + (desc->has_presence() && !mapEntryField_)), + ignoreDefault_(field.ignore() == IGNORE_IF_DEFAULT_VALUE && + (desc->has_presence() && !mapEntryField_)), required_(field.required()), anyRules_(anyRules) {} @@ -56,11 +62,15 @@ class FieldConstraintRules : public CelConstraintRules { [[nodiscard]] const AnyRules* getAnyRules() const { return anyRules_; } - [[nodiscard]] const bool getIgnoreEmpty() const { return ignoreEmpty_; } + [[nodiscard]] bool getIgnoreEmpty() const { return ignoreEmpty_; } + + [[nodiscard]] bool getIgnoreDefault() const { return ignoreDefault_; } protected: const google::protobuf::FieldDescriptor* field_ = nullptr; + bool mapEntryField_ = false; bool ignoreEmpty_ = false; + bool ignoreDefault_ = false; bool required_ = false; const AnyRules* anyRules_ = nullptr; }; diff --git a/buf/validate/internal/extra_func.cc b/buf/validate/internal/extra_func.cc index ee0bef6..f137bfc 100644 --- a/buf/validate/internal/extra_func.cc +++ b/buf/validate/internal/extra_func.cc @@ -189,6 +189,13 @@ cel::CelValue isEmail(google::protobuf::Arena* arena, cel::CelValue::StringHolde return cel::CelValue::CreateBool(false); } + // Based on https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address + // Note that we are currently _stricter_ than this as we enforce length limits + static const re2::RE2 localPart_regex("^[a-zA-Z0-9.!#$%&'*+\\/=?^_`{|}~-]+$"); + if (!re2::RE2::FullMatch(localPart, localPart_regex)) { + return cel::CelValue::CreateBool(false); + } + // Validate the hostname return cel::CelValue::CreateBool(IsHostname(domainPart)); } diff --git a/buf/validate/internal/field_rules.cc b/buf/validate/internal/field_rules.cc index e52a394..ab43ca5 100644 --- a/buf/validate/internal/field_rules.cc +++ b/buf/validate/internal/field_rules.cc @@ -23,7 +23,7 @@ absl::StatusOr> NewFieldRules( google::api::expr::runtime::CelExpressionBuilder& builder, const google::protobuf::FieldDescriptor* field, const FieldConstraints& fieldLvl) { - if (fieldLvl.skipped()) { + if (fieldLvl.ignore() == IGNORE_ALWAYS || fieldLvl.skipped()) { return nullptr; } absl::StatusOr> rules_or; diff --git a/buf/validate/validator.cc b/buf/validate/validator.cc index c569c6a..2bb0852 100644 --- a/buf/validate/validator.cc +++ b/buf/validate/validator.cc @@ -46,9 +46,12 @@ absl::Status Validator::ValidateFields( } if (field->options().HasExtension(validate::field)) { const auto& fieldExt = field->options().GetExtension(validate::field); - if (fieldExt.skipped() || - (fieldExt.has_repeated() && fieldExt.repeated().items().skipped()) || - (fieldExt.has_map() && fieldExt.map().values().skipped())) { + if (fieldExt.ignore() == IGNORE_ALWAYS || + fieldExt.skipped() || + (fieldExt.has_repeated() && (fieldExt.repeated().items().ignore() == IGNORE_ALWAYS || + fieldExt.repeated().items().skipped())) || + (fieldExt.has_map() && (fieldExt.map().values().ignore() == IGNORE_ALWAYS || + fieldExt.map().values().skipped()))) { continue; } }