Advertise h2-16, h2-15, h2-14, and accept any of them.

(Fixing the rollback from earlier - we were passing '1' as the protocol count, not num_alpn_protocols)
	Change on 2014/12/10 by ctiller <ctiller@google.com>
-------------
Created by MOE: http://code.google.com/p/moe-java
MOE_MIGRATED_REVID=81783755

4 files changed, 77 insertions, 24 deletions

diff --git a/src/core/security/security_context.c b/src/core/security/security_context.c
index c56692ae83..cb97c35130 100644
--- a/src/core/security/security_context.c
+++ b/src/core/security/security_context.c
@@ -278,8 +278,12 @@ static grpc_security_status ssl_check_peer(const char *secure_peer_name,
   /* Check the ALPN. */
   const tsi_peer_property *p =
       tsi_peer_get_property_by_name(peer, TSI_SSL_ALPN_SELECTED_PROTOCOL);
-  if (p == NULL || p->type != TSI_PEER_PROPERTY_TYPE_STRING) {
-    gpr_log(GPR_ERROR, "Invalid or missing selected ALPN property.");
+  if (p == NULL) {
+    gpr_log(GPR_ERROR, "Missing selected ALPN property.");
+    return GRPC_SECURITY_ERROR;
+  }
+  if (p->type != TSI_PEER_PROPERTY_TYPE_STRING) {
+    gpr_log(GPR_ERROR, "Invalid selected ALPN property.");
     return GRPC_SECURITY_ERROR;
   }
   if (!grpc_chttp2_is_alpn_version_supported(p->value.string.data,
@@ -322,19 +326,29 @@ static grpc_security_context_vtable ssl_server_vtable = {
 grpc_security_status grpc_ssl_channel_security_context_create(
     grpc_credentials *request_metadata_creds, const grpc_ssl_config *config,
     const char *secure_peer_name, grpc_channel_security_context **ctx) {
-  const char *alpn_protocol_string = GRPC_CHTTP2_ALPN_VERSION;
-  unsigned char alpn_protocol_string_len =
-      (unsigned char)strlen(alpn_protocol_string);
+  size_t num_alpn_protocols = grpc_chttp2_num_alpn_versions();
+  const unsigned char **alpn_protocol_strings =
+      gpr_malloc(sizeof(const char *) * num_alpn_protocols);
+  unsigned char *alpn_protocol_string_lengths =
+      gpr_malloc(sizeof(unsigned char) * num_alpn_protocols);
   tsi_result result = TSI_OK;
   grpc_ssl_channel_security_context *c;
+  size_t i;
+
+  for (i = 0; i < num_alpn_protocols; i++) {
+    alpn_protocol_strings[i] =
+        (const unsigned char *)grpc_chttp2_get_alpn_version_index(i);
+    alpn_protocol_string_lengths[i] =
+        strlen(grpc_chttp2_get_alpn_version_index(i));
+  }
 
   if (config == NULL || secure_peer_name == NULL ||
       config->pem_root_certs == NULL) {
     gpr_log(GPR_ERROR, "An ssl channel needs a secure name and root certs.");
-    return GRPC_SECURITY_ERROR;
+    goto error;
   }
   if (!check_request_metadata_creds(request_metadata_creds)) {
-    return GRPC_SECURITY_ERROR;
+    goto error;
   }
 
   c = gpr_malloc(sizeof(grpc_ssl_channel_security_context));
@@ -351,31 +365,48 @@ grpc_security_status grpc_ssl_channel_security_context_create(
       config->pem_private_key, config->pem_private_key_size,
       config->pem_cert_chain, config->pem_cert_chain_size,
       config->pem_root_certs, config->pem_root_certs_size,
-      GRPC_SSL_CIPHER_SUITES, (const unsigned char **)&alpn_protocol_string,
-      &alpn_protocol_string_len, 1, &c->handshaker_factory);
+      GRPC_SSL_CIPHER_SUITES, alpn_protocol_strings,
+      alpn_protocol_string_lengths, num_alpn_protocols, &c->handshaker_factory);
   if (result != TSI_OK) {
     gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.",
             tsi_result_to_string(result));
     ssl_channel_destroy(&c->base.base);
     *ctx = NULL;
-    return GRPC_SECURITY_ERROR;
+    goto error;
   }
   *ctx = &c->base;
+  gpr_free(alpn_protocol_strings);
+  gpr_free(alpn_protocol_string_lengths);
   return GRPC_SECURITY_OK;
+
+error:
+  gpr_free(alpn_protocol_strings);
+  gpr_free(alpn_protocol_string_lengths);
+  return GRPC_SECURITY_ERROR;
 }
 
 grpc_security_status grpc_ssl_server_security_context_create(
     const grpc_ssl_config *config, grpc_security_context **ctx) {
-  const char *alpn_protocol_string = GRPC_CHTTP2_ALPN_VERSION;
-  unsigned char alpn_protocol_string_len =
-      (unsigned char)strlen(alpn_protocol_string);
+  size_t num_alpn_protocols = grpc_chttp2_num_alpn_versions();
+  const unsigned char **alpn_protocol_strings =
+      gpr_malloc(sizeof(const char *) * num_alpn_protocols);
+  unsigned char *alpn_protocol_string_lengths =
+      gpr_malloc(sizeof(unsigned char) * num_alpn_protocols);
   tsi_result result = TSI_OK;
   grpc_ssl_server_security_context *c;
+  size_t i;
+
+  for (i = 0; i < num_alpn_protocols; i++) {
+    alpn_protocol_strings[i] =
+        (const unsigned char *)grpc_chttp2_get_alpn_version_index(i);
+    alpn_protocol_string_lengths[i] =
+        strlen(grpc_chttp2_get_alpn_version_index(i));
+  }
 
   if (config == NULL || config->pem_private_key == NULL ||
       config->pem_cert_chain == NULL) {
     gpr_log(GPR_ERROR, "An SSL server needs a key and a cert.");
-    return GRPC_SECURITY_ERROR;
+    goto error;
   }
   c = gpr_malloc(sizeof(grpc_ssl_server_security_context));
   memset(c, 0, sizeof(grpc_ssl_server_security_context));
@@ -388,17 +419,24 @@ grpc_security_status grpc_ssl_server_security_context_create(
       (const unsigned char **)&config->pem_cert_chain,
       (const gpr_uint32 *)&config->pem_cert_chain_size, 1,
       config->pem_root_certs, config->pem_root_certs_size,
-      GRPC_SSL_CIPHER_SUITES, (const unsigned char **)&alpn_protocol_string,
-      &alpn_protocol_string_len, 1, &c->handshaker_factory);
+      GRPC_SSL_CIPHER_SUITES, alpn_protocol_strings,
+      alpn_protocol_string_lengths, num_alpn_protocols, &c->handshaker_factory);
   if (result != TSI_OK) {
     gpr_log(GPR_ERROR, "Handshaker factory creation failed with %s.",
             tsi_result_to_string(result));
     ssl_server_destroy(&c->base);
     *ctx = NULL;
-    return GRPC_SECURITY_ERROR;
+    goto error;
   }
   *ctx = &c->base;
+  gpr_free(alpn_protocol_strings);
+  gpr_free(alpn_protocol_string_lengths);
   return GRPC_SECURITY_OK;
+
+error:
+  gpr_free(alpn_protocol_strings);
+  gpr_free(alpn_protocol_string_lengths);
+  return GRPC_SECURITY_ERROR;
 }
 
 
diff --git a/src/core/transport/chttp2/alpn.c b/src/core/transport/chttp2/alpn.c
index cd9cf67a90..8107406f8b 100644
--- a/src/core/transport/chttp2/alpn.c
+++ b/src/core/transport/chttp2/alpn.c
@@ -32,14 +32,25 @@
  */
 
 #include "src/core/transport/chttp2/alpn.h"
+#include <grpc/support/log.h>
+#include <grpc/support/useful.h>
 
-static const char *const supported_versions[] = {GRPC_CHTTP2_ALPN_VERSION,
-                                                 "h2-15", "h2-14"};
+/* in order of preference */
+static const char *const supported_versions[] = {"h2-16", "h2-15", "h2-14"};
 
 int grpc_chttp2_is_alpn_version_supported(const char *version, size_t size) {
   size_t i;
-  for (i = 0; i < sizeof(supported_versions) / sizeof(const char *); i++) {
+  for (i = 0; i < GPR_ARRAY_SIZE(supported_versions); i++) {
     if (!strncmp(version, supported_versions[i], size)) return 1;
   }
   return 0;
 }
+
+size_t grpc_chttp2_num_alpn_versions() {
+  return GPR_ARRAY_SIZE(supported_versions);
+}
+
+const char *grpc_chttp2_get_alpn_version_index(size_t i) {
+  GPR_ASSERT(i < GPR_ARRAY_SIZE(supported_versions));
+  return supported_versions[i];
+}
diff --git a/src/core/transport/chttp2/alpn.h b/src/core/transport/chttp2/alpn.h
index 1353a18b1b..de4da8fedb 100644
--- a/src/core/transport/chttp2/alpn.h
+++ b/src/core/transport/chttp2/alpn.h
@@ -36,9 +36,14 @@
 
 #include <string.h>
 
-#define GRPC_CHTTP2_ALPN_VERSION "h2-15"
-
 /* Retuns 1 if the version is supported, 0 otherwise. */
 int grpc_chttp2_is_alpn_version_supported(const char *version, size_t size);
 
+/* Returns the number of protocol versions to advertise */
+size_t grpc_chttp2_num_alpn_versions();
+
+/* Returns the protocol version at index i (0 <= i <
+ * grpc_chttp2_num_alpn_versions()) */
+const char *grpc_chttp2_get_alpn_version_index(size_t i);
+
 #endif /* __GRPC_INTERNAL_TRANSPORT_CHTTP2_ALPN_H_ */
diff --git a/test/core/transport/chttp2/alpn_test.c b/test/core/transport/chttp2/alpn_test.c
index 65b7af8683..7a70b0ca7f 100644
--- a/test/core/transport/chttp2/alpn_test.c
+++ b/test/core/transport/chttp2/alpn_test.c
@@ -37,8 +37,7 @@
 #include "test/core/util/test_config.h"
 
 static void test_alpn_success(void) {
-  const char *version = GRPC_CHTTP2_ALPN_VERSION;
-  GPR_ASSERT(grpc_chttp2_is_alpn_version_supported(version, strlen(version)));
+  GPR_ASSERT(grpc_chttp2_is_alpn_version_supported("h2-16", 5));
   GPR_ASSERT(grpc_chttp2_is_alpn_version_supported("h2-15", 5));
   GPR_ASSERT(grpc_chttp2_is_alpn_version_supported("h2-14", 5));
 }