Skip to content

Commit 08cc263

Browse files
jnthntatumcopybara-github
authored andcommitted
Add range check and configurable limits for lists.range()
PiperOrigin-RevId: 956047173
1 parent 54c0512 commit 08cc263

4 files changed

Lines changed: 179 additions & 26 deletions

File tree

extensions/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,7 @@ cc_test(
470470
":lists_functions",
471471
"//checker:type_check_issue",
472472
"//checker:validation_result",
473+
"//common:ast",
473474
"//common:source",
474475
"//common:value",
475476
"//common:value_testing",

extensions/lists_functions.cc

Lines changed: 53 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ absl::Span<const cel::Type> SortableTypes() {
6868
return kTypes;
6969
}
7070

71+
constexpr int64_t kMaxRangeSize = 1000000;
72+
7173
// Slow distinct() implementation that uses Equal() to compare values in O(n^2).
7274
absl::Status ListDistinctHeterogeneousImpl(
7375
const ListValue& list,
@@ -223,10 +225,20 @@ absl::StatusOr<Value> ListFlatten(
223225
return std::move(*builder).Build();
224226
}
225227

226-
absl::StatusOr<ListValue> ListRange(
227-
int64_t end, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
228+
absl::StatusOr<Value> ListRange(
229+
int64_t end, int64_t max_range_size,
230+
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
228231
google::protobuf::MessageFactory* absl_nonnull message_factory,
229232
google::protobuf::Arena* absl_nonnull arena) {
233+
if (end < 0) {
234+
return ErrorValue(absl::InvalidArgumentError(absl::StrFormat(
235+
"lists.range: size must be non-negative, got %d", end)));
236+
}
237+
if (end > max_range_size) {
238+
return ErrorValue(absl::InvalidArgumentError(
239+
absl::StrFormat("lists.range: size %d exceeds maximum allowed (%d)",
240+
end, max_range_size)));
241+
}
230242
auto builder = NewListValueBuilder(arena);
231243
builder->Reserve(end);
232244
for (int64_t i = 0; i < end; ++i) {
@@ -512,11 +524,27 @@ absl::Status RegisterListFlattenFunction(FunctionRegistry& registry) {
512524
return absl::OkStatus();
513525
}
514526

515-
absl::Status RegisterListRangeFunction(FunctionRegistry& registry) {
516-
return UnaryFunctionAdapter<absl::StatusOr<Value>,
517-
int64_t>::RegisterGlobalOverload("lists.range",
518-
&ListRange,
519-
registry);
527+
absl::Status RegisterListRangeFunction(
528+
FunctionRegistry& registry,
529+
const ListsExtensionOptions& extension_options) {
530+
constexpr int64_t kMaxRangeSize = 1000000;
531+
int64_t effective_limit = kMaxRangeSize;
532+
if (extension_options.max_range_size > 0 &&
533+
extension_options.max_range_size < effective_limit) {
534+
effective_limit = extension_options.max_range_size;
535+
}
536+
return UnaryFunctionAdapter<absl::StatusOr<Value>, int64_t>::
537+
RegisterGlobalOverload(
538+
"lists.range",
539+
[effective_limit](
540+
int64_t end,
541+
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
542+
google::protobuf::MessageFactory* absl_nonnull message_factory,
543+
google::protobuf::Arena* absl_nonnull arena) -> absl::StatusOr<Value> {
544+
return ListRange(end, effective_limit, descriptor_pool,
545+
message_factory, arena);
546+
},
547+
registry);
520548
}
521549

522550
absl::Status RegisterListReverseFunction(FunctionRegistry& registry) {
@@ -657,23 +685,23 @@ absl::Status ConfigureParser(ParserBuilder& builder, int version) {
657685

658686
} // namespace
659687

660-
absl::Status RegisterListsFunctions(FunctionRegistry& registry,
661-
const RuntimeOptions& options,
662-
int version) {
688+
absl::Status RegisterListsFunctions(
689+
FunctionRegistry& registry, const RuntimeOptions& options,
690+
const ListsExtensionOptions& extension_options) {
663691
CEL_RETURN_IF_ERROR(RegisterListSliceFunction(registry));
664-
if (version == 0) {
692+
if (extension_options.version == 0) {
665693
return absl::OkStatus();
666694
}
667695

668696
// Since version 1
669697
CEL_RETURN_IF_ERROR(RegisterListFlattenFunction(registry));
670-
if (version == 1) {
698+
if (extension_options.version == 1) {
671699
return absl::OkStatus();
672700
}
673701

674702
// Since version 2
675703
CEL_RETURN_IF_ERROR(RegisterListDistinctFunction(registry));
676-
CEL_RETURN_IF_ERROR(RegisterListRangeFunction(registry));
704+
CEL_RETURN_IF_ERROR(RegisterListRangeFunction(registry, extension_options));
677705
CEL_RETURN_IF_ERROR(RegisterListReverseFunction(registry));
678706
CEL_RETURN_IF_ERROR(RegisterListSortFunction(registry));
679707
return absl::OkStatus();
@@ -684,18 +712,23 @@ absl::Status RegisterListsMacros(MacroRegistry& registry, const ParserOptions&,
684712
return registry.RegisterMacros(lists_macros(version));
685713
}
686714

687-
CheckerLibrary ListsCheckerLibrary(int version) {
715+
CheckerLibrary ListsCheckerLibrary(
716+
const ListsExtensionOptions& extension_options) {
688717
return {.id = "cel.lib.ext.lists",
689-
.configure = [version](TypeCheckerBuilder& builder) {
718+
.configure = [version = extension_options.version](
719+
TypeCheckerBuilder& builder) {
690720
return RegisterListsCheckerDecls(builder, version);
691721
}};
692722
}
693723

694-
CompilerLibrary ListsCompilerLibrary(int version) {
695-
auto lib = CompilerLibrary::FromCheckerLibrary(ListsCheckerLibrary(version));
696-
lib.configure_parser = [version](ParserBuilder& builder) {
697-
return ConfigureParser(builder, version);
698-
};
724+
CompilerLibrary ListsCompilerLibrary(
725+
const ListsExtensionOptions& extension_options) {
726+
auto lib = CompilerLibrary::FromCheckerLibrary(
727+
ListsCheckerLibrary(extension_options));
728+
lib.configure_parser =
729+
[version = extension_options.version](ParserBuilder& builder) {
730+
return ConfigureParser(builder, version);
731+
};
699732
return lib;
700733
}
701734

extensions/lists_functions.h

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
#ifndef THIRD_PARTY_CEL_CPP_EXTENSIONS_LISTS_FUNCTIONS_H_
1616
#define THIRD_PARTY_CEL_CPP_EXTENSIONS_LISTS_FUNCTIONS_H_
1717

18+
#include <cstdint>
19+
1820
#include "absl/status/status.h"
1921
#include "checker/type_checker_builder.h"
2022
#include "compiler/compiler.h"
@@ -27,6 +29,18 @@ namespace cel::extensions {
2729

2830
constexpr int kListsExtensionLatestVersion = 2;
2931

32+
struct ListsExtensionOptions {
33+
int version = kListsExtensionLatestVersion;
34+
35+
// Maximum size allowed for lists.range().
36+
// Setting a tighter limit (e.g. 100) will restrict the max size further.
37+
// A standard limit of 1,000,000 applies if a tighter limit isn't
38+
// configured.
39+
int64_t max_range_size = 1000000;
40+
};
41+
42+
using ListsFunctionsOptions = ListsExtensionOptions;
43+
3044
// Register implementations for list extension functions.
3145
//
3246
// === Since version 0 ===
@@ -45,9 +59,17 @@ constexpr int kListsExtensionLatestVersion = 2;
4559
//
4660
// <list(T)>.sort() -> list(T)
4761
//
48-
absl::Status RegisterListsFunctions(FunctionRegistry& registry,
49-
const RuntimeOptions& options,
50-
int version = kListsExtensionLatestVersion);
62+
absl::Status RegisterListsFunctions(
63+
FunctionRegistry& registry, const RuntimeOptions& options,
64+
const ListsExtensionOptions& extension_options = {});
65+
66+
inline absl::Status RegisterListsFunctions(FunctionRegistry& registry,
67+
const RuntimeOptions& options,
68+
int version) {
69+
ListsExtensionOptions extension_options;
70+
extension_options.version = version;
71+
return RegisterListsFunctions(registry, options, extension_options);
72+
}
5173

5274
// Register list macros.
5375
//
@@ -76,7 +98,14 @@ absl::Status RegisterListsMacros(MacroRegistry& registry,
7698
// <list(T)>.reverse() -> list(T)
7799
//
78100
// <list(T_)>.sort() -> list(T_) where T_ is partially orderable
79-
CheckerLibrary ListsCheckerLibrary(int version = kListsExtensionLatestVersion);
101+
CheckerLibrary ListsCheckerLibrary(
102+
const ListsExtensionOptions& extension_options = {});
103+
104+
inline CheckerLibrary ListsCheckerLibrary(int version) {
105+
ListsExtensionOptions extension_options;
106+
extension_options.version = version;
107+
return ListsCheckerLibrary(extension_options);
108+
}
80109

81110
// Provides decls for the following functions:
82111
//
@@ -96,8 +125,14 @@ CheckerLibrary ListsCheckerLibrary(int version = kListsExtensionLatestVersion);
96125
//
97126
// <list(T_)>.sort() -> list(T_) where T_ is partially orderable
98127
CompilerLibrary ListsCompilerLibrary(
99-
int version = kListsExtensionLatestVersion);
128+
const ListsExtensionOptions& extension_options = {});
129+
130+
inline CompilerLibrary ListsCompilerLibrary(int version) {
131+
ListsExtensionOptions extension_options;
132+
extension_options.version = version;
133+
return ListsCompilerLibrary(extension_options);
134+
}
100135

101136
} // namespace cel::extensions
102137

103-
#endif // THIRD_PARTY_CEL_CPP_EXTENSIONS_SETS_FUNCTIONS_H_
138+
#endif // THIRD_PARTY_CEL_CPP_EXTENSIONS_LISTS_FUNCTIONS_H_

extensions/lists_functions_test.cc

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "absl/strings/string_view.h"
2727
#include "checker/type_check_issue.h"
2828
#include "checker/validation_result.h"
29+
#include "common/ast.h"
2930
#include "common/source.h"
3031
#include "common/value.h"
3132
#include "common/value_testing.h"
@@ -123,6 +124,10 @@ INSTANTIATE_TEST_SUITE_P(
123124
// lists.range()
124125
{R"cel(lists.range(4) == [0,1,2,3])cel"},
125126
{R"cel(lists.range(0) == [])cel"},
127+
{R"cel(lists.range(-1))cel",
128+
"lists.range: size must be non-negative, got -1"},
129+
{R"cel(lists.range(1000001))cel",
130+
"lists.range: size 1000001 exceeds maximum allowed (1000000)"},
126131

127132
// .reverse()
128133
{R"cel([5,1,2,3].reverse() == [3,2,1,5])cel"},
@@ -457,5 +462,84 @@ std::vector<ListsExtensionVersionTestCase> CreateListsExtensionVersionParams() {
457462
INSTANTIATE_TEST_SUITE_P(ListsExtensionVersionTest, ListsExtensionVersionTest,
458463
ValuesIn(CreateListsExtensionVersionParams()));
459464

465+
TEST(ListsFunctionsTest, CustomMaxRangeSizeOption) {
466+
ListsExtensionOptions ext_options;
467+
ext_options.max_range_size = 100;
468+
469+
ASSERT_OK_AND_ASSIGN(
470+
auto compiler_builder,
471+
NewCompilerBuilder(internal::GetTestingDescriptorPool()));
472+
ASSERT_THAT(compiler_builder->AddLibrary(StandardCompilerLibrary()), IsOk());
473+
ASSERT_THAT(compiler_builder->AddLibrary(ListsCompilerLibrary(ext_options)),
474+
IsOk());
475+
ASSERT_OK_AND_ASSIGN(auto compiler, std::move(*compiler_builder).Build());
476+
477+
ASSERT_OK_AND_ASSIGN(ValidationResult result,
478+
compiler->Compile("lists.range(101)", "<input>"));
479+
ASSERT_TRUE(result.IsValid());
480+
ASSERT_OK_AND_ASSIGN(std::unique_ptr<Ast> ast, result.ReleaseAst());
481+
482+
const auto runtime_options = RuntimeOptions{};
483+
ASSERT_OK_AND_ASSIGN(
484+
auto runtime_builder,
485+
CreateStandardRuntimeBuilder(internal::GetTestingDescriptorPool(),
486+
runtime_options));
487+
ASSERT_THAT(RegisterListsFunctions(runtime_builder.function_registry(),
488+
runtime_options, ext_options),
489+
IsOk());
490+
ASSERT_OK_AND_ASSIGN(auto runtime, std::move(runtime_builder).Build());
491+
ASSERT_OK_AND_ASSIGN(auto program, runtime->CreateProgram(std::move(ast)));
492+
493+
google::protobuf::Arena arena;
494+
Activation activation;
495+
ASSERT_OK_AND_ASSIGN(Value eval_result,
496+
program->Evaluate(&arena, activation));
497+
EXPECT_THAT(
498+
eval_result,
499+
ErrorValueIs(StatusIs(
500+
testing::_,
501+
HasSubstr("lists.range: size 101 exceeds maximum allowed (100)"))));
502+
}
503+
504+
TEST(ListsFunctionsTest, HardCodedLimitAppliesWhenOptionIsLooser) {
505+
ListsExtensionOptions ext_options;
506+
ext_options.max_range_size = 2000000;
507+
508+
ASSERT_OK_AND_ASSIGN(
509+
auto compiler_builder,
510+
NewCompilerBuilder(internal::GetTestingDescriptorPool()));
511+
ASSERT_THAT(compiler_builder->AddLibrary(StandardCompilerLibrary()), IsOk());
512+
ASSERT_THAT(compiler_builder->AddLibrary(ListsCompilerLibrary(ext_options)),
513+
IsOk());
514+
ASSERT_OK_AND_ASSIGN(auto compiler, std::move(*compiler_builder).Build());
515+
516+
ASSERT_OK_AND_ASSIGN(ValidationResult result,
517+
compiler->Compile("lists.range(1000001)", "<input>"));
518+
ASSERT_TRUE(result.IsValid());
519+
ASSERT_OK_AND_ASSIGN(std::unique_ptr<Ast> ast, result.ReleaseAst());
520+
521+
const auto runtime_options = RuntimeOptions{};
522+
ASSERT_OK_AND_ASSIGN(
523+
auto runtime_builder,
524+
CreateStandardRuntimeBuilder(internal::GetTestingDescriptorPool(),
525+
runtime_options));
526+
ASSERT_THAT(RegisterListsFunctions(runtime_builder.function_registry(),
527+
runtime_options, ext_options),
528+
IsOk());
529+
ASSERT_OK_AND_ASSIGN(auto runtime, std::move(runtime_builder).Build());
530+
ASSERT_OK_AND_ASSIGN(auto program, runtime->CreateProgram(std::move(ast)));
531+
532+
google::protobuf::Arena arena;
533+
Activation activation;
534+
ASSERT_OK_AND_ASSIGN(Value eval_result,
535+
program->Evaluate(&arena, activation));
536+
EXPECT_THAT(
537+
eval_result,
538+
ErrorValueIs(StatusIs(
539+
testing::_,
540+
HasSubstr(
541+
"lists.range: size 1000001 exceeds maximum allowed (1000000)"))));
542+
}
543+
460544
} // namespace
461545
} // namespace cel::extensions

0 commit comments

Comments
 (0)