feat(gax): implement REST URI percent-encoding and dot segment validation

Gax/src/PathTemplate.php

@@ -83,9 +83,9 @@ public function __toString()
      *    can't be parsed.
      * @return string A rendered representation of this path template.
      */
-    public function render(array $bindings)
+    public function render(array $bindings, bool $urlEncode = false)
     {
-        return $this->resourceTemplate->render($bindings);
+        return $this->resourceTemplate->render($bindings, $urlEncode);
     }
 
     /**

Gax/src/RequestBuilder.php

@@ -259,7 +259,7 @@ private function tryRenderPathTemplate(string $uriTemplate, array $bindings)
         $template = new AbsoluteResourceTemplate($uriTemplate);
 
         try {
-            return $template->render($bindings);
+            return $template->render($bindings, true);
         } catch (ValidationException $e) {
             return null;
         }

Gax/src/ResourceTemplate/AbsoluteResourceTemplate.php

@@ -88,9 +88,9 @@ public function __toString()
     /**
      * @inheritdoc
      */
-    public function render(array $bindings)
+    public function render(array $bindings, bool $urlEncode = false)
     {
-        return sprintf('/%s%s', $this->resourceTemplate->render($bindings), $this->renderVerb());
+        return sprintf('/%s%s', $this->resourceTemplate->render($bindings, $urlEncode), $this->renderVerb());
     }
 
     /**

Gax/src/ResourceTemplate/RelativeResourceTemplate.php

@@ -101,7 +101,7 @@ public function __toString()
     /**
      * @inheritdoc
      */
-    public function render(array $bindings)
+    public function render(array $bindings, bool $urlEncode = false)
     {
         $literalSegments = [];
         $keySegmentTuples = self::buildKeySegmentTuples($this->segments);
@@ -115,25 +115,85 @@ public function render(array $bindings)
                 throw $this->renderingException($bindings, "missing required binding '$key' for segment '$segment'");
             }
             $value = $bindings[$key];
-            if (!is_null($value) && $segment->matches($value)) {
-                $literalSegments[] = new Segment(
-                    Segment::LITERAL_SEGMENT,
-                    $value,
-                    $segment->getValue(),
-                    $segment->getTemplate(),
-                    $segment->getSeparator()
+            if (is_null($value)) {
+                throw $this->renderingException(
+                    $bindings,
+                    "expected binding '$key' to match segment '$segment', instead got null"
                 );
-            } else {
-                $valueString = is_null($value) ? 'null' : "'$value'";
+            }
+
+            if (!$this->matchAndValidateSegment($segment, (string)$value, (string)$key)) {
                 throw $this->renderingException(
                     $bindings,
-                    "expected binding '$key' to match segment '$segment', instead got $valueString"
+                    "expected binding '$key' to match segment '$segment', instead got '$value'"
                 );
             }
+
+            $encodedValue = $urlEncode ? self::encodeValue($value) : $value;
+            $literalSegments[] = new Segment(
+                Segment::LITERAL_SEGMENT,
+                $encodedValue,
+                $segment->getValue(),
+                $segment->getTemplate(),
+                $segment->getSeparator()
+            );
         }
         return self::renderSegments($literalSegments);
     }
 
+    private function matchAndValidateSegment(Segment $segment, string $value, string $key): bool
+    {
+        if ($segment->getSegmentType() === Segment::VARIABLE_SEGMENT) {
+            try {
+                $wildcardBindings = $segment->getTemplate()->match($value);
+
+                // Validate wildcard bindings for . and ..
+                $innerTuples = self::buildKeySegmentTuples($segment->getTemplate()->segments);
+                foreach ($innerTuples as list($innerKey, $innerSegment)) {
+                    if ($innerKey === null || !isset($wildcardBindings[$innerKey])) {
+                        continue;
+                    }
+                    /** @var Segment $innerSegment */
+                    $wildcardValue = $wildcardBindings[$innerKey];
+                    self::validateDotSegments($innerSegment->getSegmentType(), $wildcardValue, $key);
+                }
+                return true;
+            } catch (ValidationException $e) {
+                return false;
+            }
+        }
+
+        $matches = $segment->matches($value);
+        if ($matches) {
+            self::validateDotSegments($segment->getSegmentType(), $value, $key);
+        }
+
+        return $matches;
+    }
+
+    private static function validateDotSegments(int $segmentType, string $value, string $key): void
+    {
+        if ($segmentType === Segment::WILDCARD_SEGMENT) {
+            if ($value === '.' || $value === '..') {
+                throw new \InvalidArgumentException(sprintf(
+                    'Invalid value %s for %s.',
+                    $value,
+                    $key
+                ));
+            }
+        } elseif ($segmentType === Segment::DOUBLE_WILDCARD_SEGMENT) {
+            $parts = explode('/', $value);
+            foreach ($parts as $part) {
+                if ($part === '.' || $part === '..') {
+                    throw new \InvalidArgumentException(sprintf(
+                        'Value for %s must not contain segments that are exactly . or .. .',
+                        $key
+                    ));
+                }
+            }
+        }
+    }
+
     /**
      * @inheritdoc
      */
@@ -390,4 +450,16 @@ private static function renderSegments(array $segmentsToRender)
         }
         return $renderResult;
     }
+
+    /**
+     * URL encode the value, while preserving '/' and any characters in [-_.~0-9a-zA-Z].
+     * @param string $value
+     * @return string
+     */
+    private static function encodeValue(string $value)
+    {
+        $segments = explode('/', $value);
+        $encodedSegments = array_map('rawurlencode', $segments);
+        return implode('/', $encodedSegments);
+    }
 }

Gax/tests/Unit/PathTemplateTest.php

@@ -163,9 +163,10 @@ public function testMatchColonInWildcardAndTemplate()
     {
         $template = new PathTemplate('/buckets/*/*/*/objects/*:action');
         $url = $template->render(
-            ['$0' => 'f', '$1' => 'o', '$2' => 'o', '$3' => 'google.com:a-b']
+            ['$0' => 'f', '$1' => 'o', '$2' => 'o', '$3' => 'google.com:a-b'],
+            true
         );
-        $this->assertEquals($url, '/buckets/f/o/o/objects/google.com:a-b:action');
+        $this->assertEquals($url, '/buckets/f/o/o/objects/google.com%3Aa-b:action');
     }
 
     public function testMatchUnboundedWildcardWithColon()
@@ -206,9 +207,10 @@ public function testRenderAtomicResource()
     {
         $template = new PathTemplate('buckets/*/*/*/objects/*');
         $url = $template->render(
-            ['$0' => 'f', '$1' => 'o', '$2' => 'o', '$3' => 'google.com:a-b']
+            ['$0' => 'f', '$1' => 'o', '$2' => 'o', '$3' => 'google.com:a-b'],
+            true
         );
-        $this->assertEquals($url, 'buckets/f/o/o/objects/google.com:a-b');
+        $this->assertEquals($url, 'buckets/f/o/o/objects/google.com%3Aa-b');
     }
 
     public function testRenderFailWhenTooFewVariables()
@@ -254,19 +256,21 @@ public function testSubstitutionOddChars()
     {
         $template = new PathTemplate('projects/{project}/topics/{topic}');
         $url = $template->render(
-            ['project' => 'google.com:proj-test', 'topic' => 'some-topic']
+            ['project' => 'google.com:proj-test', 'topic' => 'some-topic'],
+            true
         );
         $this->assertEquals(
             $url,
-            'projects/google.com:proj-test/topics/some-topic'
+            'projects/google.com%3Aproj-test/topics/some-topic'
         );
         $template = new PathTemplate('projects/{project}/topics/{topic}');
         $url = $template->render(
-            ['project' => 'g.,;:~`!@#$%^&()+-', 'topic' => 'sdf<>,.?[]']
+            ['project' => 'g.,;:~`!@#$%^&()+-', 'topic' => 'sdf<>,.?[]'],
+            true
         );
         $this->assertEquals(
             $url,
-            'projects/g.,;:~`!@#$%^&()+-/topics/sdf<>,.?[]'
+            'projects/g.%2C%3B%3A~%60%21%40%23%24%25%5E%26%28%29%2B-/topics/sdf%3C%3E%2C.%3F%5B%5D'
         );
     }
 }

Gax/tests/Unit/ResourceTemplate/AbsoluteResourceTemplateTest.php

@@ -165,8 +165,8 @@ public function matchData()
             ],
             [
                 '/buckets/*/*/*/objects/*:action',
-                '/buckets/f/o/o/objects/google.com:a-b:action',
-                ['$0' => 'f', '$1' => 'o', '$2' => 'o', '$3' => 'google.com:a-b'],
+                '/buckets/f/o/o/objects/google.com-a-b:action',
+                ['$0' => 'f', '$1' => 'o', '$2' => 'o', '$3' => 'google.com-a-b'],
             ],
             [
                 '/buckets/*/objects/**:action',
@@ -180,8 +180,8 @@ public function matchData()
             ],
             [
                 '/buckets/*',
-                '/buckets/{}!@#$%^&*()+=[]\|`~-_',
-                ['$0' => '{}!@#$%^&*()+=[]\|`~-_'],
+                '/buckets/abc~-_',
+                ['$0' => 'abc~-_'],
             ],
         ];
     }

Gax/tests/Unit/ResourceTemplate/RelativeResourceTemplateTest.php

@@ -186,8 +186,8 @@ public function matchData()
             ],
             [
                 'buckets/*/*/*/objects/*',
-                'buckets/f/o/o/objects/google.com:a-b',
-                ['$0' => 'f', '$1' => 'o', '$2' => 'o', '$3' => 'google.com:a-b'],
+                'buckets/f/o/o/objects/google.com-a-b',
+                ['$0' => 'f', '$1' => 'o', '$2' => 'o', '$3' => 'google.com-a-b'],
             ],
             [
                 'buckets/*/objects/**',
@@ -201,8 +201,8 @@ public function matchData()
             ],
             [
                 'buckets/*',
-                'buckets/{}!@#$%^&*()+=[]\|`~-_',
-                ['$0' => '{}!@#$%^&*()+=[]\|`~-_'],
+                'buckets/abc~-_',
+                ['$0' => 'abc~-_'],
             ],
             [
                 'foos/{foo}_{oof}',
@@ -381,4 +381,100 @@ public function invalidRenderData()
             ],
         ];
     }
+
+    /**
+     * @param string $pathTemplate
+     * @param array $bindings
+     * @param string $expectedExceptionMessage
+     * @dataProvider invalidRenderDataInvalidArgument
+     */
+    public function testFailRenderInvalidArgument($pathTemplate, $bindings, $expectedExceptionMessage = null)
+    {
+        $this->expectException(\InvalidArgumentException::class);
+        if (isset($expectedExceptionMessage)) {
+            $this->expectExceptionMessage($expectedExceptionMessage);
+        }
+
+        $template = new RelativeResourceTemplate($pathTemplate);
+        $template->render($bindings);
+    }
+
+    public function invalidRenderDataInvalidArgument()
+    {
+        return [
+            [
+                'buckets/{hello}',
+                ['hello' => '.'],
+                "Invalid value . for hello.",
+            ],
+            [
+                'buckets/{hello}',
+                ['hello' => '..'],
+                "Invalid value .. for hello.",
+            ],
+            [
+                'buckets/{hello=*}',
+                ['hello' => '.'],
+                "Invalid value . for hello.",
+            ],
+            [
+                'buckets/{hello=**}',
+                ['hello' => 'foo/./bar'],
+                "Value for hello must not contain segments that are exactly . or .. .",
+            ],
+            [
+                'buckets/{hello=**}',
+                ['hello' => 'foo/..'],
+                "Value for hello must not contain segments that are exactly . or .. .",
+            ],
+            [
+                'buckets/*/objects/**',
+                ['$0' => '.', '$1' => 'foo/bar'],
+                "Invalid value . for $0.",
+            ],
+            [
+                'buckets/*/objects/**',
+                ['$0' => 'foo', '$1' => '../bar'],
+                "Value for $1 must not contain segments that are exactly . or .. .",
+            ],
+            [
+                'projects/*/locations/*',
+                ['$0' => 'my-proj', '$1' => '.'],
+                "Invalid value . for $1.",
+            ]
+        ];
+    }
+
+    /**
+     * @dataProvider renderEncodingData
+     */
+    public function testRenderEncoding($pathTemplate, $expectedPath, $bindings)
+    {
+        $template = new RelativeResourceTemplate($pathTemplate);
+        $this->assertEquals($expectedPath, $template->render($bindings, true));
+    }
+
+    public function renderEncodingData()
+    {
+        return [
+            [
+                'buckets/{hello}',
+                'buckets/world%20order',
+                ['hello' => 'world order'],
+            ],
+            [
+                'buckets/{hello=**}',
+                'buckets/foo/bar%21/baz~',
+                ['hello' => 'foo/bar!/baz~'],
+            ],
+            [
+                'projects/{project}/locations/{location}',
+                'projects/my%20project/locations/us-central1',
+                [
+                    'project' => 'my project',
+                    'location' => 'us-central1',
+                ]
+            ],
+        ];
+    }
 }