From 3021a5fecdad89c692dc80e89e8fa6e3927f1376 Mon Sep 17 00:00:00 2001 From: "rtenneti@chromium.org" Date: Wed, 23 Jul 2014 01:40:40 +0000 Subject: QUIC - undo the chromium specific changes from the following CL which did "Moving work from the QuicSession constructor to InitializeSession()". https://codereview.chromium.org/393953009/ Merge internal change: 70661792 This CL fixes the crash bug in net::QuicSession::IsCryptoHandshakeConfirmed. We are accessing crypto_stream_ before it was created. This could be triggered if the computer is slow or goes to sleep and crypto handshake not completed timer goes off. BUG=395790 R=rch@chromium.org Review URL: https://codereview.chromium.org/407193002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@284827 0039d316-1c4b-4281-b951-d872f2087c98 --- net/quic/quic_client_session.cc | 33 +++++++++++++++------------------ net/quic/quic_client_session.h | 10 ++++------ net/quic/quic_client_session_test.cc | 11 ++++++----- net/quic/quic_http_stream_test.cc | 10 +++++----- net/quic/quic_stream_factory.cc | 9 +++++---- 5 files changed, 35 insertions(+), 38 deletions(-) (limited to 'net/quic') diff --git a/net/quic/quic_client_session.cc b/net/quic/quic_client_session.cc index fa6a211..067009e 100644 --- a/net/quic/quic_client_session.cc +++ b/net/quic/quic_client_session.cc @@ -132,15 +132,20 @@ void QuicClientSession::StreamRequest::OnRequestCompleteFailure(int rv) { ResetAndReturn(&callback_).Run(rv); } -QuicClientSession::QuicClientSession(QuicConnection* connection, - scoped_ptr socket, - scoped_ptr writer, - QuicStreamFactory* stream_factory, - scoped_ptr server_info, - const QuicConfig& config, - base::TaskRunner* task_runner, - NetLog* net_log) +QuicClientSession::QuicClientSession( + QuicConnection* connection, + scoped_ptr socket, + scoped_ptr writer, + QuicStreamFactory* stream_factory, + QuicCryptoClientStreamFactory* crypto_client_stream_factory, + scoped_ptr server_info, + const QuicServerId& server_id, + const QuicConfig& config, + QuicCryptoClientConfig* crypto_config, + base::TaskRunner* task_runner, + NetLog* net_log) : QuicClientSessionBase(connection, config), + server_host_port_(server_id.host_port_pair()), require_confirmation_(false), stream_factory_(stream_factory), socket_(socket.Pass()), @@ -155,15 +160,6 @@ QuicClientSession::QuicClientSession(QuicConnection* connection, num_packets_read_(0), going_away_(false), weak_factory_(this) { - connection->set_debug_visitor(&logger_); -} - -void QuicClientSession::InitializeSession( - const QuicServerId& server_id, - QuicCryptoClientConfig* crypto_config, - QuicCryptoClientStreamFactory* crypto_client_stream_factory) { - QuicClientSessionBase::InitializeSession(); - server_host_port_.reset(new HostPortPair(server_id.host_port_pair())); crypto_stream_.reset( crypto_client_stream_factory ? crypto_client_stream_factory->CreateQuicCryptoClientStream( @@ -172,6 +168,7 @@ void QuicClientSession::InitializeSession( new ProofVerifyContextChromium(net_log_), crypto_config)); + connection->set_debug_visitor(&logger_); // TODO(rch): pass in full host port proxy pair net_log_.BeginEvent( NetLog::TYPE_QUIC_SESSION, @@ -503,7 +500,7 @@ bool QuicClientSession::CanPool(const std::string& hostname) const { if (ssl_info.channel_id_sent && ServerBoundCertService::GetDomainForHost(hostname) != - ServerBoundCertService::GetDomainForHost(server_host_port_->host())) { + ServerBoundCertService::GetDomainForHost(server_host_port_.host())) { return false; } diff --git a/net/quic/quic_client_session.h b/net/quic/quic_client_session.h index eecebc6..b46c0ab 100644 --- a/net/quic/quic_client_session.h +++ b/net/quic/quic_client_session.h @@ -94,17 +94,15 @@ class NET_EXPORT_PRIVATE QuicClientSession : public QuicClientSessionBase { scoped_ptr socket, scoped_ptr writer, QuicStreamFactory* stream_factory, + QuicCryptoClientStreamFactory* crypto_client_stream_factory, scoped_ptr server_info, + const QuicServerId& server_id, const QuicConfig& config, + QuicCryptoClientConfig* crypto_config, base::TaskRunner* task_runner, NetLog* net_log); virtual ~QuicClientSession(); - void InitializeSession( - const QuicServerId& server_id, - QuicCryptoClientConfig* config, - QuicCryptoClientStreamFactory* crypto_client_stream_factory); - void AddObserver(Observer* observer); void RemoveObserver(Observer* observer); @@ -221,7 +219,7 @@ class NET_EXPORT_PRIVATE QuicClientSession : public QuicClientSessionBase { void OnConnectTimeout(); - scoped_ptr server_host_port_; + const HostPortPair server_host_port_; bool require_confirmation_; scoped_ptr crypto_stream_; QuicStreamFactory* stream_factory_; diff --git a/net/quic/quic_client_session_test.cc b/net/quic/quic_client_session_test.cc index 6b4e887..986801d 100644 --- a/net/quic/quic_client_session_test.cc +++ b/net/quic/quic_client_session_test.cc @@ -72,13 +72,14 @@ class QuicClientSessionTest : public ::testing::TestWithParam { : writer_(new TestPacketWriter(GetParam())), connection_( new PacketSavingConnection(false, SupportedVersions(GetParam()))), - session_(connection_, GetSocket().Pass(), writer_.Pass(), NULL, - make_scoped_ptr((QuicServerInfo*)NULL), DefaultQuicConfig(), + session_(connection_, GetSocket().Pass(), writer_.Pass(), NULL, NULL, + make_scoped_ptr((QuicServerInfo*)NULL), + QuicServerId(kServerHostname, kServerPort, false, + PRIVACY_MODE_DISABLED), + DefaultQuicConfig(), &crypto_config_, base::MessageLoop::current()->message_loop_proxy().get(), &net_log_) { - session_.InitializeSession(QuicServerId(kServerHostname, kServerPort, false, - PRIVACY_MODE_DISABLED), - &crypto_config_, NULL); + session_.InitializeSession(); session_.config()->SetDefaults(); crypto_config_.SetDefaults(); } diff --git a/net/quic/quic_http_stream_test.cc b/net/quic/quic_http_stream_test.cc index a4e3c98..2b85417 100644 --- a/net/quic/quic_http_stream_test.cc +++ b/net/quic/quic_http_stream_test.cc @@ -208,15 +208,15 @@ class QuicHttpStreamTest : public ::testing::TestWithParam { new QuicClientSession(connection_, scoped_ptr(socket), writer_.Pass(), NULL, + &crypto_client_stream_factory_, make_scoped_ptr((QuicServerInfo*)NULL), - DefaultQuicConfig(), + QuicServerId(kServerHostname, kServerPort, + false, PRIVACY_MODE_DISABLED), + DefaultQuicConfig(), &crypto_config_, base::MessageLoop::current()-> message_loop_proxy().get(), NULL)); - session_->InitializeSession(QuicServerId(kServerHostname, kServerPort, - false, PRIVACY_MODE_DISABLED), - &crypto_config_, - &crypto_client_stream_factory_); + session_->InitializeSession(); session_->GetCryptoStream()->CryptoConnect(); EXPECT_TRUE(session_->IsCryptoHandshakeConfirmed()); stream_.reset(use_closing_stream_ ? diff --git a/net/quic/quic_stream_factory.cc b/net/quic/quic_stream_factory.cc index f9f6802..337493c 100644 --- a/net/quic/quic_stream_factory.cc +++ b/net/quic/quic_stream_factory.cc @@ -858,11 +858,12 @@ int QuicStreamFactory::CreateSession( } *session = new QuicClientSession( - connection, socket.Pass(), writer.Pass(), this, server_info.Pass(), - config, base::MessageLoop::current()->message_loop_proxy().get(), + connection, socket.Pass(), writer.Pass(), this, + quic_crypto_client_stream_factory_, server_info.Pass(), server_id, + config, &crypto_config_, + base::MessageLoop::current()->message_loop_proxy().get(), net_log.net_log()); - (*session)->InitializeSession(server_id, &crypto_config_, - quic_crypto_client_stream_factory_); + (*session)->InitializeSession(); all_sessions_[*session] = server_id; // owning pointer return OK; } -- cgit v1.1