From 699c10386d6f0cbed7364e5a94144f0794f469d5 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Fri, 9 Nov 2018 19:43:00 -0800 Subject: Add method to fail recv msg for hijacked rpcs --- include/grpcpp/impl/codegen/call_op_set.h | 4 ++-- include/grpcpp/impl/codegen/interceptor.h | 3 +++ include/grpcpp/impl/codegen/interceptor_common.h | 14 +++++++++++++- include/grpcpp/impl/codegen/server_interface.h | 2 +- 4 files changed, 19 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/include/grpcpp/impl/codegen/call_op_set.h b/include/grpcpp/impl/codegen/call_op_set.h index b4c34a01c9..aae8b9d3e3 100644 --- a/include/grpcpp/impl/codegen/call_op_set.h +++ b/include/grpcpp/impl/codegen/call_op_set.h @@ -406,7 +406,7 @@ class CallOpRecvMessage { void SetInterceptionHookPoint( InterceptorBatchMethodsImpl* interceptor_methods) { - interceptor_methods->SetRecvMessage(message_); + interceptor_methods->SetRecvMessage(message_, &got_message); } void SetFinishInterceptionHookPoint( @@ -501,7 +501,7 @@ class CallOpGenericRecvMessage { void SetInterceptionHookPoint( InterceptorBatchMethodsImpl* interceptor_methods) { - interceptor_methods->SetRecvMessage(message_); + interceptor_methods->SetRecvMessage(message_, &got_message); } void SetFinishInterceptionHookPoint( diff --git a/include/grpcpp/impl/codegen/interceptor.h b/include/grpcpp/impl/codegen/interceptor.h index e449e44a23..943376a545 100644 --- a/include/grpcpp/impl/codegen/interceptor.h +++ b/include/grpcpp/impl/codegen/interceptor.h @@ -118,6 +118,9 @@ class InterceptorBatchMethods { // only interceptors after the current interceptor are created from the // factory objects registered with the channel. virtual std::unique_ptr GetInterceptedChannel() = 0; + + // On a hijacked RPC, an interceptor can decide to fail a RECV MESSAGE op. + virtual void FailHijackedRecvMessage() = 0; }; class Interceptor { diff --git a/include/grpcpp/impl/codegen/interceptor_common.h b/include/grpcpp/impl/codegen/interceptor_common.h index d0aa23cb0a..4c881c783d 100644 --- a/include/grpcpp/impl/codegen/interceptor_common.h +++ b/include/grpcpp/impl/codegen/interceptor_common.h @@ -134,7 +134,10 @@ class InterceptorBatchMethodsImpl send_trailing_metadata_ = metadata; } - void SetRecvMessage(void* message) { recv_message_ = message; } + void SetRecvMessage(void* message, bool* got_message) { + recv_message_ = message; + got_message_ = got_message; + } void SetRecvInitialMetadata(MetadataMap* map) { recv_initial_metadata_ = map; @@ -157,6 +160,8 @@ class InterceptorBatchMethodsImpl info->channel(), current_interceptor_index_ + 1)); } + void FailHijackedRecvMessage() override { *got_message_ = false; } + // Clears all state void ClearState() { reverse_ = false; @@ -345,6 +350,7 @@ class InterceptorBatchMethodsImpl std::multimap* send_trailing_metadata_ = nullptr; void* recv_message_ = nullptr; + bool* got_message_ = nullptr; MetadataMap* recv_initial_metadata_ = nullptr; @@ -451,6 +457,12 @@ class CancelInterceptorBatchMethods "method which has a Cancel notification"); return std::unique_ptr(nullptr); } + + void FailHijackedRecvMessage() override { + GPR_CODEGEN_ASSERT(false && + "It is illegal to call FailHijackedRecvMessage on a " + "method which has a Cancel notification"); + } }; } // namespace internal } // namespace grpc diff --git a/include/grpcpp/impl/codegen/server_interface.h b/include/grpcpp/impl/codegen/server_interface.h index 55c94f4d2f..23d1f445b1 100644 --- a/include/grpcpp/impl/codegen/server_interface.h +++ b/include/grpcpp/impl/codegen/server_interface.h @@ -270,7 +270,7 @@ class ServerInterface : public internal::CallHook { /* Set interception point for recv message */ interceptor_methods_.AddInterceptionHookPoint( experimental::InterceptionHookPoints::POST_RECV_MESSAGE); - interceptor_methods_.SetRecvMessage(request_); + interceptor_methods_.SetRecvMessage(request_, nullptr); return RegisteredAsyncRequest::FinalizeResult(tag, status); } -- cgit v1.2.3 From 565edf529766e66b4394f59ff33a89211c92206a Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Fri, 9 Nov 2018 19:51:11 -0800 Subject: Add safety checks --- include/grpcpp/impl/codegen/interceptor_common.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/include/grpcpp/impl/codegen/interceptor_common.h b/include/grpcpp/impl/codegen/interceptor_common.h index 4c881c783d..d23b71f8a7 100644 --- a/include/grpcpp/impl/codegen/interceptor_common.h +++ b/include/grpcpp/impl/codegen/interceptor_common.h @@ -160,7 +160,11 @@ class InterceptorBatchMethodsImpl info->channel(), current_interceptor_index_ + 1)); } - void FailHijackedRecvMessage() override { *got_message_ = false; } + void FailHijackedRecvMessage() override { + GPR_CODEGEN_ASSERT(hooks_[static_cast( + experimental::InterceptionHookPoints::PRE_RECV_MESSAGE)]); + *got_message_ = false; + } // Clears all state void ClearState() { -- cgit v1.2.3 From 5d7d6c0fbdcac75ea482e1fde3e128cd0c1646c1 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Wed, 14 Nov 2018 17:35:26 -0800 Subject: Add method to fail hijacked send messages --- include/grpcpp/impl/codegen/call_op_set.h | 10 ++- include/grpcpp/impl/codegen/interceptor.h | 4 ++ include/grpcpp/impl/codegen/interceptor_common.h | 18 +++++- .../end2end/client_interceptors_end2end_test.cc | 73 ++++++++++++++++++++++ 4 files changed, 102 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/include/grpcpp/impl/codegen/call_op_set.h b/include/grpcpp/impl/codegen/call_op_set.h index b4c34a01c9..1f2b88e9e1 100644 --- a/include/grpcpp/impl/codegen/call_op_set.h +++ b/include/grpcpp/impl/codegen/call_op_set.h @@ -314,14 +314,19 @@ class CallOpSendMessage { // Flags are per-message: clear them after use. write_options_.Clear(); } - void FinishOp(bool* status) { send_buf_.Clear(); } + void FinishOp(bool* status) { + send_buf_.Clear(); + if (hijacked_ && failed_send_) { + *status = false; + } + } void SetInterceptionHookPoint( InterceptorBatchMethodsImpl* interceptor_methods) { if (!send_buf_.Valid()) return; interceptor_methods->AddInterceptionHookPoint( experimental::InterceptionHookPoints::PRE_SEND_MESSAGE); - interceptor_methods->SetSendMessage(&send_buf_); + interceptor_methods->SetSendMessage(&send_buf_, &failed_send_); } void SetFinishInterceptionHookPoint( @@ -333,6 +338,7 @@ class CallOpSendMessage { private: bool hijacked_ = false; + bool failed_send_ = false; ByteBuffer send_buf_; WriteOptions write_options_; }; diff --git a/include/grpcpp/impl/codegen/interceptor.h b/include/grpcpp/impl/codegen/interceptor.h index e449e44a23..47239332c8 100644 --- a/include/grpcpp/impl/codegen/interceptor.h +++ b/include/grpcpp/impl/codegen/interceptor.h @@ -118,6 +118,10 @@ class InterceptorBatchMethods { // only interceptors after the current interceptor are created from the // factory objects registered with the channel. virtual std::unique_ptr GetInterceptedChannel() = 0; + + // On a hijacked RPC/ to-be hijacked RPC, this can be called to fail a SEND + // MESSAGE op + virtual void FailHijackedSendMessage() = 0; }; class Interceptor { diff --git a/include/grpcpp/impl/codegen/interceptor_common.h b/include/grpcpp/impl/codegen/interceptor_common.h index d0aa23cb0a..601a929afe 100644 --- a/include/grpcpp/impl/codegen/interceptor_common.h +++ b/include/grpcpp/impl/codegen/interceptor_common.h @@ -110,12 +110,21 @@ class InterceptorBatchMethodsImpl Status* GetRecvStatus() override { return recv_status_; } + void FailHijackedSendMessage() override { + GPR_CODEGEN_ASSERT(hooks_[static_cast( + experimental::InterceptionHookPoints::PRE_SEND_MESSAGE)]); + *fail_send_message_ = true; + } + std::multimap* GetRecvTrailingMetadata() override { return recv_trailing_metadata_->map(); } - void SetSendMessage(ByteBuffer* buf) { send_message_ = buf; } + void SetSendMessage(ByteBuffer* buf, bool* fail_send_message) { + send_message_ = buf; + fail_send_message_ = fail_send_message; + } void SetSendInitialMetadata( std::multimap* metadata) { @@ -334,6 +343,7 @@ class InterceptorBatchMethodsImpl std::function callback_; ByteBuffer* send_message_ = nullptr; + bool* fail_send_message_ = nullptr; std::multimap* send_initial_metadata_; @@ -451,6 +461,12 @@ class CancelInterceptorBatchMethods "method which has a Cancel notification"); return std::unique_ptr(nullptr); } + + void FailHijackedSendMessage() override { + GPR_CODEGEN_ASSERT(false && + "It is illegal to call FailHijackedSendMessage on a " + "method which has a Cancel notification"); + } }; } // namespace internal } // namespace grpc diff --git a/test/cpp/end2end/client_interceptors_end2end_test.cc b/test/cpp/end2end/client_interceptors_end2end_test.cc index 0b34ec93ae..81efd15452 100644 --- a/test/cpp/end2end/client_interceptors_end2end_test.cc +++ b/test/cpp/end2end/client_interceptors_end2end_test.cc @@ -269,6 +269,49 @@ class HijackingInterceptorMakesAnotherCallFactory } }; +class ClientStreamingRpcHijackingInterceptor + : public experimental::Interceptor { + public: + ClientStreamingRpcHijackingInterceptor(experimental::ClientRpcInfo* info) { + info_ = info; + } + virtual void Intercept(experimental::InterceptorBatchMethods* methods) { + bool hijack = false; + if (methods->QueryInterceptionHookPoint( + experimental::InterceptionHookPoints::PRE_SEND_INITIAL_METADATA)) { + hijack = true; + } + if (methods->QueryInterceptionHookPoint( + experimental::InterceptionHookPoints::PRE_SEND_MESSAGE)) { + if (++count_ > 10) { + methods->FailHijackedSendMessage(); + } + } + if (methods->QueryInterceptionHookPoint( + experimental::InterceptionHookPoints::PRE_RECV_STATUS)) { + auto* status = methods->GetRecvStatus(); + *status = Status(StatusCode::UNAVAILABLE, "Done sending 10 messages"); + } + if (hijack) { + methods->Hijack(); + } else { + methods->Proceed(); + } + } + + private: + experimental::ClientRpcInfo* info_; + int count_ = 0; +}; +class ClientStreamingRpcHijackingInterceptorFactory + : public experimental::ClientInterceptorFactoryInterface { + public: + virtual experimental::Interceptor* CreateClientInterceptor( + experimental::ClientRpcInfo* info) override { + return new ClientStreamingRpcHijackingInterceptor(info); + } +}; + class LoggingInterceptor : public experimental::Interceptor { public: LoggingInterceptor(experimental::ClientRpcInfo* info) { info_ = info; } @@ -535,6 +578,36 @@ TEST_F(ClientInterceptorsStreamingEnd2endTest, ServerStreamingTest) { EXPECT_EQ(DummyInterceptor::GetNumTimesRun(), 20); } +TEST_F(ClientInterceptorsStreamingEnd2endTest, ClientStreamingHijackingTest) { + ChannelArguments args; + auto creators = std::unique_ptr>>( + new std::vector< + std::unique_ptr>()); + creators->push_back( + std::unique_ptr( + new ClientStreamingRpcHijackingInterceptorFactory())); + auto channel = experimental::CreateCustomChannelWithInterceptors( + server_address_, InsecureChannelCredentials(), args, std::move(creators)); + + auto stub = grpc::testing::EchoTestService::NewStub(channel); + ClientContext ctx; + EchoRequest req; + EchoResponse resp; + req.mutable_param()->set_echo_metadata(true); + req.set_message("Hello"); + string expected_resp = ""; + auto writer = stub->RequestStream(&ctx, &resp); + for (int i = 0; i < 10; i++) { + EXPECT_TRUE(writer->Write(req)); + expected_resp += "Hello"; + } + // Expect that the interceptor will reject the 11th message + EXPECT_FALSE(writer->Write(req)); + Status s = writer->Finish(); + EXPECT_EQ(s.ok(), false); +} + TEST_F(ClientInterceptorsStreamingEnd2endTest, BidiStreamingTest) { ChannelArguments args; DummyInterceptor::Reset(); -- cgit v1.2.3 From 0911e489e3fe22e2ca5d7c927dac83358f2f05b7 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Thu, 15 Nov 2018 13:55:56 -0800 Subject: Add a method to check whether the message was received successfully --- include/grpcpp/impl/codegen/call_op_set.h | 6 ++++-- include/grpcpp/impl/codegen/interceptor.h | 3 +++ include/grpcpp/impl/codegen/interceptor_common.h | 10 ++++++++++ test/cpp/end2end/client_interceptors_end2end_test.cc | 12 ++++++++++++ 4 files changed, 29 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/grpcpp/impl/codegen/call_op_set.h b/include/grpcpp/impl/codegen/call_op_set.h index aae8b9d3e3..3699ec94f2 100644 --- a/include/grpcpp/impl/codegen/call_op_set.h +++ b/include/grpcpp/impl/codegen/call_op_set.h @@ -406,12 +406,13 @@ class CallOpRecvMessage { void SetInterceptionHookPoint( InterceptorBatchMethodsImpl* interceptor_methods) { + if (message_ == nullptr) return; interceptor_methods->SetRecvMessage(message_, &got_message); } void SetFinishInterceptionHookPoint( InterceptorBatchMethodsImpl* interceptor_methods) { - if (!got_message) return; + if (message_ == nullptr) return; interceptor_methods->AddInterceptionHookPoint( experimental::InterceptionHookPoints::POST_RECV_MESSAGE); } @@ -501,12 +502,13 @@ class CallOpGenericRecvMessage { void SetInterceptionHookPoint( InterceptorBatchMethodsImpl* interceptor_methods) { + if (!deserialize_) return; interceptor_methods->SetRecvMessage(message_, &got_message); } void SetFinishInterceptionHookPoint( InterceptorBatchMethodsImpl* interceptor_methods) { - if (!got_message) return; + if (!deserialize_) return; interceptor_methods->AddInterceptionHookPoint( experimental::InterceptionHookPoints::POST_RECV_MESSAGE); } diff --git a/include/grpcpp/impl/codegen/interceptor.h b/include/grpcpp/impl/codegen/interceptor.h index 943376a545..b977c35016 100644 --- a/include/grpcpp/impl/codegen/interceptor.h +++ b/include/grpcpp/impl/codegen/interceptor.h @@ -103,6 +103,9 @@ class InterceptorBatchMethods { // is already deserialized virtual void* GetRecvMessage() = 0; + // Checks whether the RECV MESSAGE op completed successfully + virtual bool GetRecvMessageStatus() = 0; + // Returns a modifiable multimap of the received initial metadata virtual std::multimap* GetRecvInitialMetadata() = 0; diff --git a/include/grpcpp/impl/codegen/interceptor_common.h b/include/grpcpp/impl/codegen/interceptor_common.h index d23b71f8a7..b2e92dd6f3 100644 --- a/include/grpcpp/impl/codegen/interceptor_common.h +++ b/include/grpcpp/impl/codegen/interceptor_common.h @@ -103,6 +103,8 @@ class InterceptorBatchMethodsImpl void* GetRecvMessage() override { return recv_message_; } + bool GetRecvMessageStatus() override { return *got_message_; } + std::multimap* GetRecvInitialMetadata() override { return recv_initial_metadata_->map(); @@ -432,6 +434,14 @@ class CancelInterceptorBatchMethods return nullptr; } + bool GetRecvMessageStatus() override { + GPR_CODEGEN_ASSERT( + false && + "It is illegal to call GetRecvMessageStatus on a method which " + "has a Cancel notification"); + return false; + } + std::multimap* GetRecvInitialMetadata() override { GPR_CODEGEN_ASSERT(false && diff --git a/test/cpp/end2end/client_interceptors_end2end_test.cc b/test/cpp/end2end/client_interceptors_end2end_test.cc index 459788ba51..4d4760a652 100644 --- a/test/cpp/end2end/client_interceptors_end2end_test.cc +++ b/test/cpp/end2end/client_interceptors_end2end_test.cc @@ -325,6 +325,12 @@ class ServerStreamingRpcHijackingInterceptor static_cast(methods->GetRecvMessage()); resp->set_message("Hello"); } + if (methods->QueryInterceptionHookPoint( + experimental::InterceptionHookPoints::POST_RECV_MESSAGE)) { + // Only the last message will be a failure + EXPECT_FALSE(got_failed_message_); + got_failed_message_ = !methods->GetRecvMessageStatus(); + } if (methods->QueryInterceptionHookPoint( experimental::InterceptionHookPoints::PRE_RECV_STATUS)) { auto* map = methods->GetRecvTrailingMetadata(); @@ -341,11 +347,16 @@ class ServerStreamingRpcHijackingInterceptor } } + static bool GotFailedMessage() { return got_failed_message_; } + private: experimental::ClientRpcInfo* info_; + static bool got_failed_message_; int count_ = 0; }; +bool ServerStreamingRpcHijackingInterceptor::got_failed_message_ = false; + class ServerStreamingRpcHijackingInterceptorFactory : public experimental::ClientInterceptorFactoryInterface { public: @@ -634,6 +645,7 @@ TEST_F(ClientInterceptorsStreamingEnd2endTest, ServerStreamingHijackingTest) { auto channel = experimental::CreateCustomChannelWithInterceptors( server_address_, InsecureChannelCredentials(), args, std::move(creators)); MakeServerStreamingCall(channel); + EXPECT_TRUE(ServerStreamingRpcHijackingInterceptor::GotFailedMessage()); } TEST_F(ClientInterceptorsStreamingEnd2endTest, BidiStreamingTest) { -- cgit v1.2.3 From d4ebd30eb2eb94f77ac9b52c44880e3d70c6aef0 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Thu, 15 Nov 2018 15:57:43 -0800 Subject: Add method to get status of send message op on POST_SEND_MESSAGE --- include/grpcpp/impl/codegen/call_op_set.h | 18 ++++++++++++++++-- include/grpcpp/impl/codegen/interceptor.h | 6 +++++- include/grpcpp/impl/codegen/interceptor_common.h | 10 ++++++++++ test/cpp/end2end/client_interceptors_end2end_test.cc | 18 ++++++++++++++++-- 4 files changed, 47 insertions(+), 5 deletions(-) (limited to 'include') diff --git a/include/grpcpp/impl/codegen/call_op_set.h b/include/grpcpp/impl/codegen/call_op_set.h index 1f2b88e9e1..f330679ffc 100644 --- a/include/grpcpp/impl/codegen/call_op_set.h +++ b/include/grpcpp/impl/codegen/call_op_set.h @@ -315,9 +315,16 @@ class CallOpSendMessage { write_options_.Clear(); } void FinishOp(bool* status) { - send_buf_.Clear(); + if (!send_buf_.Valid()) { + return; + } if (hijacked_ && failed_send_) { + // Hijacking interceptor failed this Op *status = false; + } else if (!*status) { + // This Op was passed down to core and the Op failed + gpr_log(GPR_ERROR, "failure status"); + failed_send_ = true; } } @@ -330,7 +337,14 @@ class CallOpSendMessage { } void SetFinishInterceptionHookPoint( - InterceptorBatchMethodsImpl* interceptor_methods) {} + InterceptorBatchMethodsImpl* interceptor_methods) { + if (send_buf_.Valid()) { + interceptor_methods->AddInterceptionHookPoint( + experimental::InterceptionHookPoints::POST_SEND_MESSAGE); + // We had already registered failed_send_ earlier. No need to do it again. + } + send_buf_.Clear(); + } void SetHijackingState(InterceptorBatchMethodsImpl* interceptor_methods) { hijacked_ = true; diff --git a/include/grpcpp/impl/codegen/interceptor.h b/include/grpcpp/impl/codegen/interceptor.h index 47239332c8..154172dd81 100644 --- a/include/grpcpp/impl/codegen/interceptor.h +++ b/include/grpcpp/impl/codegen/interceptor.h @@ -41,9 +41,10 @@ class InterceptedMessage { }; enum class InterceptionHookPoints { - /* The first two in this list are for clients and servers */ + /* The first three in this list are for clients and servers */ PRE_SEND_INITIAL_METADATA, PRE_SEND_MESSAGE, + POST_SEND_MESSAGE, PRE_SEND_STATUS /* server only */, PRE_SEND_CLOSE /* client only */, /* The following three are for hijacked clients only and can only be @@ -85,6 +86,9 @@ class InterceptorBatchMethods { // sent virtual ByteBuffer* GetSendMessage() = 0; + // Checks whether the SEND MESSAGE op succeeded + virtual bool GetSendMessageStatus() = 0; + // Returns a modifiable multimap of the initial metadata to be sent virtual std::multimap* GetSendInitialMetadata() = 0; diff --git a/include/grpcpp/impl/codegen/interceptor_common.h b/include/grpcpp/impl/codegen/interceptor_common.h index 601a929afe..21326df73b 100644 --- a/include/grpcpp/impl/codegen/interceptor_common.h +++ b/include/grpcpp/impl/codegen/interceptor_common.h @@ -81,6 +81,8 @@ class InterceptorBatchMethodsImpl ByteBuffer* GetSendMessage() override { return send_message_; } + bool GetSendMessageStatus() override { return !*fail_send_message_; } + std::multimap* GetSendInitialMetadata() override { return send_initial_metadata_; } @@ -113,6 +115,7 @@ class InterceptorBatchMethodsImpl void FailHijackedSendMessage() override { GPR_CODEGEN_ASSERT(hooks_[static_cast( experimental::InterceptionHookPoints::PRE_SEND_MESSAGE)]); + gpr_log(GPR_ERROR, "failing"); *fail_send_message_ = true; } @@ -396,6 +399,13 @@ class CancelInterceptorBatchMethods return nullptr; } + bool GetSendMessageStatus() override { + GPR_CODEGEN_ASSERT( + false && + "It is illegal to call GetSendMessageStatus on a method which " + "has a Cancel notification"); + } + std::multimap* GetSendInitialMetadata() override { GPR_CODEGEN_ASSERT(false && "It is illegal to call GetSendInitialMetadata on a " diff --git a/test/cpp/end2end/client_interceptors_end2end_test.cc b/test/cpp/end2end/client_interceptors_end2end_test.cc index 81efd15452..97947e7393 100644 --- a/test/cpp/end2end/client_interceptors_end2end_test.cc +++ b/test/cpp/end2end/client_interceptors_end2end_test.cc @@ -287,6 +287,13 @@ class ClientStreamingRpcHijackingInterceptor methods->FailHijackedSendMessage(); } } + if (methods->QueryInterceptionHookPoint( + experimental::InterceptionHookPoints::POST_SEND_MESSAGE)) { + EXPECT_FALSE(got_failed_send_); + gpr_log(GPR_ERROR, "%d", got_failed_send_); + got_failed_send_ = !methods->GetSendMessageStatus(); + gpr_log(GPR_ERROR, "%d", got_failed_send_); + } if (methods->QueryInterceptionHookPoint( experimental::InterceptionHookPoints::PRE_RECV_STATUS)) { auto* status = methods->GetRecvStatus(); @@ -299,10 +306,16 @@ class ClientStreamingRpcHijackingInterceptor } } + static bool GotFailedSend() { return got_failed_send_; } + private: experimental::ClientRpcInfo* info_; int count_ = 0; + static bool got_failed_send_; }; + +bool ClientStreamingRpcHijackingInterceptor::got_failed_send_ = false; + class ClientStreamingRpcHijackingInterceptorFactory : public experimental::ClientInterceptorFactoryInterface { public: @@ -602,10 +615,11 @@ TEST_F(ClientInterceptorsStreamingEnd2endTest, ClientStreamingHijackingTest) { EXPECT_TRUE(writer->Write(req)); expected_resp += "Hello"; } - // Expect that the interceptor will reject the 11th message - EXPECT_FALSE(writer->Write(req)); + // The interceptor will reject the 11th message + writer->Write(req); Status s = writer->Finish(); EXPECT_EQ(s.ok(), false); + EXPECT_TRUE(ClientStreamingRpcHijackingInterceptor::GotFailedSend()); } TEST_F(ClientInterceptorsStreamingEnd2endTest, BidiStreamingTest) { -- cgit v1.2.3 From 00c9c40004d011f01c72d253a530edb3364992bf Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Thu, 15 Nov 2018 16:00:33 -0800 Subject: Remove extraneous logging statements --- include/grpcpp/impl/codegen/call_op_set.h | 1 - include/grpcpp/impl/codegen/interceptor_common.h | 1 - test/cpp/end2end/client_interceptors_end2end_test.cc | 2 -- 3 files changed, 4 deletions(-) (limited to 'include') diff --git a/include/grpcpp/impl/codegen/call_op_set.h b/include/grpcpp/impl/codegen/call_op_set.h index f330679ffc..ac3ba17bd9 100644 --- a/include/grpcpp/impl/codegen/call_op_set.h +++ b/include/grpcpp/impl/codegen/call_op_set.h @@ -323,7 +323,6 @@ class CallOpSendMessage { *status = false; } else if (!*status) { // This Op was passed down to core and the Op failed - gpr_log(GPR_ERROR, "failure status"); failed_send_ = true; } } diff --git a/include/grpcpp/impl/codegen/interceptor_common.h b/include/grpcpp/impl/codegen/interceptor_common.h index 21326df73b..321691236b 100644 --- a/include/grpcpp/impl/codegen/interceptor_common.h +++ b/include/grpcpp/impl/codegen/interceptor_common.h @@ -115,7 +115,6 @@ class InterceptorBatchMethodsImpl void FailHijackedSendMessage() override { GPR_CODEGEN_ASSERT(hooks_[static_cast( experimental::InterceptionHookPoints::PRE_SEND_MESSAGE)]); - gpr_log(GPR_ERROR, "failing"); *fail_send_message_ = true; } diff --git a/test/cpp/end2end/client_interceptors_end2end_test.cc b/test/cpp/end2end/client_interceptors_end2end_test.cc index 97947e7393..3708c11235 100644 --- a/test/cpp/end2end/client_interceptors_end2end_test.cc +++ b/test/cpp/end2end/client_interceptors_end2end_test.cc @@ -290,9 +290,7 @@ class ClientStreamingRpcHijackingInterceptor if (methods->QueryInterceptionHookPoint( experimental::InterceptionHookPoints::POST_SEND_MESSAGE)) { EXPECT_FALSE(got_failed_send_); - gpr_log(GPR_ERROR, "%d", got_failed_send_); got_failed_send_ = !methods->GetSendMessageStatus(); - gpr_log(GPR_ERROR, "%d", got_failed_send_); } if (methods->QueryInterceptionHookPoint( experimental::InterceptionHookPoints::PRE_RECV_STATUS)) { -- cgit v1.2.3 From 31a775b425eac37bb43c301cfb25e1f6a4bde106 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Tue, 18 Dec 2018 12:52:14 -0800 Subject: Add missing argument --- include/grpcpp/impl/codegen/call_op_set.h | 3 +-- include/grpcpp/impl/codegen/interceptor_common.h | 1 + test/cpp/end2end/client_interceptors_end2end_test.cc | 8 +++----- 3 files changed, 5 insertions(+), 7 deletions(-) (limited to 'include') diff --git a/include/grpcpp/impl/codegen/call_op_set.h b/include/grpcpp/impl/codegen/call_op_set.h index 3db9f48bff..1c0ccbab52 100644 --- a/include/grpcpp/impl/codegen/call_op_set.h +++ b/include/grpcpp/impl/codegen/call_op_set.h @@ -340,12 +340,11 @@ class CallOpSendMessage { if (send_buf_.Valid()) { interceptor_methods->AddInterceptionHookPoint( experimental::InterceptionHookPoints::POST_SEND_MESSAGE); - // We had already registered failed_send_ earlier. No need to do it again. } send_buf_.Clear(); // The contents of the SendMessage value that was previously set // has had its references stolen by core's operations - interceptor_methods->SetSendMessage(nullptr); + interceptor_methods->SetSendMessage(nullptr, &failed_send_); } void SetHijackingState(InterceptorBatchMethodsImpl* interceptor_methods) { diff --git a/include/grpcpp/impl/codegen/interceptor_common.h b/include/grpcpp/impl/codegen/interceptor_common.h index 321691236b..b01706af8d 100644 --- a/include/grpcpp/impl/codegen/interceptor_common.h +++ b/include/grpcpp/impl/codegen/interceptor_common.h @@ -403,6 +403,7 @@ class CancelInterceptorBatchMethods false && "It is illegal to call GetSendMessageStatus on a method which " "has a Cancel notification"); + return false; } std::multimap* GetSendInitialMetadata() override { diff --git a/test/cpp/end2end/client_interceptors_end2end_test.cc b/test/cpp/end2end/client_interceptors_end2end_test.cc index 33773e3b3b..c55eaab4d6 100644 --- a/test/cpp/end2end/client_interceptors_end2end_test.cc +++ b/test/cpp/end2end/client_interceptors_end2end_test.cc @@ -580,11 +580,9 @@ TEST_F(ClientInterceptorsStreamingEnd2endTest, ServerStreamingTest) { TEST_F(ClientInterceptorsStreamingEnd2endTest, ClientStreamingHijackingTest) { ChannelArguments args; - auto creators = std::unique_ptr>>( - new std::vector< - std::unique_ptr>()); - creators->push_back( + std::vector> + creators; + creators.push_back( std::unique_ptr( new ClientStreamingRpcHijackingInterceptorFactory())); auto channel = experimental::CreateCustomChannelWithInterceptors( -- cgit v1.2.3 From 7eeda22d9ed2f9936ec5a2ad61076274dfb5282b Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Thu, 3 Jan 2019 18:23:15 -0800 Subject: s/two/three --- include/grpcpp/impl/codegen/interceptor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include') diff --git a/include/grpcpp/impl/codegen/interceptor.h b/include/grpcpp/impl/codegen/interceptor.h index 0ad257bcd6..519c65c717 100644 --- a/include/grpcpp/impl/codegen/interceptor.h +++ b/include/grpcpp/impl/codegen/interceptor.h @@ -46,7 +46,7 @@ namespace experimental { /// operation has been requested and it is available. POST_RECV means that a /// result is available but has not yet been passed back to the application. enum class InterceptionHookPoints { - /// The first two in this list are for clients and servers + /// The first three in this list are for clients and servers PRE_SEND_INITIAL_METADATA, PRE_SEND_MESSAGE, POST_SEND_MESSAGE, -- cgit v1.2.3 From 9b9ef640278fd5d0c9a64c1b0c7182277bc35f53 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Fri, 4 Jan 2019 11:29:57 -0800 Subject: Add more information on the usage of FailHijackedRecvMessage --- include/grpcpp/impl/codegen/interceptor.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/include/grpcpp/impl/codegen/interceptor.h b/include/grpcpp/impl/codegen/interceptor.h index a1cb80013d..cf92ce46b4 100644 --- a/include/grpcpp/impl/codegen/interceptor.h +++ b/include/grpcpp/impl/codegen/interceptor.h @@ -157,7 +157,9 @@ class InterceptorBatchMethods { /// list. virtual std::unique_ptr GetInterceptedChannel() = 0; - // On a hijacked RPC, an interceptor can decide to fail a RECV MESSAGE op. + /// On a hijacked RPC, an interceptor can decide to fail a PRE_RECV_MESSAGE + /// op. This would be a signal to the reader that there will be no more + /// messages, or the stream has failed or been cancelled. virtual void FailHijackedRecvMessage() = 0; }; -- cgit v1.2.3