Handle case where crypto stream is not immediately available.
gfe-relnote: Not flag protected, fixes implementation bug in CL/275355012
First, CL/275355012 subtly changed how QuicSession reports whether the
handshake has been confirmed or not.
Prior to the CL, there was a bool in QuicSession,
is_handshake_confirmed_, that was initialized to false. When the
handshake completed/etc, the bool was flipped to true. The failure
point that I ran across[1] used to test for handshake confirmation by
calling QuicSession::IsHandshakeConfirmed(), which merely returned the
value of the bool.
CL/275355012 changed that. First, it got rid of the bool &
QuicSession::IsHandshakeConfirmed(). Second, it changed
QuicSpdySession::SetMaxAllowedPushId. This method used to call
QuicSession::IsHandshakeConfirmed() to see if the handshake was done.
The CL changed it to call QuicSession::IsCryptoHandshakeConfirmed().
QuicSession::IsCryptoHandshakeConfirmed() was implemented as
{
return GetCryptoStream()->handshake_confirmed()
}
So, if GetCryptoStream() returned NULL (as it would if called prior to
the crypto stream being set up) then an addressing exception would occur.
Second
This happens because the failing Chromium tests all:
1) Create a QuicChromiumClientSession
2) Call <mumble>Session::Initialize()
<mumble>Session::Initialize() calls the internal method to create the
crypto stream.
But the QuicChromiumClientSession constructor calls
QuicSpdySession::SetMaxAllowedPushId(), which calls
QuicSession::IsCryptoHandshakeConfirmed() So the more detailed call
pattern is:
This happens because the failing Chromium tests all:
1) Create a QuicChromiumClientSession
Call QuicSpdySession::SetMaxAllowedPushId()
Call QuicSession::IsCryptoHandshakeConfirmed()
Call GetCryptoStream()
Dereference a null pointer
2) Call <mumble>Session::Initialize()
Set up the Crypto Stream.
Set CryptoStream pointer to be a valid ptr.
This _used_ to work because the call pattern was
1) Create a QuicChromiumClientSession
Call QuicSpdySession::SetMaxAllowedPushId()
Call QuicSession::IsHandshakeConfirmed()
return the boolean
2) Call <mumble>Session::Initialize()
Set up the Crypto Stream.
set the boolean true.
The fix is to first test the CryptoStream pointer. If it's null, it
means that the crypto stream was not set up yet, so the handshake can
not have occured, much less "be confirmed" and IsCryptoHandshakeConfirmed
returns fals. QED.
This is functionally identical to the state of things prior to
CL/275355012 because if the stream has not been created, the boolean
would still be false, and IsHandshakeConfirmed would have returned
false.
PiperOrigin-RevId: 279322611
Change-Id: I7edd34d0597809dc3086ae8eb940ef87f46d4d23
1 file changed
