Merge 4979b64 into 89ead4d

Author: jrhodnik

CefSharp.Core.Runtime/Internals/CefCertificateCallbackWrapper.h

@@ -19,18 +19,29 @@ namespace CefSharp
         {
         private:
             MCefRefPtr<CefSelectClientCertificateCallback> _callback;
-            const CefRequestHandler::X509CertificateList& _certificateList;
+            // Owned copy of the certificates Chromium offered, not a reference to the caller's list.
+            // That list belongs to CEF for the duration of ClientAdapter::OnSelectClientCertificate,
+            // and this wrapper deliberately outlives that call - CEF allows Select to be called
+            // "either in this method or at a later time" - so a reference would dangle the moment
+            // the handler returns and a deferred Select would read freed memory.
+            // A ref class cannot hold a std::vector by value, hence the pointer. Copying the vector
+            // copies the reference-counted CefX509Certificate pointers, and those references are
+            // what keep the certificates themselves alive.
+            CefRequestHandler::X509CertificateList* _certificateList;
 
         public:
             CefCertificateCallbackWrapper(CefRefPtr<CefSelectClientCertificateCallback>& callback, const CefRequestHandler::X509CertificateList& certificates)
-                : _callback(callback), _certificateList(certificates)
+                : _callback(callback), _certificateList(new CefRequestHandler::X509CertificateList(certificates))
             {
 
             }
 
             !CefCertificateCallbackWrapper()
             {
                 _callback = nullptr;
+
+                delete _certificateList;
+                _certificateList = nullptr;
             }
 
             ~CefCertificateCallbackWrapper()
@@ -53,8 +64,8 @@ namespace CefSharp
                     auto certThumbprint = cert->Thumbprint;
 
                     std::vector<CefRefPtr<CefX509Certificate>>::const_iterator it =
-                        _certificateList.begin();
-                    for (; it != _certificateList.end(); ++it)
+                        _certificateList->begin();
+                    for (; it != _certificateList->end(); ++it)
                     {
                         auto bytes((*it)->GetDEREncoded());
                         auto byteSize = bytes->GetSize();

CefSharp.Core.Runtime/Internals/ClientAdapter.cpp

@@ -764,8 +764,6 @@ namespace CefSharp
             auto browserWrapper = GetBrowserWrapper(browser->GetIdentifier(), browser->IsPopup());
 
             auto list = gcnew X509Certificate2Collection();
-            // Create a copy of the vector in an attempt to fix #2948
-            CefRequestHandler::X509CertificateList certs;
 
             std::vector<CefRefPtr<CefX509Certificate> >::const_iterator it =
                 certificates.begin();
@@ -780,11 +778,11 @@ namespace CefSharp
                 bytes->GetData(static_cast<void*>(src), byteSize, 0);
                 auto cert = gcnew X509Certificate2(bufferByte);
                 list->Add(cert);
-
-                certs.push_back(*it);
             }
 
-            auto callbackWrapper = gcnew CefCertificateCallbackWrapper(callback, certs);
+            // Passed straight through. The wrapper takes its own reference to each certificate, so
+            // there is no need to copy the vector here.
+            auto callbackWrapper = gcnew CefCertificateCallbackWrapper(callback, certificates);
 
             return handler->OnSelectClientCertificate(
                 _browserControl, browserWrapper, isProxy,