Add a callback that notifies the QuicTransportClientSession visitor when the session is ready.
Also add a bit of extra defense in depth for ALPN checks.
gfe-relnote: n/a (not used in production)
PiperOrigin-RevId: 284658419
Change-Id: Ic665263a024821e529c564e7c8b86d362f9991a8
diff --git a/quic/quic_transport/quic_transport_client_session.cc b/quic/quic_transport/quic_transport_client_session.cc
index e9487ec..f316813 100644
--- a/quic/quic_transport/quic_transport_client_session.cc
+++ b/quic/quic_transport/quic_transport_client_session.cc
@@ -67,6 +67,19 @@
proof_handler);
}
+void QuicTransportClientSession::OnAlpnSelected(QuicStringPiece alpn) {
+ // Defense in-depth: ensure the ALPN selected is the desired one.
+ if (alpn != QuicTransportAlpn()) {
+ QUIC_BUG << "QuicTransport negotiated non-QuicTransport ALPN: " << alpn;
+ connection()->CloseConnection(
+ QUIC_INTERNAL_ERROR, "QuicTransport negotiated non-QuicTransport ALPN",
+ ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ return;
+ }
+
+ alpn_received_ = true;
+}
+
QuicStream* QuicTransportClientSession::CreateIncomingStream(QuicStreamId id) {
QUIC_DVLOG(1) << "Creating incoming QuicTransport stream " << id;
QuicTransportStream* stream = CreateStream(id);
@@ -222,12 +235,24 @@
/*fin=*/true, nullptr);
client_indication_sent_ = true;
+ // Defense in depth: never set the ready bit unless ALPN has been confirmed.
+ if (!alpn_received_) {
+ QUIC_BUG << "ALPN confirmation missing after handshake complete";
+ connection()->CloseConnection(
+ QUIC_INTERNAL_ERROR,
+ "ALPN confirmation missing after handshake complete",
+ ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ return;
+ }
+
// Don't set the ready bit if we closed the connection due to any error
// beforehand.
if (!connection()->connected()) {
return;
}
+
ready_ = true;
+ visitor_->OnSessionReady();
}
} // namespace quic
diff --git a/quic/quic_transport/quic_transport_client_session.h b/quic/quic_transport/quic_transport_client_session.h
index b3d8f17..c09b076 100644
--- a/quic/quic_transport/quic_transport_client_session.h
+++ b/quic/quic_transport/quic_transport_client_session.h
@@ -37,6 +37,10 @@
public:
virtual ~ClientVisitor() {}
+ // Notifies the visitor when the client indication has been sent and the
+ // connection is ready to exchange application data.
+ virtual void OnSessionReady() = 0;
+
// Notifies the visitor when a new stream has been received. The stream in
// question can be retrieved using AcceptIncomingBidirectionalStream() or
// AcceptIncomingUnidirectionalStream().
@@ -56,6 +60,8 @@
std::vector<std::string> GetAlpnsToOffer() const override {
return std::vector<std::string>({QuicTransportAlpn()});
}
+ void OnAlpnSelected(QuicStringPiece alpn) override;
+ bool alpn_received() const { return alpn_received_; }
void CryptoConnect() { crypto_stream_->CryptoConnect(); }
@@ -121,6 +127,7 @@
url::Origin origin_;
ClientVisitor* visitor_; // not owned
bool client_indication_sent_ = false;
+ bool alpn_received_ = false;
bool ready_ = false;
// Contains all of the streams that has been received by the session but have
diff --git a/quic/quic_transport/quic_transport_client_session_test.cc b/quic/quic_transport/quic_transport_client_session_test.cc
index 51a0eb8..1225352 100644
--- a/quic/quic_transport/quic_transport_client_session_test.cc
+++ b/quic/quic_transport/quic_transport_client_session_test.cc
@@ -103,6 +103,7 @@
"\0\x01" // length
"/"; // value
+ EXPECT_CALL(visitor_, OnSessionReady());
Connect();
EXPECT_TRUE(session_->IsSessionReady());
diff --git a/quic/quic_transport/quic_transport_integration_test.cc b/quic/quic_transport/quic_transport_integration_test.cc
index 1695a19..f5c804d 100644
--- a/quic/quic_transport/quic_transport_integration_test.cc
+++ b/quic/quic_transport/quic_transport_integration_test.cc
@@ -205,6 +205,7 @@
TEST_F(QuicTransportIntegrationTest, SuccessfulHandshake) {
CreateDefaultEndpoints("/discard");
WireUpEndpoints();
+ EXPECT_CALL(*client_->visitor(), OnSessionReady());
RunHandshake();
EXPECT_TRUE(client_->session()->IsSessionReady());
EXPECT_TRUE(server_->session()->IsSessionReady());
diff --git a/quic/test_tools/quic_transport_test_tools.h b/quic/test_tools/quic_transport_test_tools.h
index 9632d4c..9c5cef4 100644
--- a/quic/test_tools/quic_transport_test_tools.h
+++ b/quic/test_tools/quic_transport_test_tools.h
@@ -14,6 +14,7 @@
class MockClientVisitor : public QuicTransportClientSession::ClientVisitor {
public:
+ MOCK_METHOD0(OnSessionReady, void());
MOCK_METHOD0(OnIncomingBidirectionalStreamAvailable, void());
MOCK_METHOD0(OnIncomingUnidirectionalStreamAvailable, void());
};
