Skip to content

Commit 9a03332

Browse files
tanx16copybara-github
authored andcommitted
Make DebugString print debug output, enable debug markers for debug output
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
1 parent a8fca08 commit 9a03332

File tree

3 files changed

+123
-20
lines changed

3 files changed

+123
-20
lines changed

src/google/protobuf/text_format.cc

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,12 @@
2121
#include <cstdint>
2222
#include <limits>
2323
#include <memory>
24+
#include <random>
2425
#include <string>
2526
#include <utility>
2627
#include <vector>
2728

29+
#include "absl/base/macros.h"
2830
#include "absl/container/btree_set.h"
2931
#include "absl/log/absl_check.h"
3032
#include "absl/strings/ascii.h"
@@ -36,6 +38,8 @@
3638
#include "absl/strings/str_format.h"
3739
#include "absl/strings/str_join.h"
3840
#include "absl/strings/string_view.h"
41+
#include "absl/time/clock.h"
42+
#include "absl/time/time.h"
3943
#include "google/protobuf/any.h"
4044
#include "google/protobuf/descriptor.h"
4145
#include "google/protobuf/descriptor.pb.h"
@@ -99,7 +103,7 @@ const char kDebugStringSilentMarkerForDetection[] = "\t ";
99103

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

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

2306+
internal::PrintTextMarker(&generator, redact_debug_string_,
2307+
randomize_debug_string_, single_line_mode_);
2308+
23022309

23032310
Print(message, &generator);
23042311

@@ -3109,6 +3116,74 @@ bool TextFormat::Printer::TryRedactFieldValue(
31093116
}
31103117
return false;
31113118
}
3119+
3120+
class TextMarkerGenerator final {
3121+
public:
3122+
static TextMarkerGenerator CreateRandom();
3123+
3124+
void PrintMarker(TextFormat::BaseTextGenerator* generator, bool redact,
3125+
bool randomize, bool single_line_mode) const {
3126+
if (redact) {
3127+
generator->Print(redaction_marker_.data(), redaction_marker_.size());
3128+
}
3129+
if (randomize) {
3130+
generator->Print(random_marker_.data(), random_marker_.size());
3131+
}
3132+
if ((redact || randomize) && !single_line_mode) {
3133+
generator->PrintLiteral("\n");
3134+
}
3135+
}
3136+
3137+
private:
3138+
static constexpr absl::string_view kRedactionMarkers[] = {
3139+
"goo.gle/debugonly ", "goo.gle/nodeserialize ", "goo.gle/debugstr ",
3140+
"goo.gle/debugproto "};
3141+
3142+
static constexpr absl::string_view kRandomMarker = " ";
3143+
3144+
static_assert(!kRandomMarker.empty(), "The random marker cannot be empty!");
3145+
3146+
constexpr TextMarkerGenerator(absl::string_view redaction_marker,
3147+
absl::string_view random_marker)
3148+
: redaction_marker_(redaction_marker), random_marker_(random_marker) {}
3149+
3150+
absl::string_view redaction_marker_;
3151+
absl::string_view random_marker_;
3152+
};
3153+
3154+
TextMarkerGenerator TextMarkerGenerator::CreateRandom() {
3155+
// We avoid using sources backed by system entropy to allow the marker
3156+
// generator to work in sandboxed environments that have no access to syscalls
3157+
// such as getrandom or getpid. Note that this randomization has no security
3158+
// implications, it's only used to break code that attempts to deserialize
3159+
// debug strings.
3160+
std::mt19937_64 random{
3161+
static_cast<uint64_t>(absl::ToUnixMicros(absl::Now()))};
3162+
3163+
size_t redaction_marker_index = std::uniform_int_distribution<size_t>{
3164+
0, ABSL_ARRAYSIZE(kRedactionMarkers) - 1}(random);
3165+
3166+
size_t random_marker_size =
3167+
std::uniform_int_distribution<size_t>{1, kRandomMarker.size()}(random);
3168+
3169+
return TextMarkerGenerator(kRedactionMarkers[redaction_marker_index],
3170+
kRandomMarker.substr(0, random_marker_size));
3171+
}
3172+
3173+
const TextMarkerGenerator& GetGlobalTextMarkerGenerator() {
3174+
static const TextMarkerGenerator kTextMarkerGenerator =
3175+
TextMarkerGenerator::CreateRandom();
3176+
return kTextMarkerGenerator;
3177+
}
3178+
3179+
namespace internal {
3180+
void PrintTextMarker(TextFormat::BaseTextGenerator* generator, bool redact,
3181+
bool randomize, bool single_line_mode) {
3182+
GetGlobalTextMarkerGenerator().PrintMarker(generator, redact, randomize,
3183+
single_line_mode);
3184+
}
3185+
} // namespace internal
3186+
31123187
} // namespace protobuf
31133188
} // namespace google
31143189

src/google/protobuf/text_format.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,10 @@ class PROTOBUF_EXPORT TextFormat {
829829
const T&... values);
830830
};
831831

832+
namespace internal {
833+
void PrintTextMarker(TextFormat::BaseTextGenerator* generator, bool redact,
834+
bool randomize, bool single_line_mode);
835+
} // namespace internal
832836

833837
inline void TextFormat::RecordLocation(ParseInfoTree* info_tree,
834838
const FieldDescriptor* field,

src/google/protobuf/text_format_unittest.cc

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ class UnsetFieldsMetadataTextFormatTestUtil {
7676
// Can't use an anonymous namespace here due to brokenness of Tru64 compiler.
7777
namespace text_format_unittest {
7878

79-
using ::google::protobuf::internal::kDebugStringSilentMarker;
8079
using ::google::protobuf::internal::UnsetFieldsMetadataTextFormatTestUtil;
8180
using ::testing::AllOf;
8281
using ::testing::HasSubstr;
@@ -94,10 +93,16 @@ constexpr absl::string_view kEscapeTestStringEscaped =
9493

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

96+
constexpr absl::string_view kTextMarkerRegex = "goo\\.gle/.+ +";
97+
9798
class TextFormatTestBase : public testing::Test {
9899
public:
99100
void SetUp() override {
100-
single_line_debug_format_prefix_ = "";
101+
// DebugString APIs insert a per-process randomized
102+
// prefix. Here we obtain the prefixes by calling DebugString APIs on an
103+
// empty proto. Note that Message::ShortDebugString() trims the last empty
104+
// space so we have to add it back.
105+
single_line_debug_format_prefix_ = proto_.ShortDebugString() + " ";
101106
multi_line_debug_format_prefix_ = proto_.DebugString();
102107
}
103108

@@ -210,6 +215,7 @@ TEST_F(TextFormatTest, ShortFormat) {
210215
std::string value_replacement = "\\[REDACTED\\]";
211216
EXPECT_THAT(google::protobuf::ShortFormat(proto),
212217
testing::MatchesRegex(absl::Substitute(
218+
"$1"
213219
"optional_redacted_string: $0 "
214220
"optional_unredacted_string: \"bar\" "
215221
"repeated_redacted_string: $0 "
@@ -226,7 +232,7 @@ TEST_F(TextFormatTest, ShortFormat) {
226232
"\\{ optional_unredacted_nested_string: \"8\" \\} "
227233
"map_redacted_string: $0 "
228234
"map_unredacted_string \\{ key: \"ghi\" value: \"jkl\" \\}",
229-
value_replacement)));
235+
value_replacement, kTextMarkerRegex)));
230236
}
231237

232238
TEST_F(TextFormatTest, Utf8Format) {
@@ -262,6 +268,7 @@ TEST_F(TextFormatTest, Utf8Format) {
262268

263269
EXPECT_THAT(google::protobuf::Utf8Format(proto),
264270
testing::MatchesRegex(absl::Substitute(
271+
"$1\n"
265272
"optional_redacted_string: $0\n"
266273
"optional_unredacted_string: \"bar\"\n"
267274
"repeated_redacted_string: $0\n"
@@ -280,7 +287,7 @@ TEST_F(TextFormatTest, Utf8Format) {
280287
"map_redacted_string: $0\n"
281288
"map_unredacted_string \\{\n "
282289
"key: \"ghi\"\n value: \"jkl\"\n\\}\n",
283-
value_replacement)));
290+
value_replacement, kTextMarkerRegex)));
284291
}
285292

286293
TEST_F(TextFormatTest, ShortPrimitiveRepeateds) {
@@ -433,6 +440,7 @@ TEST_F(TextFormatTest, PrintUnknownFields) {
433440
message_text);
434441

435442
EXPECT_THAT(absl::StrCat(message), testing::MatchesRegex(absl::Substitute(
443+
"$1\n"
436444
"5: UNKNOWN_VARINT $0\n"
437445
"5: UNKNOWN_FIXED32 $0\n"
438446
"5: UNKNOWN_FIXED64 $0\n"
@@ -441,7 +449,7 @@ TEST_F(TextFormatTest, PrintUnknownFields) {
441449
"8: UNKNOWN_VARINT $0\n"
442450
"8: UNKNOWN_VARINT $0\n"
443451
"8: UNKNOWN_VARINT $0\n",
444-
value_replacement)));
452+
value_replacement, kTextMarkerRegex)));
445453
}
446454

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

2690+
TEST(AbslStringifyTest, DebugStringIsTheSame) {
2691+
unittest::TestAllTypes proto;
2692+
proto.set_optional_int32(1);
2693+
proto.set_optional_string("foo");
2694+
2695+
EXPECT_THAT(proto.DebugString(), absl::StrCat(proto));
2696+
}
26822697

26832698
TEST(AbslStringifyTest, TextFormatIsUnchanged) {
26842699
unittest::TestAllTypes proto;
@@ -2698,34 +2713,40 @@ TEST(AbslStringifyTest, StringifyHasRedactionMarker) {
26982713
proto.set_optional_int32(1);
26992714
proto.set_optional_string("foo");
27002715

2701-
EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(
2716+
EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute(
2717+
"$0\n"
27022718
"optional_int32: 1\n"
2703-
"optional_string: \"foo\"\n"));
2719+
"optional_string: \"foo\"\n",
2720+
kTextMarkerRegex)));
27042721
}
27052722

27062723

27072724
TEST(AbslStringifyTest, StringifyMetaAnnotatedIsRedacted) {
27082725
unittest::TestRedactedMessage proto;
27092726
proto.set_meta_annotated("foo");
27102727
EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute(
2711-
"meta_annotated: $0\n",
2712-
value_replacement)));
2728+
"$0\n"
2729+
"meta_annotated: $1\n",
2730+
kTextMarkerRegex, value_replacement)));
27132731
}
27142732

27152733
TEST(AbslStringifyTest, StringifyRepeatedMetaAnnotatedIsRedacted) {
27162734
unittest::TestRedactedMessage proto;
27172735
proto.set_repeated_meta_annotated("foo");
27182736
EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute(
2719-
"repeated_meta_annotated: $0\n",
2720-
value_replacement)));
2737+
"$0\n"
2738+
"repeated_meta_annotated: $1\n",
2739+
kTextMarkerRegex, value_replacement)));
27212740
}
27222741

27232742
TEST(AbslStringifyTest, StringifyRepeatedMetaAnnotatedIsNotRedacted) {
27242743
unittest::TestRedactedMessage proto;
27252744
proto.set_unredacted_repeated_annotations("foo");
27262745
EXPECT_THAT(absl::StrCat(proto),
27272746
testing::MatchesRegex(
2728-
"unredacted_repeated_annotations: \"foo\"\n"));
2747+
absl::Substitute("$0\n"
2748+
"unredacted_repeated_annotations: \"foo\"\n",
2749+
kTextMarkerRegex)));
27292750
}
27302751

27312752
TEST(AbslStringifyTest, TextFormatMetaAnnotatedIsNotRedacted) {
@@ -2739,23 +2760,26 @@ TEST(AbslStringifyTest, StringifyDirectMessageEnumIsRedacted) {
27392760
unittest::TestRedactedMessage proto;
27402761
proto.set_test_direct_message_enum("foo");
27412762
EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute(
2742-
"test_direct_message_enum: $0\n",
2743-
value_replacement)));
2763+
"$0\n"
2764+
"test_direct_message_enum: $1\n",
2765+
kTextMarkerRegex, value_replacement)));
27442766
}
27452767
TEST(AbslStringifyTest, StringifyNestedMessageEnumIsRedacted) {
27462768
unittest::TestRedactedMessage proto;
27472769
proto.set_test_nested_message_enum("foo");
27482770
EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute(
2749-
"test_nested_message_enum: $0\n",
2750-
value_replacement)));
2771+
"$0\n"
2772+
"test_nested_message_enum: $1\n",
2773+
kTextMarkerRegex, value_replacement)));
27512774
}
27522775

27532776
TEST(AbslStringifyTest, StringifyRedactedOptionDoesNotRedact) {
27542777
unittest::TestRedactedMessage proto;
27552778
proto.set_test_redacted_message_enum("foo");
2756-
EXPECT_THAT(absl::StrCat(proto),
2757-
testing::MatchesRegex(
2758-
"test_redacted_message_enum: \"foo\"\n"));
2779+
EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute(
2780+
"$0\n"
2781+
"test_redacted_message_enum: \"foo\"\n",
2782+
kTextMarkerRegex)));
27592783
}
27602784

27612785

0 commit comments

Comments
 (0)