Skip to content

Commit

Permalink
Make DebugString print debug output, enable debug markers for debug o…
Browse files Browse the repository at this point in the history
…utput

Debug output is unstable and redacts fields marked directly with debug_redact, and indirectly via options. This is a breaking change for anyone who depends on DebugString output to be stable and/or redacting.
See https://protobuf.dev/news/2024-12-04/

PiperOrigin-RevId: 721918779
  • Loading branch information
tanx16 authored and copybara-github committed Jan 31, 2025
1 parent a8fca08 commit 9a03332
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 20 deletions.
77 changes: 76 additions & 1 deletion src/google/protobuf/text_format.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@
#include <cstdint>
#include <limits>
#include <memory>
#include <random>
#include <string>
#include <utility>
#include <vector>

#include "absl/base/macros.h"
#include "absl/container/btree_set.h"
#include "absl/log/absl_check.h"
#include "absl/strings/ascii.h"
Expand All @@ -36,6 +38,8 @@
#include "absl/strings/str_format.h"
#include "absl/strings/str_join.h"
#include "absl/strings/string_view.h"
#include "absl/time/clock.h"
#include "absl/time/time.h"
#include "google/protobuf/any.h"
#include "google/protobuf/descriptor.h"
#include "google/protobuf/descriptor.pb.h"
Expand Down Expand Up @@ -99,7 +103,7 @@ const char kDebugStringSilentMarkerForDetection[] = "\t ";

// Controls insertion of a marker making debug strings non-parseable, and
// redacting annotated fields in Protobuf's DebugString APIs.
PROTOBUF_EXPORT std::atomic<bool> enable_debug_string_safe_format{false};
PROTOBUF_EXPORT std::atomic<bool> enable_debug_string_safe_format{true};

int64_t GetRedactedFieldCount() {
return num_redacted_field.load(std::memory_order_relaxed);
Expand Down Expand Up @@ -2299,6 +2303,9 @@ bool TextFormat::Printer::Print(const Message& message,
internal::FieldReporterLevel reporter) const {
TextGenerator generator(output, insert_silent_marker_, initial_indent_level_);

internal::PrintTextMarker(&generator, redact_debug_string_,
randomize_debug_string_, single_line_mode_);


Print(message, &generator);

Expand Down Expand Up @@ -3109,6 +3116,74 @@ bool TextFormat::Printer::TryRedactFieldValue(
}
return false;
}

class TextMarkerGenerator final {
public:
static TextMarkerGenerator CreateRandom();

void PrintMarker(TextFormat::BaseTextGenerator* generator, bool redact,
bool randomize, bool single_line_mode) const {
if (redact) {
generator->Print(redaction_marker_.data(), redaction_marker_.size());
}
if (randomize) {
generator->Print(random_marker_.data(), random_marker_.size());
}
if ((redact || randomize) && !single_line_mode) {
generator->PrintLiteral("\n");
}
}

private:
static constexpr absl::string_view kRedactionMarkers[] = {
"goo.gle/debugonly ", "goo.gle/nodeserialize ", "goo.gle/debugstr ",
"goo.gle/debugproto "};

static constexpr absl::string_view kRandomMarker = " ";

static_assert(!kRandomMarker.empty(), "The random marker cannot be empty!");

constexpr TextMarkerGenerator(absl::string_view redaction_marker,
absl::string_view random_marker)
: redaction_marker_(redaction_marker), random_marker_(random_marker) {}

absl::string_view redaction_marker_;
absl::string_view random_marker_;
};

TextMarkerGenerator TextMarkerGenerator::CreateRandom() {
// We avoid using sources backed by system entropy to allow the marker
// generator to work in sandboxed environments that have no access to syscalls
// such as getrandom or getpid. Note that this randomization has no security
// implications, it's only used to break code that attempts to deserialize
// debug strings.
std::mt19937_64 random{
static_cast<uint64_t>(absl::ToUnixMicros(absl::Now()))};

size_t redaction_marker_index = std::uniform_int_distribution<size_t>{
0, ABSL_ARRAYSIZE(kRedactionMarkers) - 1}(random);

size_t random_marker_size =
std::uniform_int_distribution<size_t>{1, kRandomMarker.size()}(random);

return TextMarkerGenerator(kRedactionMarkers[redaction_marker_index],
kRandomMarker.substr(0, random_marker_size));
}

const TextMarkerGenerator& GetGlobalTextMarkerGenerator() {
static const TextMarkerGenerator kTextMarkerGenerator =
TextMarkerGenerator::CreateRandom();
return kTextMarkerGenerator;
}

namespace internal {
void PrintTextMarker(TextFormat::BaseTextGenerator* generator, bool redact,
bool randomize, bool single_line_mode) {
GetGlobalTextMarkerGenerator().PrintMarker(generator, redact, randomize,
single_line_mode);
}
} // namespace internal

} // namespace protobuf
} // namespace google

Expand Down
4 changes: 4 additions & 0 deletions src/google/protobuf/text_format.h
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,10 @@ class PROTOBUF_EXPORT TextFormat {
const T&... values);
};

namespace internal {
void PrintTextMarker(TextFormat::BaseTextGenerator* generator, bool redact,
bool randomize, bool single_line_mode);
} // namespace internal

inline void TextFormat::RecordLocation(ParseInfoTree* info_tree,
const FieldDescriptor* field,
Expand Down
62 changes: 43 additions & 19 deletions src/google/protobuf/text_format_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ class UnsetFieldsMetadataTextFormatTestUtil {
// Can't use an anonymous namespace here due to brokenness of Tru64 compiler.
namespace text_format_unittest {

using ::google::protobuf::internal::kDebugStringSilentMarker;
using ::google::protobuf::internal::UnsetFieldsMetadataTextFormatTestUtil;
using ::testing::AllOf;
using ::testing::HasSubstr;
Expand All @@ -94,10 +93,16 @@ constexpr absl::string_view kEscapeTestStringEscaped =

constexpr absl::string_view value_replacement = "\\[REDACTED\\]";

constexpr absl::string_view kTextMarkerRegex = "goo\\.gle/.+ +";

class TextFormatTestBase : public testing::Test {
public:
void SetUp() override {
single_line_debug_format_prefix_ = "";
// DebugString APIs insert a per-process randomized
// prefix. Here we obtain the prefixes by calling DebugString APIs on an
// empty proto. Note that Message::ShortDebugString() trims the last empty
// space so we have to add it back.
single_line_debug_format_prefix_ = proto_.ShortDebugString() + " ";
multi_line_debug_format_prefix_ = proto_.DebugString();
}

Expand Down Expand Up @@ -210,6 +215,7 @@ TEST_F(TextFormatTest, ShortFormat) {
std::string value_replacement = "\\[REDACTED\\]";
EXPECT_THAT(google::protobuf::ShortFormat(proto),
testing::MatchesRegex(absl::Substitute(
"$1"
"optional_redacted_string: $0 "
"optional_unredacted_string: \"bar\" "
"repeated_redacted_string: $0 "
Expand All @@ -226,7 +232,7 @@ TEST_F(TextFormatTest, ShortFormat) {
"\\{ optional_unredacted_nested_string: \"8\" \\} "
"map_redacted_string: $0 "
"map_unredacted_string \\{ key: \"ghi\" value: \"jkl\" \\}",
value_replacement)));
value_replacement, kTextMarkerRegex)));
}

TEST_F(TextFormatTest, Utf8Format) {
Expand Down Expand Up @@ -262,6 +268,7 @@ TEST_F(TextFormatTest, Utf8Format) {

EXPECT_THAT(google::protobuf::Utf8Format(proto),
testing::MatchesRegex(absl::Substitute(
"$1\n"
"optional_redacted_string: $0\n"
"optional_unredacted_string: \"bar\"\n"
"repeated_redacted_string: $0\n"
Expand All @@ -280,7 +287,7 @@ TEST_F(TextFormatTest, Utf8Format) {
"map_redacted_string: $0\n"
"map_unredacted_string \\{\n "
"key: \"ghi\"\n value: \"jkl\"\n\\}\n",
value_replacement)));
value_replacement, kTextMarkerRegex)));
}

TEST_F(TextFormatTest, ShortPrimitiveRepeateds) {
Expand Down Expand Up @@ -433,6 +440,7 @@ TEST_F(TextFormatTest, PrintUnknownFields) {
message_text);

EXPECT_THAT(absl::StrCat(message), testing::MatchesRegex(absl::Substitute(
"$1\n"
"5: UNKNOWN_VARINT $0\n"
"5: UNKNOWN_FIXED32 $0\n"
"5: UNKNOWN_FIXED64 $0\n"
Expand All @@ -441,7 +449,7 @@ TEST_F(TextFormatTest, PrintUnknownFields) {
"8: UNKNOWN_VARINT $0\n"
"8: UNKNOWN_VARINT $0\n"
"8: UNKNOWN_VARINT $0\n",
value_replacement)));
value_replacement, kTextMarkerRegex)));
}

TEST_F(TextFormatTest, PrintUnknownFieldsDeepestStackWorks) {
Expand Down Expand Up @@ -2679,6 +2687,13 @@ TEST(TextFormatUnknownFieldTest, TestUnknownExtension) {
EXPECT_FALSE(parser.ParseFromString("unknown_field: 1", &proto));
}

TEST(AbslStringifyTest, DebugStringIsTheSame) {
unittest::TestAllTypes proto;
proto.set_optional_int32(1);
proto.set_optional_string("foo");

EXPECT_THAT(proto.DebugString(), absl::StrCat(proto));
}

TEST(AbslStringifyTest, TextFormatIsUnchanged) {
unittest::TestAllTypes proto;
Expand All @@ -2698,34 +2713,40 @@ TEST(AbslStringifyTest, StringifyHasRedactionMarker) {
proto.set_optional_int32(1);
proto.set_optional_string("foo");

EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(
EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute(
"$0\n"
"optional_int32: 1\n"
"optional_string: \"foo\"\n"));
"optional_string: \"foo\"\n",
kTextMarkerRegex)));
}


TEST(AbslStringifyTest, StringifyMetaAnnotatedIsRedacted) {
unittest::TestRedactedMessage proto;
proto.set_meta_annotated("foo");
EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute(
"meta_annotated: $0\n",
value_replacement)));
"$0\n"
"meta_annotated: $1\n",
kTextMarkerRegex, value_replacement)));
}

TEST(AbslStringifyTest, StringifyRepeatedMetaAnnotatedIsRedacted) {
unittest::TestRedactedMessage proto;
proto.set_repeated_meta_annotated("foo");
EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute(
"repeated_meta_annotated: $0\n",
value_replacement)));
"$0\n"
"repeated_meta_annotated: $1\n",
kTextMarkerRegex, value_replacement)));
}

TEST(AbslStringifyTest, StringifyRepeatedMetaAnnotatedIsNotRedacted) {
unittest::TestRedactedMessage proto;
proto.set_unredacted_repeated_annotations("foo");
EXPECT_THAT(absl::StrCat(proto),
testing::MatchesRegex(
"unredacted_repeated_annotations: \"foo\"\n"));
absl::Substitute("$0\n"
"unredacted_repeated_annotations: \"foo\"\n",
kTextMarkerRegex)));
}

TEST(AbslStringifyTest, TextFormatMetaAnnotatedIsNotRedacted) {
Expand All @@ -2739,23 +2760,26 @@ TEST(AbslStringifyTest, StringifyDirectMessageEnumIsRedacted) {
unittest::TestRedactedMessage proto;
proto.set_test_direct_message_enum("foo");
EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute(
"test_direct_message_enum: $0\n",
value_replacement)));
"$0\n"
"test_direct_message_enum: $1\n",
kTextMarkerRegex, value_replacement)));
}
TEST(AbslStringifyTest, StringifyNestedMessageEnumIsRedacted) {
unittest::TestRedactedMessage proto;
proto.set_test_nested_message_enum("foo");
EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute(
"test_nested_message_enum: $0\n",
value_replacement)));
"$0\n"
"test_nested_message_enum: $1\n",
kTextMarkerRegex, value_replacement)));
}

TEST(AbslStringifyTest, StringifyRedactedOptionDoesNotRedact) {
unittest::TestRedactedMessage proto;
proto.set_test_redacted_message_enum("foo");
EXPECT_THAT(absl::StrCat(proto),
testing::MatchesRegex(
"test_redacted_message_enum: \"foo\"\n"));
EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute(
"$0\n"
"test_redacted_message_enum: \"foo\"\n",
kTextMarkerRegex)));
}


Expand Down

0 comments on commit 9a03332

Please sign in to comment.