diff --git a/Gax/src/PathTemplate.php b/Gax/src/PathTemplate.php index 4556acf077b..464f32dc1ff 100644 --- a/Gax/src/PathTemplate.php +++ b/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); } /** diff --git a/Gax/src/RequestBuilder.php b/Gax/src/RequestBuilder.php index 330d13dd423..24af4c050d0 100644 --- a/Gax/src/RequestBuilder.php +++ b/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; } diff --git a/Gax/src/ResourceTemplate/AbsoluteResourceTemplate.php b/Gax/src/ResourceTemplate/AbsoluteResourceTemplate.php index 7f2425f2eaf..de08c38c7d5 100644 --- a/Gax/src/ResourceTemplate/AbsoluteResourceTemplate.php +++ b/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()); } /** diff --git a/Gax/src/ResourceTemplate/RelativeResourceTemplate.php b/Gax/src/ResourceTemplate/RelativeResourceTemplate.php index 47ca7dac262..cefc3b8acf2 100644 --- a/Gax/src/ResourceTemplate/RelativeResourceTemplate.php +++ b/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); + } } diff --git a/Gax/tests/Unit/PathTemplateTest.php b/Gax/tests/Unit/PathTemplateTest.php index a53c89e1b98..f994374e4fe 100644 --- a/Gax/tests/Unit/PathTemplateTest.php +++ b/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' ); } } diff --git a/Gax/tests/Unit/ResourceTemplate/AbsoluteResourceTemplateTest.php b/Gax/tests/Unit/ResourceTemplate/AbsoluteResourceTemplateTest.php index 65c84ffa4b9..8c8879042b9 100644 --- a/Gax/tests/Unit/ResourceTemplate/AbsoluteResourceTemplateTest.php +++ b/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~-_'], ], ]; } diff --git a/Gax/tests/Unit/ResourceTemplate/RelativeResourceTemplateTest.php b/Gax/tests/Unit/ResourceTemplate/RelativeResourceTemplateTest.php index 6e95f663a76..3a84b1fffd2 100644 --- a/Gax/tests/Unit/ResourceTemplate/RelativeResourceTemplateTest.php +++ b/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', + ] + ], + ]; + } }