Skip to content

Commit 725b335

Browse files
jnthntatumcopybara-github
authored andcommitted
Incrementally check code point limit in NewSource
Avoids copying the full source if it is outside of the configured limit. cross ref: cel-expr/cel-go#1302 PiperOrigin-RevId: 957218893
1 parent 2fbbd6b commit 725b335

11 files changed

Lines changed: 203 additions & 13 deletions

File tree

common/BUILD

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,8 +640,8 @@ cc_test(
640640
deps = [
641641
":source",
642642
"//internal:testing",
643+
"@com_google_absl//absl/status",
643644
"@com_google_absl//absl/strings:cord",
644-
"@com_google_absl//absl/types:optional",
645645
],
646646
)
647647

common/source.cc

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -307,12 +307,18 @@ struct SourceTextTraits<absl::Cord> {
307307

308308
template <typename T>
309309
absl::StatusOr<SourcePtr> NewSourceImpl(std::string description, const T& text,
310-
const size_t text_size) {
310+
const size_t text_size,
311+
const size_t max_codepoints) {
311312
if (ABSL_PREDICT_FALSE(
312313
text_size >
313314
static_cast<size_t>(std::numeric_limits<int32_t>::max()))) {
314315
return absl::InvalidArgumentError("expression larger than 2GiB limit");
315316
}
317+
if ((text_size >> 2) > max_codepoints) {
318+
// If byte size is 4 times the codepoint limit, then definitely exceeded.
319+
return absl::InvalidArgumentError(absl::StrCat(
320+
"expression is larger than codepoint limit ", max_codepoints));
321+
}
316322
using Traits = SourceTextTraits<T>;
317323
size_t index = 0;
318324
typename Traits::iterator_type it = Traits::Begin(text);
@@ -324,6 +330,10 @@ absl::StatusOr<SourcePtr> NewSourceImpl(std::string description, const T& text,
324330
std::vector<char32_t> data32;
325331
absl::InlinedVector<SourcePosition, 1> line_offsets;
326332
while (index < text_size) {
333+
if (offset >= max_codepoints) {
334+
return absl::InvalidArgumentError(absl::StrCat(
335+
"expression is larger than codepoint limit ", max_codepoints));
336+
}
327337
std::tie(code_point, code_units) = cel::internal::Utf8Decode(it);
328338
if (ABSL_PREDICT_FALSE(code_point ==
329339
cel::internal::kUnicodeReplacementCharacter &&
@@ -375,6 +385,10 @@ absl::StatusOr<SourcePtr> NewSourceImpl(std::string description, const T& text,
375385
std::move(description), std::move(line_offsets), Traits::ToVector(text));
376386
latin1:
377387
while (index < text_size) {
388+
if (offset >= max_codepoints) {
389+
return absl::InvalidArgumentError(absl::StrCat(
390+
"expression is larger than codepoint limit ", max_codepoints));
391+
}
378392
std::tie(code_point, code_units) = internal::Utf8Decode(it);
379393
if (ABSL_PREDICT_FALSE(code_point ==
380394
internal::kUnicodeReplacementCharacter &&
@@ -420,6 +434,10 @@ absl::StatusOr<SourcePtr> NewSourceImpl(std::string description, const T& text,
420434
std::move(description), std::move(line_offsets), std::move(data8));
421435
basic:
422436
while (index < text_size) {
437+
if (offset >= max_codepoints) {
438+
return absl::InvalidArgumentError(absl::StrCat(
439+
"expression is larger than codepoint limit ", max_codepoints));
440+
}
423441
std::tie(code_point, code_units) = internal::Utf8Decode(it);
424442
if (ABSL_PREDICT_FALSE(code_point ==
425443
internal::kUnicodeReplacementCharacter &&
@@ -453,6 +471,10 @@ absl::StatusOr<SourcePtr> NewSourceImpl(std::string description, const T& text,
453471
std::move(description), std::move(line_offsets), std::move(data16));
454472
supplemental:
455473
while (index < text_size) {
474+
if (offset >= max_codepoints) {
475+
return absl::InvalidArgumentError(absl::StrCat(
476+
"expression is larger than codepoint limit ", max_codepoints));
477+
}
456478
std::tie(code_point, code_units) = internal::Utf8Decode(it);
457479
if (ABSL_PREDICT_FALSE(code_point ==
458480
internal::kUnicodeReplacementCharacter &&
@@ -631,16 +653,27 @@ absl::Span<const SourcePosition> SourceSubrange::line_offsets() const {
631653
return absl::MakeConstSpan(line_offsets_);
632654
}
633655

656+
static size_t ClampLimit(int value) {
657+
if (value < 0) {
658+
return std::numeric_limits<size_t>::max();
659+
}
660+
return static_cast<size_t>(value);
661+
}
662+
634663
absl::StatusOr<absl_nonnull SourcePtr> NewSource(absl::string_view content,
635-
std::string description) {
664+
std::string description,
665+
const SourceOptions& options) {
636666
return common_internal::NewSourceImpl(std::move(description), content,
637-
content.size());
667+
content.size(),
668+
ClampLimit(options.max_codepoint_size));
638669
}
639670

640671
absl::StatusOr<absl_nonnull SourcePtr> NewSource(const absl::Cord& content,
641-
std::string description) {
672+
std::string description,
673+
const SourceOptions& options) {
642674
return common_internal::NewSourceImpl(std::move(description), content,
643-
content.size());
675+
content.size(),
676+
ClampLimit(options.max_codepoint_size));
644677
}
645678

646679
} // namespace cel

common/source.h

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -221,11 +221,40 @@ class SourceSubrange final : public Source {
221221

222222
using SourcePtr = std::unique_ptr<Source>;
223223

224-
absl::StatusOr<absl_nonnull SourcePtr> NewSource(
225-
absl::string_view content, std::string description = "<input>");
224+
struct SourceOptions {
225+
// The maximum number of code points allowed in the source.
226+
// A negative value indicates no limit (though still limited by
227+
// int32_t max value).
228+
int max_codepoint_size = 100'000;
229+
};
230+
231+
absl::StatusOr<absl_nonnull SourcePtr> NewSource(absl::string_view content,
232+
std::string description,
233+
const SourceOptions& options);
234+
235+
absl::StatusOr<absl_nonnull SourcePtr> NewSource(const absl::Cord& content,
236+
std::string description,
237+
const SourceOptions& options);
238+
239+
inline absl::StatusOr<absl_nonnull SourcePtr> NewSource(
240+
absl::string_view content, std::string description) {
241+
return NewSource(content, std::move(description), SourceOptions{});
242+
}
226243

227-
absl::StatusOr<absl_nonnull SourcePtr> NewSource(
228-
const absl::Cord& content, std::string description = "<input>");
244+
inline absl::StatusOr<absl_nonnull SourcePtr> NewSource(
245+
const absl::Cord& content, std::string description) {
246+
return NewSource(content, std::move(description), SourceOptions{});
247+
}
248+
249+
inline absl::StatusOr<absl_nonnull SourcePtr> NewSource(
250+
absl::string_view content) {
251+
return NewSource(content, "<input>", SourceOptions{});
252+
}
253+
254+
inline absl::StatusOr<absl_nonnull SourcePtr> NewSource(
255+
const absl::Cord& content) {
256+
return NewSource(content, "<input>", SourceOptions{});
257+
}
229258

230259
} // namespace cel
231260

common/source_test.cc

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,11 @@
1414

1515
#include "common/source.h"
1616

17+
#include <cstdint>
18+
#include <optional>
19+
20+
#include "absl/status/status.h"
1721
#include "absl/strings/cord.h"
18-
#include "absl/types/optional.h"
1922
#include "internal/testing.h"
2023

2124
namespace cel {
@@ -262,5 +265,51 @@ TEST(SourceSubrange, LineOffsetsMiddleSubrange) {
262265
EXPECT_THAT(subrange.line_offsets(), ElementsAre(6, 10));
263266
}
264267

268+
TEST(StringSource, CodepointLimitExceeded) {
269+
SourceOptions options;
270+
options.max_codepoint_size = 5;
271+
272+
EXPECT_THAT(
273+
NewSource("123456", "test", options),
274+
::absl_testing::StatusIs(
275+
absl::StatusCode::kInvalidArgument,
276+
::testing::HasSubstr("expression is larger than codepoint limit 5")));
277+
278+
ASSERT_OK_AND_ASSIGN(auto source, NewSource("12345", "test", options));
279+
EXPECT_THAT(source->content().ToString(), ::testing::Eq("12345"));
280+
}
281+
282+
TEST(StringSource, CodepointLimitMultibyteUtf8) {
283+
SourceOptions options;
284+
options.max_codepoint_size = 5;
285+
286+
// "Hello" consists of 5 full-width Unicode characters (15 bytes in
287+
// UTF-8).
288+
ASSERT_OK_AND_ASSIGN(auto source, NewSource("Hello", "test", options));
289+
EXPECT_THAT(source->content().ToString(), ::testing::Eq("Hello"));
290+
291+
options.max_codepoint_size = 4;
292+
EXPECT_THAT(
293+
NewSource("Hello", "test", options),
294+
::absl_testing::StatusIs(
295+
absl::StatusCode::kInvalidArgument,
296+
::testing::HasSubstr("expression is larger than codepoint limit 4")));
297+
}
298+
299+
TEST(CordSource, CodepointLimitExceeded) {
300+
SourceOptions options;
301+
options.max_codepoint_size = 5;
302+
303+
EXPECT_THAT(
304+
NewSource(absl::Cord("123456"), "test", options),
305+
::absl_testing::StatusIs(
306+
absl::StatusCode::kInvalidArgument,
307+
::testing::HasSubstr("expression is larger than codepoint limit 5")));
308+
309+
ASSERT_OK_AND_ASSIGN(auto source,
310+
NewSource(absl::Cord("12345"), "test", options));
311+
EXPECT_THAT(source->content().ToString(), ::testing::Eq("12345"));
312+
}
313+
265314
} // namespace
266315
} // namespace cel

compiler/compiler.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,8 @@ class Compiler {
178178
inline absl::StatusOr<ValidationResult> Compiler::Compile(
179179
absl::string_view source, absl::string_view description,
180180
google::protobuf::Arena* absl_nullable arena) const {
181-
absl::StatusOr<SourcePtr> source_obj =
182-
NewSource(source, std::string(description));
181+
absl::StatusOr<std::unique_ptr<Source>> source_obj =
182+
GetParser().PrepareSource(source, description);
183183
if (!source_obj.ok()) {
184184
return source_obj.status();
185185
}

compiler/compiler_factory_test.cc

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,5 +441,24 @@ TEST(CompilerFactoryTest, CompileSourceOverload) {
441441
EXPECT_TRUE(result.IsValid());
442442
}
443443

444+
TEST(CompilerFactoryTest, CodepointLimitExceeded) {
445+
CompilerOptions options;
446+
options.parser_options.expression_size_codepoint_limit = 10;
447+
ASSERT_OK_AND_ASSIGN(
448+
auto builder,
449+
NewCompilerBuilder(cel::internal::GetSharedTestingDescriptorPool(),
450+
options));
451+
ASSERT_OK_AND_ASSIGN(auto compiler, builder->Build());
452+
453+
EXPECT_THAT(
454+
compiler->Compile("123456789012345", "test.cel"),
455+
StatusIs(absl::StatusCode::kInvalidArgument,
456+
HasSubstr("expression is larger than codepoint limit 10")));
457+
458+
ASSERT_OK_AND_ASSIGN(ValidationResult result,
459+
compiler->Compile("1234567890", "test.cel"));
460+
EXPECT_TRUE(result.IsValid());
461+
}
462+
444463
} // namespace
445464
} // namespace cel

parser/internal/pratt_parser.cc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,14 @@ absl::StatusOr<std::unique_ptr<cel::Ast>> PrattParserImpl::ParseImpl(
163163
return PrattParseImpl(source, macro_registry_, options_, parse_issues);
164164
}
165165

166+
absl::StatusOr<std::unique_ptr<cel::Source>> PrattParserImpl::PrepareSourceImpl(
167+
absl::string_view input, absl::string_view description) const {
168+
return cel::NewSource(
169+
input, std::string(description),
170+
cel::SourceOptions{.max_codepoint_size =
171+
options_.expression_size_codepoint_limit});
172+
}
173+
166174
absl::StatusOr<std::unique_ptr<cel::Ast>> PrattParseImpl(
167175
const cel::Source& source, const cel::MacroRegistry& registry,
168176
const ParserOptions& options, std::vector<cel::ParseIssue>* parse_issues) {

parser/internal/pratt_parser.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ class PrattParserImpl final : public cel::Parser {
5959
const cel::Source& source,
6060
std::vector<cel::ParseIssue>* absl_nullable parse_issues) const override;
6161

62+
absl::StatusOr<std::unique_ptr<cel::Source>> PrepareSourceImpl(
63+
absl::string_view input, absl::string_view description) const override;
64+
6265
std::unique_ptr<cel::ParserBuilder> ToBuilder() const override;
6366

6467
private:

parser/parser.cc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1781,6 +1781,14 @@ class ParserImpl : public cel::Parser {
17811781
std::move(parse_result.source_info));
17821782
}
17831783

1784+
absl::StatusOr<std::unique_ptr<cel::Source>> PrepareSourceImpl(
1785+
absl::string_view input, absl::string_view description) const override {
1786+
return cel::NewSource(
1787+
input, std::string(description),
1788+
cel::SourceOptions{.max_codepoint_size =
1789+
options_.expression_size_codepoint_limit});
1790+
}
1791+
17841792
std::unique_ptr<cel::ParserBuilder> ToBuilder() const override;
17851793

17861794
private:

parser/parser_interface.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,13 +114,24 @@ class Parser {
114114
absl::StatusOr<std::unique_ptr<cel::Ast>> Parse(
115115
const cel::Source& source, std::vector<ParseIssue>* issues) const;
116116

117+
// Returns a Source object from the given input.
118+
// Validates that the input is well-formed utf-8 and within the configured
119+
// source limits.
120+
absl::StatusOr<std::unique_ptr<cel::Source>> PrepareSource(
121+
absl::string_view input, absl::string_view description) const;
122+
absl::StatusOr<std::unique_ptr<cel::Source>> PrepareSource(
123+
absl::string_view input) const;
124+
117125
// Returns a builder initialized with the configuration of this parser.
118126
virtual std::unique_ptr<ParserBuilder> ToBuilder() const = 0;
119127

120128
protected:
121129
virtual absl::StatusOr<std::unique_ptr<cel::Ast>> ParseImpl(
122130
const cel::Source& source,
123131
std::vector<ParseIssue>* absl_nullable parse_issues) const = 0;
132+
133+
virtual absl::StatusOr<std::unique_ptr<cel::Source>> PrepareSourceImpl(
134+
absl::string_view input, absl::string_view description) const = 0;
124135
};
125136

126137
inline absl::StatusOr<std::unique_ptr<cel::Ast>> Parser::Parse(
@@ -134,6 +145,16 @@ inline absl::StatusOr<std::unique_ptr<cel::Ast>> Parser::Parse(
134145
return ParseImpl(source, issues);
135146
}
136147

148+
inline absl::StatusOr<std::unique_ptr<cel::Source>> Parser::PrepareSource(
149+
absl::string_view input, absl::string_view description) const {
150+
return PrepareSourceImpl(input, description);
151+
}
152+
153+
inline absl::StatusOr<std::unique_ptr<cel::Source>> Parser::PrepareSource(
154+
absl::string_view input) const {
155+
return PrepareSourceImpl(input, "<input>");
156+
}
157+
137158
} // namespace cel
138159

139160
#endif // THIRD_PARTY_CEL_CPP_PARSER_PARSER_INTERFACE_H_

0 commit comments

Comments
 (0)