diff --git a/src/core/ext/transport/binder/wire_format/wire_reader_impl.cc b/src/core/ext/transport/binder/wire_format/wire_reader_impl.cc index 7b1777ba792..fbce41620c2 100644 --- a/src/core/ext/transport/binder/wire_format/wire_reader_impl.cc +++ b/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 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); diff --git a/test/core/transport/binder/wire_reader_test.cc b/test/core/transport/binder/wire_reader_test.cc index ec30b7acdd9..4193022a2c6 100644 --- a/test/core/transport/binder/wire_reader_test.cc +++ b/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>()), - 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>(); + wire_reader_ = std::make_shared( + 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()); + wire_reader_->SetupTransport(absl::make_unique()); } template absl::Status CallProcessTransaction(T tx_code) { - return wire_reader_.ProcessTransaction( + return wire_reader_->ProcessTransaction( static_cast(tx_code), &mock_readable_parcel_, /*uid=*/0); } std::shared_ptr> transport_stream_receiver_; - WireReaderImpl wire_reader_; + std::shared_ptr 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> 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) {