From 3df76b1df1ae5657eb82bab63fde0f0f3513fd4e Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 29 Sep 2017 12:27:18 -0400 Subject: [PATCH] Use unique_ptr in SSLCertChain. In particular, this allows us to have a SSLCertChain(std::vector>) constructor that avoids making a copy. Bug: webrtc:8289 Change-Id: If353ad886a73aecff98b2939db15c40ada6b4474 Reviewed-on: https://webrtc-review.googlesource.com/4361 Commit-Queue: David Benjamin Reviewed-by: Taylor Brandstetter Cr-Commit-Position: refs/heads/master@{#20093} --- rtc_base/fakesslidentity.h | 11 +++++------ rtc_base/sslidentity.cc | 29 +++++++++++++++++++++++++---- rtc_base/sslidentity.h | 22 ++++++++-------------- 3 files changed, 38 insertions(+), 24 deletions(-) diff --git a/rtc_base/fakesslidentity.h b/rtc_base/fakesslidentity.h index db1762a0e5..30507290e0 100644 --- a/rtc_base/fakesslidentity.h +++ b/rtc_base/fakesslidentity.h @@ -17,6 +17,7 @@ #include "rtc_base/checks.h" #include "rtc_base/messagedigest.h" +#include "rtc_base/ptr_util.h" #include "rtc_base/sslidentity.h" namespace rtc { @@ -72,16 +73,14 @@ class FakeSSLCertificate : public rtc::SSLCertificate { std::unique_ptr GetChain() const override { if (certs_.empty()) return nullptr; - std::vector new_certs(certs_.size()); + std::vector> new_certs(certs_.size()); std::transform(certs_.begin(), certs_.end(), new_certs.begin(), DupCert); - std::unique_ptr chain(new SSLCertChain(new_certs)); - std::for_each(new_certs.begin(), new_certs.end(), DeleteCert); - return chain; + return MakeUnique(std::move(new_certs)); } private: - static FakeSSLCertificate* DupCert(FakeSSLCertificate cert) { - return cert.GetReference(); + static std::unique_ptr DupCert(FakeSSLCertificate cert) { + return cert.GetUniqueReference(); } static void DeleteCert(SSLCertificate* cert) { delete cert; } std::string data_; diff --git a/rtc_base/sslidentity.cc b/rtc_base/sslidentity.cc index 6fd7ad5bbd..79e2ad56b7 100644 --- a/rtc_base/sslidentity.cc +++ b/rtc_base/sslidentity.cc @@ -13,11 +13,13 @@ #include #include +#include #include "rtc_base/base64.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "rtc_base/opensslidentity.h" +#include "rtc_base/ptr_util.h" #include "rtc_base/sslfingerprint.h" namespace rtc { @@ -59,6 +61,10 @@ std::unique_ptr SSLCertificate::GetStats() const { return GetStats(std::move(issuer)); } +std::unique_ptr SSLCertificate::GetUniqueReference() const { + return WrapUnique(GetReference()); +} + std::unique_ptr SSLCertificate::GetStats( std::unique_ptr issuer) const { // TODO(bemasc): Move this computation to a helper class that caches these @@ -193,18 +199,33 @@ std::string SSLIdentity::DerToPem(const std::string& pem_type, return result.str(); } +SSLCertChain::SSLCertChain(std::vector> certs) + : certs_(std::move(certs)) {} + SSLCertChain::SSLCertChain(const std::vector& certs) { RTC_DCHECK(!certs.empty()); certs_.resize(certs.size()); - std::transform(certs.begin(), certs.end(), certs_.begin(), DupCert); + std::transform( + certs.begin(), certs.end(), certs_.begin(), + [](const SSLCertificate* cert) -> std::unique_ptr { + return cert->GetUniqueReference(); + }); } SSLCertChain::SSLCertChain(const SSLCertificate* cert) { - certs_.push_back(cert->GetReference()); + certs_.push_back(cert->GetUniqueReference()); } -SSLCertChain::~SSLCertChain() { - std::for_each(certs_.begin(), certs_.end(), DeleteCert); +SSLCertChain::~SSLCertChain() {} + +SSLCertChain* SSLCertChain::Copy() const { + std::vector> new_certs(certs_.size()); + std::transform(certs_.begin(), certs_.end(), new_certs.begin(), + [](const std::unique_ptr& cert) + -> std::unique_ptr { + return cert->GetUniqueReference(); + }); + return new SSLCertChain(std::move(new_certs)); } // static diff --git a/rtc_base/sslidentity.h b/rtc_base/sslidentity.h index d81d01302e..f20c5470ef 100644 --- a/rtc_base/sslidentity.h +++ b/rtc_base/sslidentity.h @@ -60,10 +60,13 @@ class SSLCertificate { virtual ~SSLCertificate() {} // Returns a new SSLCertificate object instance wrapping the same - // underlying certificate, including its chain if present. - // Caller is responsible for freeing the returned object. + // underlying certificate, including its chain if present. Caller is + // responsible for freeing the returned object. Use GetUniqueReference + // instead. virtual SSLCertificate* GetReference() const = 0; + std::unique_ptr GetUniqueReference() const; + // Provides the cert chain, or null. The chain includes a copy of each // certificate, excluding the leaf. virtual std::unique_ptr GetChain() const = 0; @@ -103,6 +106,7 @@ class SSLCertificate { // SSLCertificate pointers. class SSLCertChain { public: + explicit SSLCertChain(std::vector> certs); // These constructors copy the provided SSLCertificate(s), so the caller // retains ownership. explicit SSLCertChain(const std::vector& certs); @@ -117,20 +121,10 @@ class SSLCertChain { // Returns a new SSLCertChain object instance wrapping the same underlying // certificate chain. Caller is responsible for freeing the returned object. - SSLCertChain* Copy() const { - return new SSLCertChain(certs_); - } + SSLCertChain* Copy() const; private: - // Helper function for duplicating a vector of certificates. - static SSLCertificate* DupCert(const SSLCertificate* cert) { - return cert->GetReference(); - } - - // Helper function for deleting a vector of certificates. - static void DeleteCert(SSLCertificate* cert) { delete cert; } - - std::vector certs_; + std::vector> certs_; RTC_DISALLOW_COPY_AND_ASSIGN(SSLCertChain); };