)]}'
{
  "commit": "ba94bc3a3c663af192e9d42f343cc2a7e52ef551",
  "tree": "2c7bdc4a215c447ef60b1cdb6c430b88fd52ba24",
  "parents": [
    "02703964432d6730ee5d51db48e9e2e9708d9f21"
  ],
  "author": {
    "name": "fkastenholz",
    "email": "fkastenholz@google.com",
    "time": "Fri Nov 08 08:50:25 2019 -0800"
  },
  "committer": {
    "name": "Copybara-Service",
    "email": "copybara-worker@google.com",
    "time": "Fri Nov 08 08:51:05 2019 -0800"
  },
  "message": "Handle case where crypto stream is not immediately available.\n\ngfe-relnote: Not flag protected, fixes implementation bug in CL/275355012\n\nFirst, CL/275355012 subtly changed how QuicSession reports whether the\nhandshake has been confirmed or not.\n\nPrior to the CL, there was a bool in QuicSession,\nis_handshake_confirmed_, that was initialized to false. When the\nhandshake completed/etc, the bool was flipped to true.  The failure\npoint that I ran across[1] used to test for handshake confirmation by\ncalling QuicSession::IsHandshakeConfirmed(), which merely returned the\nvalue of the bool.\n\nCL/275355012 changed that. First, it got rid of the bool \u0026\nQuicSession::IsHandshakeConfirmed(). Second, it changed\nQuicSpdySession::SetMaxAllowedPushId. This method used to call\nQuicSession::IsHandshakeConfirmed() to see if the handshake was done.\nThe CL changed it to call QuicSession::IsCryptoHandshakeConfirmed().\n\nQuicSession::IsCryptoHandshakeConfirmed() was implemented as\n   {\n      return GetCryptoStream()-\u003ehandshake_confirmed()\n   }\nSo, if GetCryptoStream() returned NULL (as it would if called prior to\nthe crypto stream being set  up) then an addressing exception would occur.\n\nSecond\n\nThis happens because the failing Chromium tests all:\n 1) Create a QuicChromiumClientSession\n 2) Call \u003cmumble\u003eSession::Initialize()\n\n\u003cmumble\u003eSession::Initialize() calls the internal method to create the\ncrypto stream.\n\nBut the QuicChromiumClientSession constructor calls\nQuicSpdySession::SetMaxAllowedPushId(), which calls\nQuicSession::IsCryptoHandshakeConfirmed() So the more detailed call\npattern is:\n\nThis happens because the failing Chromium tests all:\n 1) Create a QuicChromiumClientSession\n      Call QuicSpdySession::SetMaxAllowedPushId()\n        Call QuicSession::IsCryptoHandshakeConfirmed()\n          Call GetCryptoStream()\n\t    Dereference a null pointer\n 2) Call \u003cmumble\u003eSession::Initialize()\n      Set up the Crypto Stream.\n        Set CryptoStream pointer to be a valid ptr.\n\nThis _used_ to work because the call pattern was\n 1) Create a QuicChromiumClientSession\n      Call QuicSpdySession::SetMaxAllowedPushId()\n        Call QuicSession::IsHandshakeConfirmed()\n          return the boolean\n 2) Call \u003cmumble\u003eSession::Initialize()\n      Set up the Crypto Stream.\n          set the boolean true.\n\nThe fix is to first test the CryptoStream pointer. If it\u0027s null, it\nmeans that the crypto stream was not set up yet, so the handshake can\nnot have occured, much less \"be confirmed\" and IsCryptoHandshakeConfirmed\nreturns fals. QED.\n\nThis is functionally identical to the state of things prior to\nCL/275355012 because if the stream has not been created, the boolean\nwould still be false, and IsHandshakeConfirmed would have returned\nfalse.\n\nPiperOrigin-RevId: 279322611\nChange-Id: I7edd34d0597809dc3086ae8eb940ef87f46d4d23\n",
  "tree_diff": [
    {
      "type": "modify",
      "old_id": "779476eac683e7ccecfc0d5703e15cbe44182a1b",
      "old_mode": 33188,
      "old_path": "quic/core/quic_session.cc",
      "new_id": "205f7ad657ec4b99b2e2e19346f88dbf63716997",
      "new_mode": 33188,
      "new_path": "quic/core/quic_session.cc"
    }
  ]
}