diff --git a/src/core/ext/transport/binder/server/binder_server.cc b/src/core/ext/transport/binder/server/binder_server.cc index 31c79ab9960..994a04a0351 100644 --- a/src/core/ext/transport/binder/server/binder_server.cc +++ b/src/core/ext/transport/binder/server/binder_server.cc @@ -196,7 +196,9 @@ class BinderServerListener : public Server::ListenerInterface { return status; } gpr_log(GPR_INFO, "BinderTransport client protocol version = %d", version); - // TODO(waynetu): Check supported version. + // TODO(mingcl): Make sure we only give client a version that is not newer + // than the version they specify. For now, we always tell client that we + // only support version=1. std::unique_ptr client_binder{}; status = parcel->ReadBinder(&client_binder); if (!status.ok()) { 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 bba7c556631..7b1777ba792 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 @@ -42,6 +42,8 @@ namespace grpc_binder { namespace { +const int32_t kWireFormatVersion = 1; + absl::StatusOr parse_metadata(ReadableParcel* reader) { int num_header; RETURN_IF_ERROR(reader->ReadInt32(&num_header)); @@ -114,9 +116,8 @@ void WireReaderImpl::SendSetupTransport(Binder* binder) { gpr_log(GPR_INFO, "prepare transaction = %d", binder->PrepareTransaction().ok()); WritableParcel* writable_parcel = binder->GetWritableParcel(); - int32_t version = 77; gpr_log(GPR_INFO, "write int32 = %d", - writable_parcel->WriteInt32(version).ok()); + writable_parcel->WriteInt32(kWireFormatVersion).ok()); // The lifetime of the transaction receiver is the same as the wire writer's. // The transaction receiver is responsible for not calling the on-transact // callback when it's dead. @@ -194,7 +195,15 @@ absl::Status WireReaderImpl::ProcessTransaction(transaction_code_t code, int version; RETURN_IF_ERROR(parcel->ReadInt32(&version)); - gpr_log(GPR_INFO, "version = %d", version); + gpr_log(GPR_INFO, "The other end respond with version = %d", version); + // We only support this single lowest possible version, so server must + // respond that version too. + if (version != kWireFormatVersion) { + gpr_log(GPR_ERROR, + "The other end respond with version = %d, but we requested " + "version %d, trying to continue anyway", + version, kWireFormatVersion); + } std::unique_ptr binder{}; RETURN_IF_ERROR(parcel->ReadBinder(&binder)); if (!binder) { diff --git a/test/core/transport/binder/mock_objects.h b/test/core/transport/binder/mock_objects.h index 9a341ac493c..a48aa4e79a3 100644 --- a/test/core/transport/binder/mock_objects.h +++ b/test/core/transport/binder/mock_objects.h @@ -76,7 +76,13 @@ class MockTransactionReceiver : public TransactionReceiver { public: explicit MockTransactionReceiver(OnTransactCb transact_cb, BinderTransportTxCode code, - ReadableParcel* output) { + MockReadableParcel* output) { + if (code == BinderTransportTxCode::SETUP_TRANSPORT) { + EXPECT_CALL(*output, ReadInt32).WillOnce([](int32_t* version) { + *version = 1; + return absl::OkStatus(); + }); + } transact_cb(static_cast(code), output, /*uid=*/0) .IgnoreError(); } diff --git a/test/core/transport/binder/wire_reader_test.cc b/test/core/transport/binder/wire_reader_test.cc index cc6f7398628..ec30b7acdd9 100644 --- a/test/core/transport/binder/wire_reader_test.cc +++ b/test/core/transport/binder/wire_reader_test.cc @@ -114,19 +114,7 @@ TEST_F(WireReaderTest, SetupTransport) { EXPECT_CALL(mock_binder_ref, GetWritableParcel); // Write version. - EXPECT_CALL(mock_binder_ref.GetWriter(), WriteInt32(77)); - - // The transaction receiver immediately informs the wire writer that the - // transport has been successfully set up. - EXPECT_CALL(mock_binder_ref, ConstructTxReceiver); - - EXPECT_CALL(mock_binder_ref.GetReader(), ReadInt32); - EXPECT_CALL(mock_binder_ref.GetReader(), ReadBinder); - - // Write transaction receiver. - EXPECT_CALL(mock_binder_ref.GetWriter(), WriteBinder); - // Perform transaction. - EXPECT_CALL(mock_binder_ref, Transact); + EXPECT_CALL(mock_binder_ref.GetWriter(), WriteInt32(1)); wire_reader_.SetupTransport(std::move(mock_binder)); }