From f5d47860805e37a1e99a478ab7cd33a77ba9007b Mon Sep 17 00:00:00 2001 From: kwiberg Date: Tue, 15 Mar 2016 12:53:24 -0700 Subject: [PATCH] SSLCertificate::GetChain: Return scoped_ptr Instead of using a raw pointer output parameter. This is a good idea in general, but will also be very convenient when scoped_ptr is gone, since unique_ptr doesn't have an .accept() method. BUG=webrtc:5520 Review URL: https://codereview.webrtc.org/1799233002 Cr-Commit-Position: refs/heads/master@{#12004} --- webrtc/api/statscollector.cc | 4 ++-- webrtc/base/fakesslidentity.h | 8 ++++---- webrtc/base/opensslidentity.cc | 4 ++-- webrtc/base/opensslidentity.h | 2 +- webrtc/base/sslidentity.h | 7 ++++--- webrtc/base/sslstreamadapter_unittest.cc | 6 ++---- 6 files changed, 15 insertions(+), 16 deletions(-) diff --git a/webrtc/api/statscollector.cc b/webrtc/api/statscollector.cc index 7fb0c2b7b3..d77953b3be 100644 --- a/webrtc/api/statscollector.cc +++ b/webrtc/api/statscollector.cc @@ -575,8 +575,8 @@ StatsReport* StatsCollector::AddCertificateReports( RTC_DCHECK(cert != NULL); StatsReport* issuer = nullptr; - rtc::scoped_ptr chain; - if (cert->GetChain(chain.accept())) { + rtc::scoped_ptr chain = cert->GetChain(); + if (chain) { // This loop runs in reverse, i.e. from root to leaf, so that each // certificate's issuer's report ID is known before the child certificate's // report is generated. The root certificate does not have an issuer ID diff --git a/webrtc/base/fakesslidentity.h b/webrtc/base/fakesslidentity.h index ec603a541d..47ff86d03a 100644 --- a/webrtc/base/fakesslidentity.h +++ b/webrtc/base/fakesslidentity.h @@ -68,14 +68,14 @@ class FakeSSLCertificate : public rtc::SSLCertificate { digest, size); return (*length != 0); } - virtual bool GetChain(SSLCertChain** chain) const { + virtual rtc::scoped_ptr GetChain() const { if (certs_.empty()) - return false; + return nullptr; std::vector new_certs(certs_.size()); std::transform(certs_.begin(), certs_.end(), new_certs.begin(), DupCert); - *chain = new SSLCertChain(new_certs); + rtc::scoped_ptr chain(new SSLCertChain(new_certs)); std::for_each(new_certs.begin(), new_certs.end(), DeleteCert); - return true; + return chain; } private: diff --git a/webrtc/base/opensslidentity.cc b/webrtc/base/opensslidentity.cc index 0260387925..3c421db07c 100644 --- a/webrtc/base/opensslidentity.cc +++ b/webrtc/base/opensslidentity.cc @@ -280,11 +280,11 @@ bool OpenSSLCertificate::GetSignatureDigestAlgorithm( return true; } -bool OpenSSLCertificate::GetChain(SSLCertChain** chain) const { +rtc::scoped_ptr OpenSSLCertificate::GetChain() const { // Chains are not yet supported when using OpenSSL. // OpenSSLStreamAdapter::SSLVerifyCallback currently requires the remote // certificate to be self-signed. - return false; + return nullptr; } bool OpenSSLCertificate::ComputeDigest(const std::string& algorithm, diff --git a/webrtc/base/opensslidentity.h b/webrtc/base/opensslidentity.h index dda50be775..8b30e6092a 100644 --- a/webrtc/base/opensslidentity.h +++ b/webrtc/base/opensslidentity.h @@ -85,7 +85,7 @@ class OpenSSLCertificate : public SSLCertificate { size_t* length); bool GetSignatureDigestAlgorithm(std::string* algorithm) const override; - bool GetChain(SSLCertChain** chain) const override; + rtc::scoped_ptr GetChain() const override; int64_t CertificateExpirationTime() const override; diff --git a/webrtc/base/sslidentity.h b/webrtc/base/sslidentity.h index 8793e4b2de..be0f3aa107 100644 --- a/webrtc/base/sslidentity.h +++ b/webrtc/base/sslidentity.h @@ -19,6 +19,7 @@ #include "webrtc/base/buffer.h" #include "webrtc/base/messagedigest.h" +#include "webrtc/base/scoped_ptr.h" #include "webrtc/base/timeutils.h" namespace rtc { @@ -50,9 +51,9 @@ class SSLCertificate { // Caller is responsible for freeing the returned object. virtual SSLCertificate* GetReference() const = 0; - // Provides the cert chain, or returns false. The caller owns the chain. - // The chain includes a copy of each certificate, excluding the leaf. - virtual bool GetChain(SSLCertChain** chain) const = 0; + // Provides the cert chain, or null. The chain includes a copy of each + // certificate, excluding the leaf. + virtual rtc::scoped_ptr GetChain() const = 0; // Returns a PEM encoded string representation of the certificate. virtual std::string ToPEMString() const = 0; diff --git a/webrtc/base/sslstreamadapter_unittest.cc b/webrtc/base/sslstreamadapter_unittest.cc index 2eaa475391..1e18b8305c 100644 --- a/webrtc/base/sslstreamadapter_unittest.cc +++ b/webrtc/base/sslstreamadapter_unittest.cc @@ -1056,8 +1056,7 @@ TEST_F(SSLStreamAdapterTestDTLSFromPEMStrings, TestDTLSGetPeerCertificate) { ASSERT_NE(kCERT_PEM, client_peer_string); // It must not have a chain, because the test certs are self-signed. - rtc::SSLCertChain* client_peer_chain; - ASSERT_FALSE(client_peer_cert->GetChain(&client_peer_chain)); + ASSERT_FALSE(client_peer_cert->GetChain()); // The server should have a peer certificate after the handshake. ASSERT_TRUE(GetPeerCertificate(false, server_peer_cert.accept())); @@ -1067,8 +1066,7 @@ TEST_F(SSLStreamAdapterTestDTLSFromPEMStrings, TestDTLSGetPeerCertificate) { ASSERT_EQ(kCERT_PEM, server_peer_cert->ToPEMString()); // It must not have a chain, because the test certs are self-signed. - rtc::SSLCertChain* server_peer_chain; - ASSERT_FALSE(server_peer_cert->GetChain(&server_peer_chain)); + ASSERT_FALSE(server_peer_cert->GetChain()); } // Test getting the used DTLS ciphers.