Support hex strings to prevent broken log lines

Firestore/core/src/remote/grpc_stream.cc

@@ -98,7 +98,7 @@ GrpcStream::GrpcStream(
 }
 
 GrpcStream::~GrpcStream() {
-  LOG_DEBUG("GrpcStream('%s'): destroying stream", this);
+  LOG_DEBUG("GrpcStream('%x'): destroying stream", this);
   HARD_ASSERT(completions_.empty(),
               "GrpcStream is being destroyed without proper shutdown");
   MaybeUnregister();
@@ -160,14 +160,14 @@ void GrpcStream::MaybeWrite(absl::optional<BufferedWrite> maybe_write) {
 }
 
 void GrpcStream::FinishImmediately() {
-  LOG_DEBUG("GrpcStream('%s'): finishing without notifying observers", this);
+  LOG_DEBUG("GrpcStream('%x'): finishing without notifying observers", this);
 
   Shutdown();
   UnsetObserver();
 }
 
 void GrpcStream::FinishAndNotify(const Status& status) {
-  LOG_DEBUG("GrpcStream('%s'): finishing and notifying observers", this);
+  LOG_DEBUG("GrpcStream('%x'): finishing and notifying observers", this);
 
   Shutdown();
 
@@ -181,7 +181,7 @@ void GrpcStream::FinishAndNotify(const Status& status) {
 }
 
 void GrpcStream::Shutdown() {
-  LOG_DEBUG("GrpcStream('%s'): shutting down; completions: %s, is finished: %s",
+  LOG_DEBUG("GrpcStream('%x'): shutting down; completions: %s, is finished: %s",
             this, completions_.size(), is_grpc_call_finished_);
 
   MaybeUnregister();
@@ -216,7 +216,7 @@ void GrpcStream::MaybeUnregister() {
 }
 
 void GrpcStream::FinishGrpcCall(const OnSuccess& callback) {
-  LOG_DEBUG("GrpcStream('%s'): finishing the underlying call", this);
+  LOG_DEBUG("GrpcStream('%x'): finishing the underlying call", this);
 
   HARD_ASSERT(!is_grpc_call_finished_, "FinishGrpcCall called twice");
   is_grpc_call_finished_ = true;
@@ -229,7 +229,7 @@ void GrpcStream::FinishGrpcCall(const OnSuccess& callback) {
 }
 
 void GrpcStream::FastFinishCompletionsBlocking() {
-  LOG_DEBUG("GrpcStream('%s'): fast finishing %s completion(s)", this,
+  LOG_DEBUG("GrpcStream('%x'): fast finishing %s completion(s)", this,
             completions_.size());
 
   // TODO(varconst): reset buffered_writer_? Should not be necessary, because it
@@ -344,7 +344,7 @@ std::shared_ptr<GrpcCompletion> GrpcStream::NewCompletion(
         } else {
           // Use the same error-handling for all operations; all errors are
           // unrecoverable.
-          LOG_DEBUG("GrpcStream('%s'): operation of type %s failed", this,
+          LOG_DEBUG("GrpcStream('%x'): operation of type %s failed", this,
                     completion->type());
           OnOperationFailed();
         }

Firestore/core/src/remote/remote_store.cc

@@ -85,14 +85,14 @@ void RemoteStore::Start() {
       [this](ConnectivityMonitor::NetworkStatus network_status) {
         if (network_status == ConnectivityMonitor::NetworkStatus::Unavailable) {
           LOG_DEBUG(
-              "RemoteStore %s ignoring connectivity callback for unavailable "
+              "RemoteStore %x ignoring connectivity callback for unavailable "
               "network",
               this);
           return;
         }
 
         if (CanUseNetwork()) {
-          LOG_DEBUG("RemoteStore %s restarting streams as connectivity changed",
+          LOG_DEBUG("RemoteStore %x restarting streams as connectivity changed",
                     this);
           RestartNetwork();
         }
@@ -139,7 +139,7 @@ void RemoteStore::DisableNetworkInternal() {
 }
 
 void RemoteStore::Shutdown() {
-  LOG_DEBUG("RemoteStore %s shutting down", this);
+  LOG_DEBUG("RemoteStore %x shutting down", this);
   is_network_enabled_ = false;
   DisableNetworkInternal();
 
@@ -514,7 +514,7 @@ void RemoteStore::HandleHandshakeError(const Status& status) {
   if (Datastore::IsPermanentError(status)) {
     std::string token = util::ToString(write_stream_->last_stream_token());
     LOG_DEBUG(
-        "RemoteStore %s error before completed handshake; resetting "
+        "RemoteStore %x error before completed handshake; resetting "
         "stream token %s: "
         "error code: '%s', details: '%s'",
         this, token, status.code(), status.error_message());
@@ -590,7 +590,7 @@ void RemoteStore::HandleCredentialChange() {
     // Tear down and re-create our network streams. This will ensure we get a
     // fresh auth token for the new user and re-fill the write pipeline with new
     // mutations from the `LocalStore` (since mutations are per-user).
-    LOG_DEBUG("RemoteStore %s restarting streams for new credential", this);
+    LOG_DEBUG("RemoteStore %x restarting streams for new credential", this);
     RestartNetwork();
   }
 }

Firestore/core/src/remote/stream.cc

@@ -380,7 +380,7 @@ void Stream::Write(grpc::ByteBuffer&& message) {
 
 std::string Stream::GetDebugDescription() const {
   EnsureOnQueue();
-  return StringFormat("%s (%s)", GetDebugName(), this);
+  return StringFormat("%s (%x)", GetDebugName(), this);
 }
 
 }  // namespace remote

Firestore/core/src/util/string_format.cc

@@ -16,6 +16,10 @@
 
 #include "Firestore/core/src/util/string_format.h"
 
+#include <string>
+#include "absl/strings/escaping.h"
+#include "absl/strings/string_view.h"
+
 namespace firebase {
 namespace firestore {
 namespace util {
@@ -39,7 +43,7 @@ __attribute__((no_sanitize_address)) std::string StringFormatPieces(
 
   auto pieces_iter = pieces.begin();
   auto pieces_end = pieces.end();
-  auto append_next_piece = [&](std::string* dest) {
+  auto append_next_string_piece = [&](std::string* dest) {
     if (pieces_iter == pieces_end) {
       dest->append(kMissing);
     } else {
@@ -49,6 +53,17 @@ __attribute__((no_sanitize_address)) std::string StringFormatPieces(
     }
   };
 
+  auto append_next_hex_piece = [&](std::string* dest) {
+    if (pieces_iter == pieces_end) {
+      dest->append(kMissing);
+    } else {
+      std::string hex =
+          absl::BytesToHexString(absl::string_view(pieces_iter->data()));
+      dest->append(hex.data(), hex.size());
+      ++pieces_iter;
+    }
+  };
+
   auto append_specifier = [&](char spec) {
     switch (spec) {
       case '%':
@@ -57,7 +72,12 @@ __attribute__((no_sanitize_address)) std::string StringFormatPieces(
         break;
 
       case 's': {
-        append_next_piece(&result);
+        append_next_string_piece(&result);
+        break;
+      }
+
+      case 'x': {
+        append_next_hex_piece(&result);
         break;
       }
 

Firestore/core/test/unit/util/string_format_test.cc

@@ -88,6 +88,10 @@ TEST(StringFormatTest, Mixed) {
             StringFormat("%s%%%s%%%s%%%s", "World", true, 42, 1.5));
 }
 
+TEST(StringFormatTest, Hex) {
+  EXPECT_EQ("test=42", StringFormat("test=%x", "B"));
+}
+
 TEST(StringFormatTest, Literal) {
   EXPECT_EQ("Hello %", StringFormat("Hello %%"));
   EXPECT_EQ("% World", StringFormat("%% World"));