Add functions to convert from Span<const uint8> and std::string_view
These can replace the current der::Input-specific string_view methods. I
converted many of the uses within the library, but we should take a pass
over the library and turn many of the std::strings into
std::vector<uint8_t>.
On MSVC, we have to pass /Zc:__cplusplus for __cplusplus to be set
correctly. For now, I'm going to try just adding the flag, but if pki
gets used in more places before we bump our minimum to C++17, we may
want to start looking at _MSVC_LANG instead.
There are actually a few places outside of pki that could use this, but
we sadly can't use them until we require C++17 across the board.
Bug: 661
Change-Id: I1002d9f2e1e4a2592760e8938560894d42a9b58c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65908
Reviewed-by: Bob Beck <bbe@google.com>
Reviewed-by: Matt Mueller <mattm@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 0639217..5933f02 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -203,7 +203,11 @@
string(REPLACE "C" " -wd" MSVC_DISABLED_WARNINGS_STR
${MSVC_DISABLED_WARNINGS_LIST})
set(CMAKE_C_FLAGS "-utf-8 -W4 -WX ${MSVC_DISABLED_WARNINGS_STR}")
- set(CMAKE_CXX_FLAGS "-utf-8 -W4 -WX ${MSVC_DISABLED_WARNINGS_STR}")
+ # Without /Zc:__cplusplus, MSVC does not define the right value for
+ # __cplusplus. See https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/
+ # If this becomes too problematic for downstream code, we can look at
+ # _MSVC_LANG.
+ set(CMAKE_CXX_FLAGS "-utf-8 -W4 -WX ${MSVC_DISABLED_WARNINGS_STR} -Zc:__cplusplus")
endif()
if(WIN32)
diff --git a/include/openssl/span.h b/include/openssl/span.h
index dd66f88..38196ae 100644
--- a/include/openssl/span.h
+++ b/include/openssl/span.h
@@ -26,6 +26,10 @@
#include <algorithm>
#include <type_traits>
+#if __cplusplus >= 201703L
+#include <string_view>
+#endif
+
BSSL_NAMESPACE_BEGIN
template <typename T>
@@ -210,6 +214,16 @@
return array;
}
+#if __cplusplus >= 201703L
+inline Span<const uint8_t> StringAsBytes(std::string_view s) {
+ return MakeConstSpan(reinterpret_cast<const uint8_t *>(s.data()), s.size());
+}
+
+inline std::string_view BytesAsStringView(bssl::Span<const uint8_t> b) {
+ return std::string_view(reinterpret_cast<const char *>(b.data()), b.size());
+}
+#endif
+
BSSL_NAMESPACE_END
} // extern C++
diff --git a/pki/cert_error_params.cc b/pki/cert_error_params.cc
index c64fa1b..075d7ef 100644
--- a/pki/cert_error_params.cc
+++ b/pki/cert_error_params.cc
@@ -22,9 +22,9 @@
CertErrorParams2Der(const char *name1, der::Input der1, const char *name2,
der::Input der2)
: name1_(name1),
- der1_(der1.AsString()),
+ der1_(BytesAsStringView(der1)),
name2_(name2),
- der2_(der2.AsString()) {}
+ der2_(BytesAsStringView(der2)) {}
CertErrorParams2Der(const CertErrorParams2Der &) = delete;
CertErrorParams2Der &operator=(const CertErrorParams2Der &) = delete;
diff --git a/pki/cert_issuer_source_static.cc b/pki/cert_issuer_source_static.cc
index bbf967f..fc20eb9 100644
--- a/pki/cert_issuer_source_static.cc
+++ b/pki/cert_issuer_source_static.cc
@@ -12,7 +12,7 @@
void CertIssuerSourceStatic::AddCert(
std::shared_ptr<const ParsedCertificate> cert) {
intermediates_.insert(std::make_pair(
- cert->normalized_subject().AsStringView(), std::move(cert)));
+ BytesAsStringView(cert->normalized_subject()), std::move(cert)));
}
void CertIssuerSourceStatic::Clear() { intermediates_.clear(); }
@@ -20,7 +20,7 @@
void CertIssuerSourceStatic::SyncGetIssuersOf(const ParsedCertificate *cert,
ParsedCertificateList *issuers) {
auto range =
- intermediates_.equal_range(cert->normalized_issuer().AsStringView());
+ intermediates_.equal_range(BytesAsStringView(cert->normalized_issuer()));
for (auto it = range.first; it != range.second; ++it) {
issuers->push_back(it->second);
}
diff --git a/pki/general_names.cc b/pki/general_names.cc
index cc7bf88..74874ba 100644
--- a/pki/general_names.cc
+++ b/pki/general_names.cc
@@ -114,7 +114,7 @@
} else if (tag == der::ContextSpecificPrimitive(1)) {
// rfc822Name [1] IA5String,
name_type = GENERAL_NAME_RFC822_NAME;
- const std::string_view s = value.AsStringView();
+ const std::string_view s = BytesAsStringView(value);
if (!bssl::string_util::IsAscii(s)) {
errors->AddError(kRFC822NameNotAscii);
return false;
@@ -123,7 +123,7 @@
} else if (tag == der::ContextSpecificPrimitive(2)) {
// dNSName [2] IA5String,
name_type = GENERAL_NAME_DNS_NAME;
- const std::string_view s = value.AsStringView();
+ const std::string_view s = BytesAsStringView(value);
if (!bssl::string_util::IsAscii(s)) {
errors->AddError(kDnsNameNotAscii);
return false;
@@ -152,7 +152,7 @@
} else if (tag == der::ContextSpecificPrimitive(6)) {
// uniformResourceIdentifier [6] IA5String,
name_type = GENERAL_NAME_UNIFORM_RESOURCE_IDENTIFIER;
- const std::string_view s = value.AsStringView();
+ const std::string_view s = BytesAsStringView(value);
if (!bssl::string_util::IsAscii(s)) {
errors->AddError(kURINotAscii);
return false;
diff --git a/pki/input.cc b/pki/input.cc
index 82450ea..156d248 100644
--- a/pki/input.cc
+++ b/pki/input.cc
@@ -8,15 +8,7 @@
namespace bssl::der {
-std::string Input::AsString() const {
- return std::string(reinterpret_cast<const char *>(data_.data()),
- data_.size());
-}
-
-std::string_view Input::AsStringView() const {
- return std::string_view(reinterpret_cast<const char *>(data_.data()),
- data_.size());
-}
+std::string Input::AsString() const { return std::string(AsStringView()); }
bool operator==(Input lhs, Input rhs) {
return MakeConstSpan(lhs) == MakeConstSpan(rhs);
diff --git a/pki/input.h b/pki/input.h
index 4a94110..30ce5d4 100644
--- a/pki/input.h
+++ b/pki/input.h
@@ -44,6 +44,8 @@
constexpr explicit Input(const uint8_t *data, size_t len)
: data_(MakeConstSpan(data, len)) {}
+ // Deprecated: Use StringAsBytes.
+ //
// Creates an Input from a std::string_view. The constructed Input is only
// valid as long as |data| points to live memory. If constructed from, say, a
// |std::string|, mutating the vector will invalidate the Input.
@@ -69,13 +71,17 @@
constexpr Input first(size_t len) const { return Input(data_.first(len)); }
constexpr Input last(size_t len) const { return Input(data_.last(len)); }
+ // Deprecated: use BytesAsStringView and convert to std::string.
+ //
// Returns a copy of the data represented by this object as a std::string.
std::string AsString() const;
+ // Deprecated: Use ByteAsString.
+ //
// Returns a std::string_view pointing to the same data as the Input. The
// resulting string_view must not outlive the data that was used to construct
// this Input.
- std::string_view AsStringView() const;
+ std::string_view AsStringView() const { return BytesAsStringView(data_); }
// Deprecated: This class implicitly converts to bssl::Span<const uint8_t>.
//
diff --git a/pki/ocsp.cc b/pki/ocsp.cc
index 2e5a98f..49076e9 100644
--- a/pki/ocsp.cc
+++ b/pki/ocsp.cc
@@ -696,7 +696,7 @@
// (3) Has signed the OCSP response using its public key.
for (const auto &responder_cert_tlv : response.certs) {
std::shared_ptr<const ParsedCertificate> cur_responder_certificate =
- OCSPParseCertificate(responder_cert_tlv.AsStringView());
+ OCSPParseCertificate(BytesAsStringView(responder_cert_tlv));
// If failed parsing the certificate, keep looking.
if (!cur_responder_certificate) {
diff --git a/pki/parse_certificate.cc b/pki/parse_certificate.cc
index 2de9f71..de910d3 100644
--- a/pki/parse_certificate.cc
+++ b/pki/parse_certificate.cc
@@ -872,7 +872,7 @@
// GeneralName ::= CHOICE {
if (access_location_tag == der::ContextSpecificPrimitive(6)) {
// uniformResourceIdentifier [6] IA5String,
- std::string_view uri = access_location_value.AsStringView();
+ std::string_view uri = BytesAsStringView(access_location_value);
if (!bssl::string_util::IsAscii(uri)) {
return false;
}
diff --git a/pki/parse_name.cc b/pki/parse_name.cc
index 87131a5..dca76b4 100644
--- a/pki/parse_name.cc
+++ b/pki/parse_name.cc
@@ -38,7 +38,7 @@
case der::kPrintableString:
return der::ParsePrintableString(value, out);
case der::kUtf8String:
- *out = value.AsString();
+ *out = BytesAsStringView(value);
return true;
case der::kUniversalString:
return der::ParseUniversalString(value, out);
@@ -53,7 +53,7 @@
PrintableStringHandling printable_string_handling, std::string *out) const {
if (printable_string_handling == PrintableStringHandling::kAsUTF8Hack &&
value_tag == der::kPrintableString) {
- *out = value.AsString();
+ *out = BytesAsStringView(value);
return true;
}
return ValueAsString(out);
@@ -65,7 +65,7 @@
case der::kPrintableString:
case der::kTeletexString:
case der::kUtf8String:
- *out = value.AsString();
+ *out = BytesAsStringView(value);
return true;
case der::kUniversalString:
return der::ParseUniversalString(value, out);
diff --git a/pki/parse_name_unittest.cc b/pki/parse_name_unittest.cc
index 27989df..9858121 100644
--- a/pki/parse_name_unittest.cc
+++ b/pki/parse_name_unittest.cc
@@ -187,13 +187,13 @@
ASSERT_EQ(3u, atv.size());
ASSERT_EQ(1u, atv[0].size());
ASSERT_EQ(der::Input(kTypeCountryNameOid), atv[0][0].type);
- ASSERT_EQ("US", atv[0][0].value.AsString());
+ ASSERT_EQ("US", BytesAsStringView(atv[0][0].value));
ASSERT_EQ(1u, atv[1].size());
ASSERT_EQ(der::Input(kTypeOrganizationNameOid), atv[1][0].type);
- ASSERT_EQ("Google Inc.", atv[1][0].value.AsString());
+ ASSERT_EQ("Google Inc.", BytesAsStringView(atv[1][0].value));
ASSERT_EQ(1u, atv[2].size());
ASSERT_EQ(der::Input(kTypeCommonNameOid), atv[2][0].type);
- ASSERT_EQ("Google Test CA", atv[2][0].value.AsString());
+ ASSERT_EQ("Google Test CA", BytesAsStringView(atv[2][0].value));
}
TEST(ParseNameTest, InvalidNameExtraData) {
diff --git a/pki/parse_values.cc b/pki/parse_values.cc
index b90ead3..8d07b3d 100644
--- a/pki/parse_values.cc
+++ b/pki/parse_values.cc
@@ -363,12 +363,12 @@
}
bool ParseIA5String(Input in, std::string *out) {
- for (char c : in.AsStringView()) {
- if (static_cast<uint8_t>(c) > 127) {
+ for (uint8_t c : in) {
+ if (c > 127) {
return false;
}
}
- *out = in.AsString();
+ *out = BytesAsStringView(in);
return true;
}
@@ -379,23 +379,23 @@
// Also ITU-T X.691 says it much more clearly:
// "for VisibleString [the range] is 32 to 126 ... For VisibleString .. all
// the values in the range are present."
- for (char c : in.AsStringView()) {
- if (static_cast<uint8_t>(c) < 32 || static_cast<uint8_t>(c) > 126) {
+ for (uint8_t c : in) {
+ if (c < 32 || c > 126) {
return false;
}
}
- *out = in.AsString();
+ *out = BytesAsStringView(in);
return true;
}
bool ParsePrintableString(Input in, std::string *out) {
- for (char c : in.AsStringView()) {
+ for (uint8_t c : in) {
if (!(OPENSSL_isalpha(c) || c == ' ' || (c >= '\'' && c <= ':') ||
c == '=' || c == '?')) {
return false;
}
}
- *out = in.AsString();
+ *out = BytesAsStringView(in);
return true;
}
diff --git a/pki/path_builder.cc b/pki/path_builder.cc
index 98c6438..4bb5d2f 100644
--- a/pki/path_builder.cc
+++ b/pki/path_builder.cc
@@ -295,11 +295,11 @@
void CertIssuersIter::AddIssuers(ParsedCertificateList new_issuers) {
for (std::shared_ptr<const ParsedCertificate> &issuer : new_issuers) {
- if (present_issuers_.find(issuer->der_cert().AsStringView()) !=
+ if (present_issuers_.find(BytesAsStringView(issuer->der_cert())) !=
present_issuers_.end()) {
continue;
}
- present_issuers_.insert(issuer->der_cert().AsStringView());
+ present_issuers_.insert(BytesAsStringView(issuer->der_cert()));
// Look up the trust for this issuer.
IssuerEntry entry;
@@ -423,9 +423,9 @@
// Note that subject_alt_names_extension().value will be empty if the cert
// had no SubjectAltName extension, so there is no need for a condition on
// has_subject_alt_names().
- return Key(cert->normalized_subject().AsStringView(),
- cert->subject_alt_names_extension().value.AsStringView(),
- cert->tbs().spki_tlv.AsStringView());
+ return Key(BytesAsStringView(cert->normalized_subject()),
+ BytesAsStringView(cert->subject_alt_names_extension().value),
+ BytesAsStringView(cert->tbs().spki_tlv));
}
std::vector<std::unique_ptr<CertIssuersIter>> cur_path_;
diff --git a/pki/trust_store_in_memory.cc b/pki/trust_store_in_memory.cc
index 3da1b8d..1c77b63 100644
--- a/pki/trust_store_in_memory.cc
+++ b/pki/trust_store_in_memory.cc
@@ -47,7 +47,8 @@
void TrustStoreInMemory::SyncGetIssuersOf(const ParsedCertificate *cert,
ParsedCertificateList *issuers) {
- auto range = entries_.equal_range(cert->normalized_issuer().AsStringView());
+ auto range =
+ entries_.equal_range(BytesAsStringView(cert->normalized_issuer()));
for (auto it = range.first; it != range.second; ++it) {
issuers->push_back(it->second.cert);
}
@@ -55,7 +56,7 @@
CertificateTrust TrustStoreInMemory::GetTrust(const ParsedCertificate *cert) {
// Check SPKI distrust first.
- if (distrusted_spkis_.find(cert->tbs().spki_tlv.AsString()) !=
+ if (distrusted_spkis_.find(BytesAsStringView(cert->tbs().spki_tlv)) !=
distrusted_spkis_.end()) {
return CertificateTrust::ForDistrusted();
}
@@ -80,13 +81,13 @@
entry.trust = trust;
// TODO(mattm): should this check for duplicate certificates?
- entries_.insert(
- std::make_pair(entry.cert->normalized_subject().AsStringView(), entry));
+ entries_.emplace(BytesAsStringView(entry.cert->normalized_subject()), entry);
}
const TrustStoreInMemory::Entry *TrustStoreInMemory::GetEntry(
const ParsedCertificate *cert) const {
- auto range = entries_.equal_range(cert->normalized_subject().AsStringView());
+ auto range =
+ entries_.equal_range(BytesAsStringView(cert->normalized_subject()));
for (auto it = range.first; it != range.second; ++it) {
if (cert == it->second.cert.get() ||
cert->der_cert() == it->second.cert->der_cert()) {
diff --git a/pki/trust_store_in_memory.h b/pki/trust_store_in_memory.h
index 7e5530d..4da26cd 100644
--- a/pki/trust_store_in_memory.h
+++ b/pki/trust_store_in_memory.h
@@ -89,7 +89,7 @@
std::unordered_multimap<std::string_view, Entry> entries_;
// Set of distrusted SPKIs.
- std::set<std::string> distrusted_spkis_;
+ std::set<std::string, std::less<>> distrusted_spkis_;
// Returns the `Entry` matching `cert`, or `nullptr` if not in the trust
// store.
diff --git a/pki/trust_store_in_memory_unittest.cc b/pki/trust_store_in_memory_unittest.cc
index 92c3bb8..4292675 100644
--- a/pki/trust_store_in_memory_unittest.cc
+++ b/pki/trust_store_in_memory_unittest.cc
@@ -77,7 +77,8 @@
TEST_F(TrustStoreInMemoryTest, DistrustBySPKI) {
TrustStoreInMemory in_memory;
- in_memory.AddDistrustedCertificateBySPKI(newroot_->tbs().spki_tlv.AsString());
+ in_memory.AddDistrustedCertificateBySPKI(
+ std::string(BytesAsStringView(newroot_->tbs().spki_tlv)));
// newroot_ is distrusted.
CertificateTrust trust = in_memory.GetTrust(newroot_.get());
@@ -98,7 +99,8 @@
TEST_F(TrustStoreInMemoryTest, DistrustBySPKIOverridesTrust) {
TrustStoreInMemory in_memory;
in_memory.AddTrustAnchor(newroot_);
- in_memory.AddDistrustedCertificateBySPKI(newroot_->tbs().spki_tlv.AsString());
+ in_memory.AddDistrustedCertificateBySPKI(
+ std::string(BytesAsStringView(newroot_->tbs().spki_tlv)));
// newroot_ is distrusted.
CertificateTrust trust = in_memory.GetTrust(newroot_.get());
