Skip to content

Commit 22e8dea

Browse files
committed
New conformance fixes
Removes the string format library in favor of cel-cpp's standard implementation. This causes some tests to be fixed, but it also causes some new failures due to non-standard string formatting behavior in protovalidate. This will be fixed on the next protovalidate release, so we should probably release that and then update this PR with it before merging.
1 parent 0aada05 commit 22e8dea

File tree

8 files changed

+17
-1128
lines changed

8 files changed

+17
-1128
lines changed

buf/validate/conformance/expected_failures.yaml

+9-603
Large diffs are not rendered by default.

buf/validate/internal/BUILD.bazel

+1-19
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ cc_library(
109109
"@com_github_bufbuild_protovalidate//proto/protovalidate/buf/validate:validate_proto_cc",
110110
"@com_google_cel_cpp//eval/public:activation",
111111
"@com_google_cel_cpp//eval/public:builtin_func_registrar",
112+
"@com_google_cel_cpp//eval/public:string_extension_func_registrar",
112113
"@com_google_cel_cpp//eval/public:cel_expr_builder_factory",
113114
"@com_google_cel_cpp//eval/public:cel_expression",
114115
"@com_google_cel_cpp//eval/public/containers:field_access",
@@ -132,7 +133,6 @@ cc_library(
132133
srcs = ["extra_func.cc"],
133134
hdrs = ["extra_func.h"],
134135
deps = [
135-
":string_format",
136136
"//buf/validate/internal/lib:ipv4",
137137
"//buf/validate/internal/lib:ipv6",
138138
"//buf/validate/internal/lib:uri",
@@ -144,24 +144,6 @@ cc_library(
144144
],
145145
)
146146

147-
cc_library(
148-
name = "string_format",
149-
srcs = ["string_format.cc"],
150-
hdrs = ["string_format.h"],
151-
deps = [
152-
"@com_google_cel_cpp//eval/public:cel_value",
153-
],
154-
)
155-
156-
cc_test(
157-
name = "string_format_test",
158-
srcs = ["string_format_test.cc"],
159-
deps = [
160-
":string_format",
161-
"@com_google_googletest//:gtest_main",
162-
],
163-
)
164-
165147
cc_test(
166148
name = "extra_func_test",
167149
srcs = ["extra_func_test.cc"],

buf/validate/internal/constraints.cc

+5
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "eval/public/containers/field_access.h"
2323
#include "eval/public/containers/field_backed_list_impl.h"
2424
#include "eval/public/containers/field_backed_map_impl.h"
25+
#include "eval/public/string_extension_func_registrar.h"
2526
#include "eval/public/structs/cel_proto_wrapper.h"
2627
#include "google/protobuf/dynamic_message.h"
2728
#include "google/protobuf/util/message_differencer.h"
@@ -105,6 +106,10 @@ NewConstraintBuilder(google::protobuf::Arena* arena) {
105106
if (!register_status.ok()) {
106107
return register_status;
107108
}
109+
register_status = cel::runtime::RegisterStringExtensionFunctions(builder->GetRegistry());
110+
if (!register_status.ok()) {
111+
return register_status;
112+
}
108113
register_status = RegisterExtraFuncs(*builder->GetRegistry(), arena);
109114
if (!register_status.ok()) {
110115
return register_status;

buf/validate/internal/extra_func.cc

+1-15
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
#include "buf/validate/internal/lib/ipv4.h"
2323
#include "buf/validate/internal/lib/ipv6.h"
2424
#include "buf/validate/internal/lib/uri.h"
25-
#include "buf/validate/internal/string_format.h"
2625
#include "eval/public/cel_function_adapter.h"
2726
#include "eval/public/cel_value.h"
2827
#include "eval/public/containers/container_backed_map_impl.h"
@@ -230,7 +229,7 @@ bool IsHostAndPort(const std::string_view str, const bool portRequired) {
230229

231230
const auto splitIdx = str.rfind(':');
232231
if (str.front() == '[') {
233-
const auto end = str.find(']');
232+
const auto end = str.rfind(']');
234233
const auto afterEnd = end + 1;
235234
if (afterEnd == str.size()) { // no port
236235
const auto host = str.substr(1, end - 1);
@@ -339,19 +338,6 @@ cel::CelValue isUriRef(google::protobuf::Arena* arena, cel::CelValue::StringHold
339338

340339
absl::Status RegisterExtraFuncs(
341340
google::api::expr::runtime::CelFunctionRegistry& registry, google::protobuf::Arena* regArena) {
342-
auto* formatter = google::protobuf::Arena::Create<StringFormat>(regArena);
343-
auto status = cel::FunctionAdapter<cel::CelValue, cel::CelValue::StringHolder, cel::CelValue>::
344-
CreateAndRegister(
345-
"format",
346-
true,
347-
[formatter](
348-
google::protobuf::Arena* arena,
349-
cel::CelValue::StringHolder format,
350-
cel::CelValue arg) -> cel::CelValue { return formatter->format(arena, format, arg); },
351-
&registry);
352-
if (!status.ok()) {
353-
return status;
354-
}
355341
auto isNanStatus = cel::FunctionAdapter<cel::CelValue, cel::CelValue>::CreateAndRegister(
356342
"isNan", true, &isNan, &registry);
357343
if (!isNanStatus.ok()) {

buf/validate/internal/extra_func_test.cc

+1
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,5 @@ TEST(ExtraFuncTest, TestIpPrefix) {
5656
// Additional Tests for IsHostAndPort()
5757
EXPECT_FALSE(buf::validate::internal::IsHostAndPort("example.com:00", false));
5858
EXPECT_FALSE(buf::validate::internal::IsHostAndPort("example.com:080", false));
59+
EXPECT_TRUE(buf::validate::internal::IsHostAndPort("[::0%00]]", false));
5960
}

0 commit comments

Comments
 (0)