Skip to content

Commit 45097cc

Browse files
tjwiebelltjwiebell
authored
Merge pull request #44 from magento-research/bug/regex-resolver-matches
Fix bugs found when using venia-upward.yml:
2 parents 0e068c0 + ce318e1 commit 45097cc

12 files changed

+101
-64
lines changed

‎composer.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
"license": "OSL-3.0",
66
"require": {
77
"php": ">= 7.1",
8-
"guzzlehttp/guzzle": "^6.2",
98
"mustache/mustache": "^2.12",
9+
"ralouphie/mimey": "^2.0",
1010
"symfony/yaml": "^2.3||^3.3",
1111
"zendframework/zend-http": "^2.6",
1212
"zendframework/zend-stdlib": "^2.7"
@@ -22,6 +22,9 @@
2222
{
2323
"name": "Ben Batschelet",
2424
"email": "batschel@adobe.com"
25+
}, {
26+
"name": "Tommy Wiebell",
27+
"email": "twiebell@adobe.com"
2528
}
2629
],
2730
"autoload": {

‎package-lock.json

Lines changed: 32 additions & 26 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,6 @@
88
"test:spec": "upward-spec ./test-spec.sh"
99
},
1010
"devDependencies": {
11-
"@magento/upward-spec": "~2.0.0-rc.17"
11+
"@magento/upward-spec": "^2.0.0-rc.18"
1212
}
1313
}

‎src/DefinitionIterator.php

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,16 +85,14 @@ public function get($lookup, $definition = null)
8585
$definedValue = $definedValue->get($lookup);
8686
}
8787

88-
// Expand $lookup to full tree address so we can safely detect loops across different parts of the tree
89-
if ($definedValue instanceof Definition) {
90-
$lookup = $definedValue->getTreeAddress();
91-
}
88+
$fullLookup = empty($definition->getTreeAddress()) ? $lookup : $definition->getTreeAddress() . '.' . $lookup;
9289

93-
if (\in_array($lookup, $this->lookupStack)) {
94-
throw new \RuntimeException('Definition appears to contain a loop: ' . json_encode($this->lookupStack));
90+
if (\in_array($fullLookup, $this->lookupStack)) {
91+
$stack = array_merge($this->lookupStack, [$lookup]);
92+
throw new \RuntimeException('Definition appears to contain a loop: ' . json_encode($stack));
9593
}
9694

97-
$this->lookupStack[] = $lookup;
95+
$this->lookupStack[] = $fullLookup;
9896

9997
try {
10098
$value = $this->getFromDefinedValue($lookup, $definedValue);

‎src/Resolver/Conditional.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,9 @@ public function resolve($definition)
6464
}
6565

6666
foreach ($definition->get($this->getIndicator()) as $matcher) {
67-
$value = (string) $this->getIterator()->get($matcher->get('matches'));
68-
$pattern = '/' . addcslashes($matcher->get('pattern'), '/') . '/';
67+
$rawValue = $this->getIterator()->get($matcher->get('matches'));
68+
$value = is_scalar($rawValue) ? (string) $rawValue : json_encode($rawValue);
69+
$pattern = '/' . addcslashes($matcher->get('pattern'), '/') . '/';
6970

7071
if (preg_match($pattern, $value, $matches)) {
7172
// preface each key with '$'

‎src/Resolver/Directory.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace Magento\Upward\Resolver;
1010

1111
use Magento\Upward\Definition;
12+
use Mimey\MimeTypes;
1213
use Zend\Http\Header\ContentType;
1314
use Zend\Http\Response;
1415
use Zend\Http\Response\Stream;
@@ -53,8 +54,7 @@ public function resolve($definition)
5354
throw new \InvalidArgumentException('$definition must be an instance of ' . Definition::class);
5455
}
5556

56-
$directory = $this->getIterator()->get('directory', $definition);
57-
57+
$directory = $this->getIterator()->get('directory', $definition);
5858
$response = new Stream();
5959
$upwardRoot = $this->getIterator()->getRootDefinition()->getBasepath();
6060
$root = realpath($upwardRoot . \DIRECTORY_SEPARATOR . $directory);
@@ -64,8 +64,10 @@ public function resolve($definition)
6464
if (!$path || strpos($path, $root) !== 0 || !is_file($path)) {
6565
$response->setStatusCode(Response::STATUS_CODE_404);
6666
} else {
67+
$mimeType = (new MimeTypes())->getMimeType(pathinfo($path, PATHINFO_EXTENSION));
68+
6769
$response->setStream(fopen($path, 'r'));
68-
$response->getHeaders()->addHeader(new ContentType(mime_content_type($path)));
70+
$response->getHeaders()->addHeader(new ContentType($mimeType));
6971
}
7072

7173
return $response;

‎src/Resolver/Proxy.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ public function resolve($definition)
6060
$client = new Client(null, [
6161
'adapter' => Client\Adapter\Curl::class,
6262
'curloptions' => [
63+
CURLOPT_SSL_VERIFYHOST => $ignoreSSLErrors ? 0 : 2,
6364
CURLOPT_SSL_VERIFYPEER => !$ignoreSSLErrors,
6465
],
6566
]);

‎src/Resolver/Service.php

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,25 +49,42 @@ public function resolve($definition)
4949
throw new \InvalidArgumentException('$definition must be an instance of ' . Definition::class);
5050
}
5151

52-
$url = $this->getIterator()->get('url', $definition);
53-
$query = $this->getIterator()->get('query', $definition);
54-
$method = $definition->has('method') ? $this->getIterator()->get('method', $definition) : 'POST';
55-
$headers = $definition->has('headers') ? $this->getIterator()->get('headers', $definition) : [];
56-
$variables = $definition->has('variables') ? $this->getIterator()->get('variables', $definition) : [];
52+
$url = $this->getIterator()->get('url', $definition);
53+
$query = $this->getIterator()->get('query', $definition);
54+
$method = $definition->has('method') ? $this->getIterator()->get('method', $definition) : 'POST';
55+
$variables = $definition->has('variables') ? $this->getIterator()->get('variables', $definition) : [];
56+
$ignoreSSLErrors = $definition->has('ignoreSSLErrors')
57+
? $this->getIterator()->get('ignoreSSLErrors', $definition)
58+
: false;
5759
$requestParams = [
5860
'query' => $query,
5961
'variables' => $variables,
6062
];
6163

62-
$client = new Client($url);
64+
$client = new Client($url, [
65+
'adapter' => Client\Adapter\Curl::class,
66+
'curloptions' => [
67+
CURLOPT_SSL_VERIFYHOST => $ignoreSSLErrors ? 0 : 2,
68+
CURLOPT_SSL_VERIFYPEER => !$ignoreSSLErrors,
69+
],
70+
]);
71+
6372
$client->setMethod($method);
64-
$client->setHeaders($headers);
73+
6574
if ($method === 'POST') {
75+
$headers = $definition->has('headers')
76+
? array_merge(['Content-type' => 'application/json'], $this->getIterator()->get('headers', $definition))
77+
: ['Content-type' => 'application/json'];
78+
6679
$client->setRawBody(json_encode($requestParams));
6780
} elseif ($method === 'GET') {
81+
$headers = $definition->has('headers') ? $this->getIterator()->get('headers', $definition) : [];
82+
6883
$client->setParameterGet($requestParams);
6984
}
7085

86+
$client->setHeaders($headers);
87+
7188
$response = $client->send();
7289

7390
return json_decode($response->getBody(), true);

‎src/Resolver/Template.php

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,18 @@ public function resolve($definition)
5050
{
5151
$engineName = $definition->has('engine') ? $this->getIterator()->get('engine', $definition) : null;
5252
$templateString = $this->getIterator()->get('template', $definition);
53-
$renderData = $this->getIterator()->get('provide', $definition);
53+
$renderData = [];
5454

55-
if ($definition->get('provide')->isList()) {
56-
$renderData = array_combine($definition->get('provide')->toArray(), $renderData);
55+
if ($definition->has('provide')) {
56+
foreach ($definition->get('provide') as $index => $definition) {
57+
$key = \is_int($index) ? $definition : $index;
58+
$renderData[$key] = $definition instanceof Definition
59+
? $this->getIterator()->get('provide.' . $index)
60+
: $this->getIterator()->get($definition);
61+
}
5762
}
5863

59-
$engine = TemplateFactory::get($definition->getBasepath(), $engineName);
64+
$engine = TemplateFactory::get($this->getIterator()->getRootDefinition()->getBasepath(), $engineName);
6065

6166
return $engine->render($templateString, $renderData);
6267
}

‎test/DefinitionIteratorTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public function testDefinitionValueIsBuiltin(): void
9393
// Key has been added to context
9494
verify($context->get('key'))->is()->true();
9595

96-
verify($iterator->get('child-key', false))->is()->false();
96+
verify($iterator->get('child-key', new Definition(['child-key' => false])))->is()->false();
9797

9898
// Key was not added to context
9999
verify($context->has('child-key'))->is()->false();
@@ -157,7 +157,7 @@ public function testIteratingDefinitionTree(): void
157157
verify($context->get('key2'))->is()->true();
158158

159159
// Child definition is an address for a value in the root definition
160-
verify($iterator->get('key3', 'key4'))->is()->false();
160+
verify($iterator->get('key3', new Definition(['key3' => 'key4'])))->is()->false();
161161

162162
// Only value from root definition is added to context
163163
verify($context->has('key3'))->is()->false();

0 commit comments

Comments
 (0)