Skip to content

Commit

Permalink
Refactor SSLClientSocket::SerializeNextProtos().
Browse files Browse the repository at this point in the history
Break out HTTP/2 protocol removal from SSLClientSocket::SerializeNextProtos() to
SSLClientSocket::DisableHTTP2() method.  DisableHTTP2() will be used for NPN to
create a NextProtoVector that can be used by the callback function (no
serialization is necessary for this.)

BUG=527066

Review URL: https://codereview.chromium.org/1371263002

Cr-Commit-Position: refs/heads/master@{#351210}
  • Loading branch information
bnc authored and Commit bot committed Sep 29, 2015
1 parent cf0aab0 commit f76254d
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 19 deletions.
11 changes: 11 additions & 0 deletions net/socket/next_proto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,15 @@ bool NextProtoIsSPDY(NextProto next_proto) {
next_proto <= kProtoSPDYMaximumVersion;
}

void DisableHTTP2(NextProtoVector* next_protos) {
for (NextProtoVector::iterator it = next_protos->begin();
it != next_protos->end();) {
if (*it == kProtoHTTP2) {
it = next_protos->erase(it);
continue;
}
++it;
}
}

} // namespace net
3 changes: 3 additions & 0 deletions net/socket/next_proto.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ NET_EXPORT NextProtoVector NextProtosWithSpdyAndQuic(bool spdy_enabled,
// Returns true if |next_proto| is a version of SPDY or HTTP/2.
bool NextProtoIsSPDY(NextProto next_proto);

// Remove HTTP/2 from |next_protos|.
NET_EXPORT void DisableHTTP2(NextProtoVector* next_protos);

} // namespace net

#endif // NET_SOCKET_NEXT_PROTO_H_
6 changes: 1 addition & 5 deletions net/socket/ssl_client_socket.cc
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,9 @@ bool SSLClientSocket::IsTLSVersionAdequateForHTTP2(

// static
std::vector<uint8_t> SSLClientSocket::SerializeNextProtos(
const NextProtoVector& next_protos,
bool can_advertise_http2) {
const NextProtoVector& next_protos) {
std::vector<uint8_t> wire_protos;
for (const NextProto next_proto : next_protos) {
if (!can_advertise_http2 && next_proto == kProtoHTTP2) {
continue;
}
const std::string proto = NextProtoToString(next_proto);
if (proto.size() > 255) {
LOG(WARNING) << "Ignoring overlong NPN/ALPN protocol: " << proto;
Expand Down
8 changes: 3 additions & 5 deletions net/socket/ssl_client_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,10 @@ class NET_EXPORT SSLClientSocket : public SSLSocket {
// inadequate TLS version.
static bool IsTLSVersionAdequateForHTTP2(const SSLConfig& ssl_config);

// Serializes |next_protos| in the wire format for ALPN: protocols are listed
// in order, each prefixed by a one-byte length. Any HTTP/2 protocols in
// |next_protos| are ignored if |can_advertise_http2| is false.
// Serialize |next_protos| in the wire format for ALPN and NPN: protocols are
// listed in order, each prefixed by a one-byte length.
static std::vector<uint8_t> SerializeNextProtos(
const NextProtoVector& next_protos,
bool can_advertise_http2);
const NextProtoVector& next_protos);

private:
FRIEND_TEST_ALL_PREFIXES(SSLClientSocket, SerializeNextProtos);
Expand Down
10 changes: 6 additions & 4 deletions net/socket/ssl_client_socket_nss.cc
Original file line number Diff line number Diff line change
Expand Up @@ -858,10 +858,12 @@ bool SSLClientSocketNSS::Core::Init(PRFileDesc* socket,
PK11_TokenExists(CKM_NSS_CHACHA20_POLY1305);
const bool adequate_key_agreement = PK11_TokenExists(CKM_DH_PKCS_DERIVE) ||
PK11_TokenExists(CKM_ECDH1_DERIVE);
std::vector<uint8_t> wire_protos =
SerializeNextProtos(ssl_config_.next_protos,
adequate_encryption && adequate_key_agreement &&
IsTLSVersionAdequateForHTTP2(ssl_config_));
NextProtoVector next_protos = ssl_config_.next_protos;
if (!adequate_encryption || !adequate_key_agreement ||
!IsTLSVersionAdequateForHTTP2(ssl_config_)) {
DisableHTTP2(&next_protos);
}
std::vector<uint8_t> wire_protos = SerializeNextProtos(next_protos);
rv = SSL_SetNextProtoNego(
nss_fd_, wire_protos.empty() ? NULL : &wire_protos[0],
wire_protos.size());
Expand Down
10 changes: 6 additions & 4 deletions net/socket/ssl_client_socket_openssl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -952,10 +952,12 @@ int SSLClientSocketOpenSSL::Init() {
enabled_ciphers_vector.push_back(id);
}

std::vector<uint8_t> wire_protos =
SerializeNextProtos(ssl_config_.next_protos,
HasCipherAdequateForHTTP2(enabled_ciphers_vector) &&
IsTLSVersionAdequateForHTTP2(ssl_config_));
NextProtoVector next_protos = ssl_config_.next_protos;
if (!HasCipherAdequateForHTTP2(enabled_ciphers_vector) ||
!IsTLSVersionAdequateForHTTP2(ssl_config_)) {
DisableHTTP2(&next_protos);
}
std::vector<uint8_t> wire_protos = SerializeNextProtos(next_protos);
SSL_set_alpn_protos(ssl_, wire_protos.empty() ? NULL : &wire_protos[0],
wire_protos.size());
}
Expand Down
2 changes: 1 addition & 1 deletion net/socket/ssl_client_socket_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2373,7 +2373,7 @@ TEST(SSLClientSocket, SerializeNextProtos) {
next_protos.push_back(kProtoHTTP11);
next_protos.push_back(kProtoSPDY31);
static std::vector<uint8_t> serialized =
SSLClientSocket::SerializeNextProtos(next_protos, true);
SSLClientSocket::SerializeNextProtos(next_protos);
ASSERT_EQ(18u, serialized.size());
EXPECT_EQ(8, serialized[0]); // length("http/1.1")
EXPECT_EQ('h', serialized[1]);
Expand Down

0 comments on commit f76254d

Please sign in to comment.