diff --git a/CHANGELOG.md b/CHANGELOG.md index 64ff58d..97a4624 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ number says nothing about any other port. See the Releases section of ## [Unreleased] +### Fixed + +- **`DatasetReader` reads a row under the caller's `max_bytes_literal_length`** (#160), as protowire-go does. A row's cells were parsed under the default `kMaxBytesLiteralLength` whatever the options said. A caller who lowered the limit still got a row whose `b"…"` cell was past it, and a caller who raised it had that cell refused. `0` still means the default. Only the nesting depth used to reach the row parser. The header (a `@header` block and the `@dataset` line) was parsed under the defaults for both limits. It is now parsed under the caller's `max_nesting_depth` and `max_bytes_literal_length`, so `Create` refuses such a header as protowire-go's `NewDatasetReader` does, with the same text and position. + ## [2.0.0] — 2026-11-05 A major release, because `pb::Marshal` now returns `StatusOr>` and every caller must change (#93). `pb::Marshal` and `pxf::Marshal` refuse what a decoder at the default limits refuses, instead of writing it. A PXF `BigFloat` literal is refused in protowire-go's words and order of checks, and a big-number message no longer counts as a nesting level. Several changes narrow what decodes, encodes or binds; each says so. This release also carries the change prepared as 1.2.1, which was never tagged. diff --git a/src/pxf/dataset_reader.cc b/src/pxf/dataset_reader.cc index 0fe7f7c..62f536d 100644 --- a/src/pxf/dataset_reader.cc +++ b/src/pxf/dataset_reader.cc @@ -474,7 +474,9 @@ Status DatasetReader::Next(DatasetRow* out) { auto parsed = ParseDatasetRow(row_bytes, static_cast(columns_.size()), - opts_.max_nesting_depth > 0 ? opts_.max_nesting_depth : kMaxNestingDepth); + opts_.max_nesting_depth > 0 ? opts_.max_nesting_depth : kMaxNestingDepth, + opts_.max_bytes_literal_length > 0 ? opts_.max_bytes_literal_length + : kMaxBytesLiteralLength); // Advance past the consumed bytes whether parse succeeded or not // — on failure we don't want to retry the same bad row forever. pending_.erase(0, static_cast(end) + 1); @@ -560,7 +562,12 @@ Status DatasetReader::ReadHeader() { if (!scan_err.ok()) return scan_err; if (found) { std::string_view header(pending_.data(), static_cast(header_end) + 1); - auto doc = Parse(header); + // The header is parsed under the caller's limits, as the rows are and + // as protowire-go parses both (#160). + ParseOptions popts; + popts.max_nesting_depth = opts_.max_nesting_depth; + popts.max_bytes_literal_length = opts_.max_bytes_literal_length; + auto doc = Parse(header, popts); if (!doc.ok()) return doc.status(); if (doc->datasets.empty()) { return Status::Error("pxf: no @dataset directive in stream"); diff --git a/src/pxf/parser.cc b/src/pxf/parser.cc index bb471cc..d506a6a 100644 --- a/src/pxf/parser.cc +++ b/src/pxf/parser.cc @@ -1153,8 +1153,11 @@ StatusOr ParseDatasetAt(Lexer* lex, Token* current, int max_de return tbl; } -StatusOr ParseDatasetRow(std::string_view row, int expected, int max_depth) { - Parser p(row, max_depth, kMaxBytesLiteralLength); +StatusOr ParseDatasetRow(std::string_view row, + int expected, + int max_depth, + int max_bytes_literal) { + Parser p(row, max_depth, max_bytes_literal); int end = 0; return p.ParseTableRow(expected, &end); } diff --git a/src/pxf/parser_internal.h b/src/pxf/parser_internal.h index a5724b3..82afcf9 100644 --- a/src/pxf/parser_internal.h +++ b/src/pxf/parser_internal.h @@ -20,8 +20,12 @@ namespace protowire::pxf { // token *lex produces, and leaves both on the token after it. StatusOr ParseDatasetAt(Lexer* lex, Token* current, int max_depth); -// ParseDatasetRow parses one `( cell, ... )` of expected cells. -StatusOr ParseDatasetRow(std::string_view row, int expected, int max_depth); +// ParseDatasetRow parses one `( cell, ... )` of expected cells, under the +// caller's nesting and bytes-literal limits. +StatusOr ParseDatasetRow(std::string_view row, + int expected, + int max_depth, + int max_bytes_literal); // CheckDescriptorSchema runs ValidateDescriptor and joins any violations // into one error, the bind-time check Unmarshal and DatasetReader share. diff --git a/test/pxf_hardening_test.cc b/test/pxf_hardening_test.cc index 7a15e5e..6719eea 100644 --- a/test/pxf_hardening_test.cc +++ b/test/pxf_hardening_test.cc @@ -9,6 +9,7 @@ #include "protowire/limits.h" #include "protowire/pxf.h" +#include "protowire/pxf/dataset_reader.h" #include "protowire/pxf/lexer.h" #include "protowire/pxf/parser.h" @@ -17,6 +18,7 @@ #include "protoc_compat.h" #include +#include #include #include @@ -192,6 +194,57 @@ TEST_F(Hardening, BytesLiteralLengthBound) { EXPECT_TRUE(protowire::pxf::Parse(doc).ok()); } +// A dataset row is read under the caller's limit too, as protowire-go reads +// it (#160). It used to be read under the default whatever the options said. +TEST(HardeningDataset, RowBytesLiteralUnderTheCallersLimit) { + // 256 characters of base64 decode to 192 bytes. + const std::string doc = "@dataset x.Row ( raw )\n( b\"" + std::string(256, 'A') + "\" )\n"; + auto first_row = [&](int limit) { + std::istringstream in(doc); + UnmarshalOptions opts; + opts.max_bytes_literal_length = limit; + auto tr = protowire::pxf::DatasetReader::Create(&in, opts); + EXPECT_TRUE(tr.ok()) << tr.status().ToString(); + protowire::pxf::DatasetRow row; + return (*tr)->Next(&row); + }; + protowire::Status st = first_row(191); + ASSERT_FALSE(st.ok()); + EXPECT_NE(st.message().find("bytes literal decodes to more than MaxBytesLiteralLength=191 bytes"), + std::string::npos) + << st.ToString(); + EXPECT_TRUE(first_row(192).ok()) << first_row(192).ToString(); + EXPECT_TRUE(first_row(0).ok()) << first_row(0).ToString(); // 0 is the default +} + +// The header too: a @header block before @dataset is parsed under the +// caller's limits, so Create refuses it as protowire-go's NewDatasetReader does. +TEST(HardeningDataset, HeaderUnderTheCallersLimits) { + const std::string bytes_doc = + "@header x.H { raw = b\"" + std::string(256, 'A') + "\" }\n@dataset x.Row ( raw )\n( 1 )\n"; + const std::string deep_doc = + "@header x.H { a { b { c { d = 1 } } } }\n@dataset x.Row ( d )\n( 1 )\n"; + auto create = [](const std::string& doc, UnmarshalOptions opts) { + std::istringstream in(doc); + return protowire::pxf::DatasetReader::Create(&in, opts).status(); + }; + UnmarshalOptions opts; + opts.max_bytes_literal_length = 191; + protowire::Status st = create(bytes_doc, opts); + ASSERT_FALSE(st.ok()); + EXPECT_NE(st.message().find("bytes literal decodes to more than MaxBytesLiteralLength=191 bytes"), + std::string::npos) + << st.ToString(); + EXPECT_TRUE(create(bytes_doc, {}).ok()); + opts = {}; + opts.max_nesting_depth = 3; + st = create(deep_doc, opts); + ASSERT_FALSE(st.ok()); + EXPECT_NE(st.message().find("MaxNestingDepth=3"), std::string::npos) << st.ToString(); + opts.max_nesting_depth = 4; + EXPECT_TRUE(create(deep_doc, opts).ok()) << create(deep_doc, opts).ToString(); +} + TEST_F(Hardening, RepeatedCountBound) { std::string list = "values = ["; for (int i = 0; i < 16; ++i) list += (i ? ", " : "") + std::to_string(i);