Skip to content

Commit b955165

Browse files
acozzettecopybara-github
authored andcommitted
Internal change
PiperOrigin-RevId: 573332237
1 parent 1639639 commit b955165

File tree

4 files changed

+36
-9
lines changed

4 files changed

+36
-9
lines changed

src/google/protobuf/io/test_zero_copy_stream.h

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@
99
#define GOOGLE_PROTOBUF_IO_TEST_ZERO_COPY_STREAM_H__
1010

1111
#include <deque>
12+
#include <memory>
1213
#include <string>
1314
#include <utility>
1415
#include <vector>
1516

1617
#include "absl/log/absl_check.h"
17-
#include "absl/types/optional.h"
1818
#include "google/protobuf/io/zero_copy_stream.h"
1919

2020
// Must be included last.
@@ -37,18 +37,22 @@ class TestZeroCopyInputStream final : public ZeroCopyInputStream {
3737
TestZeroCopyInputStream(const TestZeroCopyInputStream& other)
3838
: ZeroCopyInputStream(),
3939
buffers_(other.buffers_),
40-
last_returned_buffer_(other.last_returned_buffer_),
40+
last_returned_buffer_(
41+
other.last_returned_buffer_
42+
? std::make_unique<std::string>(*other.last_returned_buffer_)
43+
: nullptr),
4144
byte_count_(other.byte_count_) {}
4245

4346
bool Next(const void** data, int* size) override {
4447
ABSL_CHECK(data) << "data must not be null";
4548
ABSL_CHECK(size) << "size must not be null";
46-
last_returned_buffer_ = absl::nullopt;
49+
last_returned_buffer_ = nullptr;
4750

4851
// We are done
4952
if (buffers_.empty()) return false;
5053

51-
last_returned_buffer_ = std::move(buffers_.front());
54+
last_returned_buffer_ =
55+
std::make_unique<std::string>(std::move(buffers_.front()));
5256
buffers_.pop_front();
5357
*data = last_returned_buffer_->data();
5458
*size = static_cast<int>(last_returned_buffer_->size());
@@ -58,19 +62,19 @@ class TestZeroCopyInputStream final : public ZeroCopyInputStream {
5862

5963
void BackUp(int count) override {
6064
ABSL_CHECK_GE(count, 0) << "count must not be negative";
61-
ABSL_CHECK(last_returned_buffer_.has_value())
65+
ABSL_CHECK(last_returned_buffer_ != nullptr)
6266
<< "The last call was not a successful Next()";
6367
ABSL_CHECK_LE(count, last_returned_buffer_->size())
6468
<< "count must be within bounds of last buffer";
6569
buffers_.push_front(
6670
last_returned_buffer_->substr(last_returned_buffer_->size() - count));
67-
last_returned_buffer_ = absl::nullopt;
71+
last_returned_buffer_ = nullptr;
6872
byte_count_ -= count;
6973
}
7074

7175
bool Skip(int count) override {
7276
ABSL_CHECK_GE(count, 0) << "count must not be negative";
73-
last_returned_buffer_ = absl::nullopt;
77+
last_returned_buffer_ = nullptr;
7478
while (true) {
7579
if (count == 0) return true;
7680
if (buffers_.empty()) return false;
@@ -96,7 +100,9 @@ class TestZeroCopyInputStream final : public ZeroCopyInputStream {
96100
// move them to `last_returned_buffer_`. It makes it simpler to keep track of
97101
// the state of the object. The extra cost is not relevant for testing.
98102
std::deque<std::string> buffers_;
99-
absl::optional<std::string> last_returned_buffer_;
103+
// absl::optional could work here, but std::unique_ptr makes it more likely
104+
// for sanitizers to detect if the string is used after it is destroyed.
105+
std::unique_ptr<std::string> last_returned_buffer_;
100106
int64_t byte_count_ = 0;
101107
};
102108

src/google/protobuf/json/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ cc_test(
4141
"//src/google/protobuf:cc_test_protos",
4242
"//src/google/protobuf:port_def",
4343
"//src/google/protobuf/io",
44+
"//src/google/protobuf/io:test_zero_copy_stream",
4445
"//src/google/protobuf/util:json_format_cc_proto",
4546
"//src/google/protobuf/util:json_format_proto3_cc_proto",
4647
"//src/google/protobuf/util:type_resolver_util",

src/google/protobuf/json/internal/parser.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1273,7 +1273,7 @@ absl::Status ParseMessage(JsonLexer& lex, const Desc<Traits>& desc,
12731273
}
12741274
}
12751275

1276-
return ParseField<Traits>(lex, desc, name.value.AsView(), msg);
1276+
return ParseField<Traits>(lex, desc, name.value.ToString(), msg);
12771277
});
12781278
}
12791279
} // namespace

src/google/protobuf/json/json_test.cc

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "absl/strings/string_view.h"
2727
#include "google/protobuf/descriptor_database.h"
2828
#include "google/protobuf/dynamic_message.h"
29+
#include "google/protobuf/io/test_zero_copy_stream.h"
2930
#include "google/protobuf/io/zero_copy_stream.h"
3031
#include "google/protobuf/io/zero_copy_stream_impl_lite.h"
3132
#include "google/protobuf/util/json_format.pb.h"
@@ -50,6 +51,7 @@ using ::proto3::TestMap;
5051
using ::proto3::TestMessage;
5152
using ::proto3::TestOneof;
5253
using ::proto3::TestWrapper;
54+
using ::testing::ContainsRegex;
5355
using ::testing::ElementsAre;
5456
using ::testing::IsEmpty;
5557
using ::testing::Not;
@@ -1331,6 +1333,24 @@ TEST_P(JsonTest, ClearPreExistingRepeatedInJsonValues) {
13311333
EXPECT_THAT(s.fields(), IsEmpty());
13321334
}
13331335

1336+
TEST(JsonErrorTest, FieldNameAndSyntaxErrorInSeparateChunks) {
1337+
std::unique_ptr<TypeResolver> resolver{
1338+
google::protobuf::util::NewTypeResolverForDescriptorPool(
1339+
"type.googleapis.com", DescriptorPool::generated_pool())};
1340+
io::internal::TestZeroCopyInputStream input_stream(
1341+
{"{\"bool_value\":", "5}"});
1342+
std::string result;
1343+
io::StringOutputStream output_stream(&result);
1344+
absl::Status s = JsonToBinaryStream(
1345+
resolver.get(), "type.googleapis.com/proto3.TestMessage", &input_stream,
1346+
&output_stream, ParseOptions{});
1347+
ASSERT_FALSE(s.ok());
1348+
EXPECT_THAT(
1349+
s.message(),
1350+
ContainsRegex("invalid *JSON *in *type.googleapis.com/proto3.TestMessage "
1351+
"*@ *bool_value"));
1352+
}
1353+
13341354
} // namespace
13351355
} // namespace json
13361356
} // namespace protobuf

0 commit comments

Comments
 (0)