Revert "Revert "[BinderTransport] Send correct version to server and verify it (#27990)" (#28090)" (#28168)

This reverts commit ea49e4c73a.

msan test failed before because the mock object returns uninitialized
value and the code tries to conditionally log when the value is wrong.
pull/28297/head
Ming-Chuan 3 years ago committed by GitHub
parent 7cb8588501
commit c952e9be63
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 4
      src/core/ext/transport/binder/server/binder_server.cc
  2. 15
      src/core/ext/transport/binder/wire_format/wire_reader_impl.cc
  3. 8
      test/core/transport/binder/mock_objects.h
  4. 14
      test/core/transport/binder/wire_reader_test.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<grpc_binder::Binder> client_binder{};
status = parcel->ReadBinder(&client_binder);
if (!status.ok()) {

@ -42,6 +42,8 @@
namespace grpc_binder {
namespace {
const int32_t kWireFormatVersion = 1;
absl::StatusOr<Metadata> 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> binder{};
RETURN_IF_ERROR(parcel->ReadBinder(&binder));
if (!binder) {

@ -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<transaction_code_t>(code), output, /*uid=*/0)
.IgnoreError();
}

@ -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));
}

Loading…
Cancel
Save