Merge remote-tracking branch 'upstream/master' into max_send_size_filter

14 files changed, 151 insertions, 174 deletions

diff --git a/BUILD b/BUILD
index cca2803a0a..6f6169e90d 100644
--- a/BUILD
+++ b/BUILD
@@ -36,6 +36,8 @@
 
 licenses(["notice"])  # 3-clause BSD
 
+exports_files(["LICENSE"])
+
 package(default_visibility = ["//visibility:public"])
 
 
diff --git a/doc/PROTOCOL-HTTP2.md b/doc/PROTOCOL-HTTP2.md
index 31d694b803..df7585d609 100644
--- a/doc/PROTOCOL-HTTP2.md
+++ b/doc/PROTOCOL-HTTP2.md
@@ -98,8 +98,11 @@ For requests, **EOS** (end-of-stream) is indicated by the presence of the END_ST
 * **Trailers-Only** → HTTP-Status Content-Type Trailers
 * **Trailers** → Status [Status-Message] \*Custom-Metadata
 * **HTTP-Status** → ":status 200"
-* **Status** → "grpc-status" <status-code-as-ASCII-string>
-* **Status-Message** → "grpc-message" <descriptive text for status as ASCII string>
+* **Status** → "grpc-status" 1\*DIGIT ; 0-9
+* **Status-Message** → "grpc-message" Percent-Encoded
+* **Percent-Encoded** → 1\*(Percent-Byte-Unencoded / Percent-Byte-Encoded)
+* **Percent-Byte-Unencoded** → 1\*( %x20-%x24 / %x26-%x7E ) ; space and VCHAR, except %
+* **Percent-Byte-Encoded** → "%" 2HEXDIGIT ; 0-9 A-F
 
 **Response-Headers** & **Trailers-Only** are each delivered in a single HTTP2 HEADERS frame block. Most responses are expected to have both headers and trailers but **Trailers-Only** is permitted for calls that produce an immediate error. Status must be sent in **Trailers** even if the status code is OK.
 
@@ -110,6 +113,21 @@ Implementations should expect broken deployments to send non-200 HTTP status cod
 Clients may limit the size of **Response-Headers**, **Trailers**, and
 **Trailers-Only**, with a default of 8 KiB each suggested.
 
+The value portion of **Status** is a decimal-encoded integer as an ASCII string,
+without any leading zeros.
+
+The value portion of **Status-Message** is conceptually a Unicode string
+description of the error, physically encoded as UTF-8 followed by
+percent-encoding. Percent-encoding is specified in [RFC 3986
+§2.1](https://tools.ietf.org/html/rfc3986#section-2.1), although the form used
+here has different restricted characters. When decoding invalid values,
+implementations MUST NOT error or throw away the message. At worst, the
+implementation can abort decoding the status message altogether such that the
+user would received the raw percent-encoded form. Alternatively, the
+implementation can decode valid portions while leaving broken %-encodings as-is
+or replacing them with a replacement character (e.g., '?' or the Unicode
+replacement character).
+
 ####Example
 
 Sample unary-call showing HTTP2 framing sequence
diff --git a/examples/php/composer.json b/examples/php/composer.json
index 97e9608fe0..e6409f87b4 100644
--- a/examples/php/composer.json
+++ b/examples/php/composer.json
@@ -2,6 +2,6 @@
   "name": "grpc/grpc-demo",
   "description": "gRPC example for PHP",
   "require": {
-    "grpc/grpc": "v1.0.0",
+    "grpc/grpc": "v1.0.0"
   }
 }
diff --git a/package.xml b/package.xml
index c93c790c70..19d067dda7 100644
--- a/package.xml
+++ b/package.xml
@@ -22,7 +22,7 @@
  </stability>
  <license>BSD</license>
  <notes>
-- TBD
+- Reject metadata keys which are not legal #7881
  </notes>
  <contents>
   <dir baseinstalldir="/" name="/">
@@ -1183,7 +1183,7 @@ Update to wrap gRPC C Core version 0.10.0
    <date>2016-08-22</date>
    <license>BSD</license>
    <notes>
-- TBD
+- Reject metadata keys which are not legal #7881
    </notes>
   </release>
  </changelog>
diff --git a/src/core/lib/iomgr/ev_epoll_linux.c b/src/core/lib/iomgr/ev_epoll_linux.c
index 02bcbaa10f..27264694d1 100644
--- a/src/core/lib/iomgr/ev_epoll_linux.c
+++ b/src/core/lib/iomgr/ev_epoll_linux.c
@@ -1353,8 +1353,10 @@ static void pollset_work_and_unlock(grpc_exec_ctx *exec_ctx,
   gpr_mu_unlock(&pollset->mu);
 
   do {
+    GRPC_SCHEDULING_START_BLOCKING_REGION;
     ep_rv = epoll_pwait(epoll_fd, ep_ev, GRPC_EPOLL_MAX_EVENTS, timeout_ms,
                         sig_mask);
+    GRPC_SCHEDULING_END_BLOCKING_REGION;
     if (ep_rv < 0) {
       if (errno != EINTR) {
         gpr_asprintf(&err_msg,
diff --git a/src/php/ext/grpc/call.c b/src/php/ext/grpc/call.c
index 66ca1513ed..31c59fe5ad 100644
--- a/src/php/ext/grpc/call.c
+++ b/src/php/ext/grpc/call.c
@@ -164,6 +164,9 @@ bool create_metadata_array(zval *array, grpc_metadata_array *metadata) {
     if (key_type1 != HASH_KEY_IS_STRING) {
       return false;
     }
+    if (!grpc_header_key_is_legal(key1, strlen(key1))) {
+      return false;
+    }
     inner_array_hash = Z_ARRVAL_P(inner_array);
     PHP_GRPC_HASH_FOREACH_VAL_START(inner_array_hash, value)
       if (Z_TYPE_P(value) != IS_STRING) {
diff --git a/src/php/ext/grpc/call_credentials.c b/src/php/ext/grpc/call_credentials.c
index 6921a5df17..25c92c91fe 100644
--- a/src/php/ext/grpc/call_credentials.c
+++ b/src/php/ext/grpc/call_credentials.c
@@ -192,24 +192,16 @@ void plugin_get_metadata(void *ptr, grpc_auth_metadata_context context,
   /* call the user callback function */
   zend_call_function(state->fci, state->fci_cache TSRMLS_CC);
 
-  if (Z_TYPE_P(retval) != IS_ARRAY) {
-    zend_throw_exception(spl_ce_InvalidArgumentException,
-                         "plugin callback must return metadata array",
-                         1 TSRMLS_CC);
-    return;
-  }
-
+  grpc_status_code code = GRPC_STATUS_OK;
   grpc_metadata_array metadata;
-  if (!create_metadata_array(retval, &metadata)) {
-    zend_throw_exception(spl_ce_InvalidArgumentException,
-                         "invalid metadata", 1 TSRMLS_CC);
+
+  if (Z_TYPE_P(retval) != IS_ARRAY) {
+    code = GRPC_STATUS_INVALID_ARGUMENT;
+  } else if (!create_metadata_array(retval, &metadata)) {
     grpc_metadata_array_destroy(&metadata);
-    return;
+    code = GRPC_STATUS_INVALID_ARGUMENT;
   }
 
-  /* TODO: handle error */
-  grpc_status_code code = GRPC_STATUS_OK;
-
   /* Pass control back to core */
   cb(user_data, metadata.metadata, metadata.count, code, NULL);
 }
diff --git a/src/php/tests/interop/interop_client.php b/src/php/tests/interop/interop_client.php
index bf40549a04..c94ba61296 100755
--- a/src/php/tests/interop/interop_client.php
+++ b/src/php/tests/interop/interop_client.php
@@ -54,6 +54,15 @@ function hardAssert($value, $error_message)
     }
 }
 
+function hardAssertIfStatusOk($status)
+{
+    if ($status->code !== Grpc\STATUS_OK) {
+        echo "Call did not complete successfully. Status object:\n";
+        var_dump($status);
+        exit(1);
+    }
+}
+
 /**
  * Run the empty_unary test.
  *
@@ -62,7 +71,7 @@ function hardAssert($value, $error_message)
 function emptyUnary($stub)
 {
     list($result, $status) = $stub->EmptyCall(new grpc\testing\EmptyMessage())->wait();
-    hardAssert($status->code === Grpc\STATUS_OK, 'Call did not complete successfully');
+    hardAssertIfStatusOk($status);
     hardAssert($result !== null, 'Call completed with a null response');
 }
 
@@ -105,7 +114,7 @@ function performLargeUnary($stub, $fillUsername = false, $fillOauthScope = false
     }
 
     list($result, $status) = $stub->UnaryCall($request, [], $options)->wait();
-    hardAssert($status->code === Grpc\STATUS_OK, 'Call did not complete successfully');
+    hardAssertIfStatusOk($status);
     hardAssert($result !== null, 'Call returned a null response');
     $payload = $result->getPayload();
     hardAssert($payload->getType() === grpc\testing\PayloadType::COMPRESSABLE,
@@ -197,7 +206,12 @@ function updateAuthMetadataCallback($context)
     $methodName = $context->method_name;
     $auth_credentials = ApplicationDefaultCredentials::getCredentials();
 
-    return $auth_credentials->updateMetadata($metadata = [], $authUri);
+    $metadata = [];
+    $result = $auth_credentials->updateMetadata([], $authUri);
+    foreach ($result as $key => $value) {
+      $metadata[strtolower($key)] = $value;
+    }
+    return $metadata;
 }
 
 /**
@@ -242,7 +256,7 @@ function clientStreaming($stub)
         $call->write($request);
     }
     list($result, $status) = $call->wait();
-    hardAssert($status->code === Grpc\STATUS_OK, 'Call did not complete successfully');
+    hardAssertIfStatusOk($status);
     hardAssert($result->getAggregatedPayloadSize() === 74922,
               'aggregated_payload_size was incorrect');
 }
@@ -275,8 +289,7 @@ function serverStreaming($stub)
                 'Response '.$i.' had the wrong length');
         $i += 1;
     }
-    hardAssert($call->getStatus()->code === Grpc\STATUS_OK,
-             'Call did not complete successfully');
+    hardAssertIfStatusOk($call->getStatus());
 }
 
 /**
@@ -312,8 +325,7 @@ function pingPong($stub)
     }
     $call->writesDone();
     hardAssert($call->read() === null, 'Server returned too many responses');
-    hardAssert($call->getStatus()->code === Grpc\STATUS_OK,
-              'Call did not complete successfully');
+    hardAssertIfStatusOk($call->getStatus());
 }
 
 /**
@@ -326,8 +338,7 @@ function emptyStream($stub)
     $call = $stub->FullDuplexCall();
     $call->writesDone();
     hardAssert($call->read() === null, 'Server returned too many responses');
-    hardAssert($call->getStatus()->code === Grpc\STATUS_OK,
-             'Call did not complete successfully');
+    hardAssertIfStatusOk($call->getStatus());
 }
 
 /**
@@ -419,8 +430,7 @@ function customMetadata($stub)
                'Incorrect initial metadata value');
 
     list($result, $status) = $call->wait();
-    hardAssert($status->code === Grpc\STATUS_OK,
-               'Call did not complete successfully');
+    hardAssertIfStatusOk($status);
 
     $trailing_metadata = $call->getTrailingMetadata();
     hardAssert(array_key_exists($ECHO_TRAILING_KEY, $trailing_metadata),
@@ -435,8 +445,7 @@ function customMetadata($stub)
     $streaming_call->write($streaming_request);
     $streaming_call->writesDone();
 
-    hardAssert($streaming_call->getStatus()->code === Grpc\STATUS_OK,
-               'Call did not complete successfully');
+    hardAssertIfStatusOk($streaming_call->getStatus());
 
     $streaming_trailing_metadata = $streaming_call->getTrailingMetadata();
     hardAssert(array_key_exists($ECHO_TRAILING_KEY,
diff --git a/src/php/tests/unit_tests/CallCredentials2Test.php b/src/php/tests/unit_tests/CallCredentials2Test.php
index a57e2b9b4e..b3b98a22ca 100644
--- a/src/php/tests/unit_tests/CallCredentials2Test.php
+++ b/src/php/tests/unit_tests/CallCredentials2Test.php
@@ -132,4 +132,69 @@ class CallCredentials2Test extends PHPUnit_Framework_TestCase
         unset($call);
         unset($server_call);
     }
+
+    public function invalidKeyCallbackFunc($context)
+    {
+        $this->assertTrue(is_string($context->service_url));
+        $this->assertTrue(is_string($context->method_name));
+
+        return ['K1' => ['v1']];
+    }
+
+    public function testCallbackWithInvalidKey()
+    {
+        $deadline = Grpc\Timeval::infFuture();
+        $status_text = 'xyz';
+        $call = new Grpc\Call($this->channel,
+                              '/abc/dummy_method',
+                              $deadline,
+                              $this->host_override);
+
+        $call_credentials = Grpc\CallCredentials::createFromPlugin(
+            array($this, 'invalidKeyCallbackFunc'));
+        $call->setCredentials($call_credentials);
+
+        $event = $call->startBatch([
+            Grpc\OP_SEND_INITIAL_METADATA => [],
+            Grpc\OP_SEND_CLOSE_FROM_CLIENT => true,
+            Grpc\OP_RECV_STATUS_ON_CLIENT => true,
+        ]);
+
+        $this->assertTrue($event->send_metadata);
+        $this->assertTrue($event->send_close);
+        $this->assertTrue($event->status->code == Grpc\STATUS_UNAUTHENTICATED);
+    }
+
+    public function invalidReturnCallbackFunc($context)
+    {
+        $this->assertTrue(is_string($context->service_url));
+        $this->assertTrue(is_string($context->method_name));
+
+        return "a string";
+    }
+
+    public function testCallbackWithInvalidReturnValue()
+    {
+        $deadline = Grpc\Timeval::infFuture();
+        $status_text = 'xyz';
+        $call = new Grpc\Call($this->channel,
+                              '/abc/dummy_method',
+                              $deadline,
+                              $this->host_override);
+
+        $call_credentials = Grpc\CallCredentials::createFromPlugin(
+            array($this, 'invalidReturnCallbackFunc'));
+        $call->setCredentials($call_credentials);
+
+        $event = $call->startBatch([
+            Grpc\OP_SEND_INITIAL_METADATA => [],
+            Grpc\OP_SEND_CLOSE_FROM_CLIENT => true,
+            Grpc\OP_RECV_STATUS_ON_CLIENT => true,
+        ]);
+
+        $this->assertTrue($event->send_metadata);
+        $this->assertTrue($event->send_close);
+        $this->assertTrue($event->status->code == Grpc\STATUS_UNAUTHENTICATED);
+    }
+
 }
diff --git a/src/php/tests/unit_tests/CallCredentials3Test.php b/src/php/tests/unit_tests/CallCredentials3Test.php
deleted file mode 100644
index 8f5e109bf5..0000000000
--- a/src/php/tests/unit_tests/CallCredentials3Test.php
+++ /dev/null
@@ -1,135 +0,0 @@
-<?php
-/*
- *
- * Copyright 2015, Google Inc.
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met:
- *
- *     * Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above
- * copyright notice, this list of conditions and the following disclaimer
- * in the documentation and/or other materials provided with the
- * distribution.
- *     * Neither the name of Google Inc. nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
- */
-
-class CallCredentials3Test extends PHPUnit_Framework_TestCase
-{
-    public function setUp()
-    {
-        $this->credentials = Grpc\ChannelCredentials::createSsl(
-            file_get_contents(dirname(__FILE__).'/../data/ca.pem'));
-        $server_credentials = Grpc\ServerCredentials::createSsl(
-            null,
-            file_get_contents(dirname(__FILE__).'/../data/server1.key'),
-            file_get_contents(dirname(__FILE__).'/../data/server1.pem'));
-        $this->server = new Grpc\Server();
-        $this->port = $this->server->addSecureHttp2Port('0.0.0.0:0',
-                                              $server_credentials);
-        $this->server->start();
-        $this->host_override = 'foo.test.google.fr';
-        $this->channel = new Grpc\Channel(
-            'localhost:'.$this->port,
-            [
-            'grpc.ssl_target_name_override' => $this->host_override,
-            'grpc.default_authority' => $this->host_override,
-            'credentials' => $this->credentials,
-            ]
-        );
-    }
-
-    public function tearDown()
-    {
-        unset($this->channel);
-        unset($this->server);
-    }
-
-    public function callbackFunc($context)
-    {
-        $this->assertTrue(is_string($context->service_url));
-        $this->assertTrue(is_string($context->method_name));
-
-        return ['k1' => ['v1'], 'k2' => ['v2']];
-    }
-
-    public function testCreateFromPlugin()
-    {
-        $deadline = Grpc\Timeval::infFuture();
-        $status_text = 'xyz';
-        $call = new Grpc\Call($this->channel,
-                              '/abc/dummy_method',
-                              $deadline,
-                              $this->host_override);
-
-        $call_credentials = Grpc\CallCredentials::createFromPlugin(
-            [$this, 'callbackFunc']);
-        $call->setCredentials($call_credentials);
-
-        $event = $call->startBatch([
-            Grpc\OP_SEND_INITIAL_METADATA => [],
-            Grpc\OP_SEND_CLOSE_FROM_CLIENT => true,
-        ]);
-
-        $this->assertTrue($event->send_metadata);
-        $this->assertTrue($event->send_close);
-
-        $event = $this->server->requestCall();
-
-        $this->assertTrue(is_array($event->metadata));
-        $metadata = $event->metadata;
-        $this->assertTrue(array_key_exists('k1', $metadata));
-        $this->assertTrue(array_key_exists('k2', $metadata));
-        $this->assertSame($metadata['k1'], ['v1']);
-        $this->assertSame($metadata['k2'], ['v2']);
-
-        $this->assertSame('/abc/dummy_method', $event->method);
-        $server_call = $event->call;
-
-        $event = $server_call->startBatch([
-            Grpc\OP_SEND_INITIAL_METADATA => [],
-            Grpc\OP_SEND_STATUS_FROM_SERVER => [
-                'metadata' => [],
-                'code' => Grpc\STATUS_OK,
-                'details' => $status_text,
-            ],
-            Grpc\OP_RECV_CLOSE_ON_SERVER => true,
-        ]);
-
-        $this->assertTrue($event->send_metadata);
-        $this->assertTrue($event->send_status);
-        $this->assertFalse($event->cancelled);
-
-        $event = $call->startBatch([
-            Grpc\OP_RECV_INITIAL_METADATA => true,
-            Grpc\OP_RECV_STATUS_ON_CLIENT => true,
-        ]);
-
-        $this->assertSame([], $event->metadata);
-        $status = $event->status;
-        $this->assertSame([], $status->metadata);
-        $this->assertSame(Grpc\STATUS_OK, $status->code);
-        $this->assertSame($status_text, $status->details);
-
-        unset($call);
-        unset($server_call);
-    }
-}
diff --git a/src/php/tests/unit_tests/CallTest.php b/src/php/tests/unit_tests/CallTest.php
index d736f51546..1205f0cd8e 100644
--- a/src/php/tests/unit_tests/CallTest.php
+++ b/src/php/tests/unit_tests/CallTest.php
@@ -113,7 +113,7 @@ class CallTest extends PHPUnit_Framework_TestCase
     /**
      * @expectedException InvalidArgumentException
      */
-    public function testInvalidMetadataKey()
+    public function testInvalidStartBatchKey()
     {
         $batch = [
             'invalid' => ['key1' => 'value1'],
@@ -124,6 +124,28 @@ class CallTest extends PHPUnit_Framework_TestCase
     /**
      * @expectedException InvalidArgumentException
      */
+    public function testInvalidMetadataStrKey()
+    {
+        $batch = [
+            Grpc\OP_SEND_INITIAL_METADATA => ['Key' => ['value1', 'value2']],
+        ];
+        $result = $this->call->startBatch($batch);
+    }
+
+    /**
+     * @expectedException InvalidArgumentException
+     */
+    public function testInvalidMetadataIntKey()
+    {
+        $batch = [
+            Grpc\OP_SEND_INITIAL_METADATA => [1 => ['value1', 'value2']],
+        ];
+        $result = $this->call->startBatch($batch);
+    }
+
+    /**
+     * @expectedException InvalidArgumentException
+     */
     public function testInvalidMetadataInnerValue()
     {
         $batch = [
diff --git a/src/python/grpcio_tests/tests/unit/_cython/cygrpc_test.py b/src/python/grpcio_tests/tests/unit/_cython/cygrpc_test.py
index 2f50263730..142387d810 100644
--- a/src/python/grpcio_tests/tests/unit/_cython/cygrpc_test.py
+++ b/src/python/grpcio_tests/tests/unit/_cython/cygrpc_test.py
@@ -114,9 +114,6 @@ class TypeSmokeTest(unittest.TestCase):
         lambda ignored_a, ignored_b: None, b'')
     del plugin
 
-  @unittest.skipIf(
-    platform.python_implementation() == "PyPy",
-    'TODO(issue 7672): figure out why this fails on PyPy')
   def testCallCredentialsFromPluginUpDown(self):
     plugin = cygrpc.CredentialsMetadataPlugin(_metadata_plugin_callback, b'')
     call_credentials = cygrpc.call_credentials_metadata_plugin(plugin)
diff --git a/templates/BUILD.template b/templates/BUILD.template
index 23a656c360..af23fb2799 100644
--- a/templates/BUILD.template
+++ b/templates/BUILD.template
@@ -38,6 +38,8 @@
   
   licenses(["notice"])  # 3-clause BSD
   
+  exports_files(["LICENSE"])
+  
   package(default_visibility = ["//visibility:public"])
   
   <%!
diff --git a/templates/package.xml.template b/templates/package.xml.template
index 65fef1892f..32ed3b633e 100644
--- a/templates/package.xml.template
+++ b/templates/package.xml.template
@@ -24,7 +24,7 @@
    </stability>
    <license>BSD</license>
    <notes>
-  - TBD
+  - Reject metadata keys which are not legal #7881
    </notes>
    <contents>
     <dir baseinstalldir="/" name="/">
@@ -291,7 +291,7 @@
      <date>2016-08-22</date>
      <license>BSD</license>
      <notes>
-  - TBD
+  - Reject metadata keys which are not legal #7881
      </notes>
     </release>
    </changelog>