[BinderTransport] add authority to initial metadata if missing (#29035)

When C++ implementation acts as server and Java implementation acts as
client, authority will not be present in initial metadata. Although
authority is not used for binder transport, when it is missing, core API
layer will fail the RPC call silently.

This commit adds a fake authority to initial metadata when it is
missing to avoid the failure.

Test case is added to make sure the wire reader implementation appends
both path and authority to metadata correctly.

src/core/ext/transport/binder/wire_format/wire_reader_impl.cc

@@ -43,6 +43,7 @@ namespace grpc_binder {
 namespace {
 
 const int32_t kWireFormatVersion = 1;
+const char kAuthorityMetadataKey[] = ":authority";
 
 absl::StatusOr<Metadata> parse_metadata(ReadableParcel* reader) {
   int num_header;
@@ -342,8 +343,23 @@ absl::Status WireReaderImpl::ProcessStreamingTransactionImpl(
       return initial_metadata_or_error.status();
     }
     if (!is_client_) {
+      // In BinderChannel wireformat specification, path is not encoded as part
+      // of metadata. So we extract the path and turn it into metadata here
+      // (this is what core API layer expects).
       initial_metadata_or_error->emplace_back(":path",
                                               std::string("/") + method_ref);
+      // Since authority metadata is not part of BinderChannel wireformat
+      // specification, and gRPC core API layer expects the presence of
+      // authority for message sent from client to server, we add one if
+      // missing (it will be missing if client grpc-java).
+      bool has_authority = false;
+      for (const auto& p : *initial_metadata_or_error) {
+        if (p.first == kAuthorityMetadataKey) has_authority = true;
+      }
+      if (!has_authority) {
+        initial_metadata_or_error->emplace_back(kAuthorityMetadataKey,
+                                                "binder.authority");
+      }
     }
     transport_stream_receiver_->NotifyRecvInitialMetadata(
         code, *initial_metadata_or_error);

test/core/transport/binder/wire_reader_test.cc

@@ -44,16 +44,17 @@ using ::testing::StrictMock;
 namespace {
 
 class WireReaderTest : public ::testing::Test {
- public:
-  WireReaderTest()
-      : transport_stream_receiver_(
-            std::make_shared<StrictMock<MockTransportStreamReceiver>>()),
-        wire_reader_(
-            transport_stream_receiver_, /*is_client=*/true,
-            std::make_shared<
-                grpc::experimental::binder::UntrustedSecurityPolicy>()) {}
-
  protected:
+  void SetUp() override { SetUp(true); }
+  void SetUp(bool is_client) {
+    transport_stream_receiver_ =
+        std::make_shared<StrictMock<MockTransportStreamReceiver>>();
+    wire_reader_ = std::make_shared<WireReaderImpl>(
+        transport_stream_receiver_, is_client,
+        std::make_shared<
+            grpc::experimental::binder::UntrustedSecurityPolicy>());
+  }
+
   void ExpectReadInt32(int result) {
     EXPECT_CALL(mock_readable_parcel_, ReadInt32)
         .WillOnce(DoAll(SetArgPointee<0>(result), Return(absl::OkStatus())));
@@ -70,24 +71,32 @@ class WireReaderTest : public ::testing::Test {
     }
   }
 
+  void ExpectReadString(const std::string& str) {
+    EXPECT_CALL(mock_readable_parcel_, ReadString)
+        .WillOnce([str](std::string* out) {
+          *out = str;
+          return absl::OkStatus();
+        });
+  }
+
   void UnblockSetupTransport() {
     // SETUP_TRANSPORT should finish before we can proceed with any other
     // requests and streaming calls. The MockBinder will construct a
     // MockTransactionReceiver, which will then sends SETUP_TRANSPORT request
     // back to us.
-    wire_reader_.SetupTransport(absl::make_unique<MockBinder>());
+    wire_reader_->SetupTransport(absl::make_unique<MockBinder>());
   }
 
   template <typename T>
   absl::Status CallProcessTransaction(T tx_code) {
-    return wire_reader_.ProcessTransaction(
+    return wire_reader_->ProcessTransaction(
         static_cast<transaction_code_t>(tx_code), &mock_readable_parcel_,
         /*uid=*/0);
   }
 
   std::shared_ptr<StrictMock<MockTransportStreamReceiver>>
       transport_stream_receiver_;
-  WireReaderImpl wire_reader_;
+  std::shared_ptr<WireReaderImpl> wire_reader_;
   MockReadableParcel mock_readable_parcel_;
 };
 
@@ -116,7 +125,7 @@ TEST_F(WireReaderTest, SetupTransport) {
   // Write version.
   EXPECT_CALL(mock_binder_ref.GetWriter(), WriteInt32(1));
 
-  wire_reader_.SetupTransport(std::move(mock_binder));
+  wire_reader_->SetupTransport(std::move(mock_binder));
 }
 
 TEST_F(WireReaderTest, ProcessTransactionControlMessageSetupTransport) {
@@ -307,6 +316,55 @@ TEST_F(WireReaderTest, InBoundFlowControl) {
   EXPECT_TRUE(CallProcessTransaction(kFirstCallId).ok());
 }
 
+TEST_F(WireReaderTest, ServerInitialMetadata) {
+  SetUp(/*is_client=*/false);
+
+  ::testing::InSequence sequence;
+  UnblockSetupTransport();
+
+  // flag
+  ExpectReadInt32(kFlagPrefix);
+  // sequence number
+  ExpectReadInt32(0);
+
+  const std::vector<std::pair<std::string, std::string>> kMetadata = {
+      {"", ""},
+      {"", "value"},
+      {"key", ""},
+      {"key", "value"},
+      {"another-key", "another-value"},
+  };
+
+  // method ref
+  ExpectReadString("test.service/rpc.method");
+
+  // metadata
+  {
+    // count
+    ExpectReadInt32(kMetadata.size());
+    for (const auto& md : kMetadata) {
+      // metadata key
+      ExpectReadByteArray(md.first);
+      // metadata val
+      // TODO(waynetu): metadata value can also be "parcelable".
+      ExpectReadByteArray(md.second);
+    }
+  }
+
+  // Since path and authority is not encoded as metadata in wire format,
+  // wire_reader implementation should insert them as metadata before passing
+  // to transport layer.
+  auto metadata_expectation = kMetadata;
+  metadata_expectation.push_back({":path", "/test.service/rpc.method"});
+  metadata_expectation.push_back({":authority", "binder.authority"});
+
+  EXPECT_CALL(*transport_stream_receiver_,
+              NotifyRecvInitialMetadata(
+                  kFirstCallId, StatusOrContainerEq(metadata_expectation)));
+
+  EXPECT_TRUE(CallProcessTransaction(kFirstCallId).ok());
+}
+
 }  // namespace grpc_binder
 
 int main(int argc, char** argv) {