diff --git a/helfi_proxy.services.yml b/helfi_proxy.services.yml index baff543..0171c80 100644 --- a/helfi_proxy.services.yml +++ b/helfi_proxy.services.yml @@ -16,7 +16,7 @@ services: tags: # Must be run before PathProcessorFront (200 weight). - { name: path_processor_inbound, priority: 201 } - # Must to be run after LanguageProcessorLanguage (100 weight). + # Must be run after LanguageProcessorLanguage (100 weight). - { name: path_processor_outbound, priority: 99 } cache_context.site_prefix: diff --git a/src/ProxyManager.php b/src/ProxyManager.php index 8d68716..083f99b 100644 --- a/src/ProxyManager.php +++ b/src/ProxyManager.php @@ -33,32 +33,6 @@ public function __construct(ConfigFactoryInterface $configFactory) { $this->config = $configFactory->get('helfi_proxy.settings'); } - /** - * Converts the given URL to relative. - * - * @param string|null $value - * The value. - * - * @return string|null - * the value. - */ - private function convertAbsoluteToRelative(?string $value) : ? string { - if (!$value) { - return $value; - } - $parts = parse_url($value); - - // Value is already relative. - if (empty($parts['host'])) { - return $value; - } - - if (isset($parts['path'])) { - return $parts['path'] . (isset($parts['query']) ? '?' . $parts['query'] : NULL); - } - return $value; - } - /** * Checks if given URL is hosted from a CDN. * @@ -95,6 +69,24 @@ private function isCdnAddress(?string $value) : bool { return FALSE; } + /** + * Checks whether the asset is hosted locally. + * + * @param string $value + * The path to given asset. + * + * @return bool + * TRUE if asset is local. + */ + private function isLocalAsset(string $value) : bool { + return match(TRUE) { + str_starts_with($value, '/sites'), + str_starts_with($value, '/core'), + str_starts_with($value, '/themes') => TRUE, + default => FALSE, + }; + } + /** * Handles tags with 'alwaysAbsolute' option. * @@ -107,12 +99,24 @@ private function isCdnAddress(?string $value) : bool { * The value. */ private function handleAlwaysAbsolute(Request $request, string $value) : ? string { - $value = $this->convertAbsoluteToRelative($value); + if (!$value) { + return $value; + } + $parts = parse_url($value); - // Skip non-relative values. - if (!str_starts_with($value, '/')) { + // Value is already relative. + if (empty($parts['host']) || empty($parts['path'])) { return $value; } + + // Skip non-local assets. + if (!$this->isLocalAsset($parts['path'])) { + return $value; + } + + if (isset($parts['path'])) { + $value = $parts['path'] . (isset($parts['query']) ? '?' . $parts['query'] : NULL); + } return sprintf('%s%s', $request->getSchemeAndHttpHost(), $this->addAssetPath($value)); } @@ -138,14 +142,16 @@ private function addAssetPath(string $value) : ? string { * The value. * @param string $separator * The separator. + * @param callable $callback + * The callback to run value through. * * @return string|null * The value. */ - private function handleMultiValue(string $value, string $separator) : ? string { + private function handleMultiValue(string $value, string $separator, callable $callback) : ? string { $parts = []; foreach (explode($separator, $value) as $item) { - $parts[] = $this->addAssetPath(trim($item)); + $parts[] = $callback(trim($item)); } return implode($separator, $parts); } @@ -176,6 +182,53 @@ private function handleSitePrefix(Request $request, string $value) : ? string { return sprintf('%s/%s', $prefix, ltrim($value, '/')); } + /** + * Checks if the given value is an image style. + * + * @param string $value + * The value. + * + * @return bool + * TRUE if image is image style. + */ + private function isImageStyle(string $value) : bool { + return str_contains($value, '/files/styles/'); + } + + /** + * Special handling for image styles. + * + * @param \Drupal\helfi_proxy\Selector\Selector $tag + * The tag. + * @param string $value + * The value with domain added to it. + * + * @return string|null + * The image style url. + */ + private function handleImageStyle(Selector $tag, string $value) : ? string { + if ($tag->multipleValues) { + return $this->handleMultiValue($value, $tag->multivalueSeparator, + function (string $value): string { + return $this->addDomain($value); + }); + } + return $this->addDomain($value); + } + + /** + * Adds domain to relative URL. + * + * @param string $value + * The value. + * + * @return string + * The value with domain added to it. + */ + private function addDomain(string $value) : string { + return sprintf('//%s/%s', $this->getHostname(), ltrim($value, '/')); + } + /** * {@inheritdoc} */ @@ -184,9 +237,14 @@ public function getAttributeValue(Request $request, Selector $tag, ?string $valu if (!$value || $this->isCdnAddress($value)) { return $value; } - // Certain elements are absolute URLs already (such as og:image:url) - // so we need to convert them to relative first and then back to - // absolute. + + // Image styles need special handling because they need to be run through + // PHP before they are uploaded to CDN. + if ($this->isImageStyle($value)) { + return $this->handleImageStyle($tag, $value); + } + // Certain elements might be absolute URLs already (such as og:image:url). + // Make sure locally hosted files are always served from correct domain. if ($tag->alwaysAbsolute) { return $this->handleAlwaysAbsolute($request, $value); } @@ -202,7 +260,11 @@ public function getAttributeValue(Request $request, Selector $tag, ?string $valu } if ($tag->multipleValues) { - return $this->handleMultiValue($value, $tag->multivalueSeparator); + return $this->handleMultiValue($value, $tag->multivalueSeparator, + function (string $value) : string { + return $this->addAssetPath($value); + } + ); } return $this->addAssetPath($value); diff --git a/src/ProxyTrait.php b/src/ProxyTrait.php index a1a23bf..e8e8da0 100644 --- a/src/ProxyTrait.php +++ b/src/ProxyTrait.php @@ -27,6 +27,7 @@ public function getHostname() : string { } $variables = [ + 'DRUPAL_BACKEND_DOMAIN', 'DRUPAL_ROUTES', 'DRUPAL_REVERSE_PROXY_ADDRESS', ]; diff --git a/src/Selector/Selector.php b/src/Selector/Selector.php index 121d83c..aa7824e 100644 --- a/src/Selector/Selector.php +++ b/src/Selector/Selector.php @@ -15,7 +15,7 @@ class Selector { * @param string $xpath * The tag. * @param string $attribute - * The atribute. + * The attribute. * @param bool $alwaysAbsolute * Whether the given value needs to be converted to relative url. * @param bool $sitePrefix diff --git a/tests/src/Kernel/ProxyManagerTest.php b/tests/src/Kernel/ProxyManagerTest.php index cc9a2b8..72417a4 100644 --- a/tests/src/Kernel/ProxyManagerTest.php +++ b/tests/src/Kernel/ProxyManagerTest.php @@ -174,9 +174,9 @@ public function testMetaTags() : void { $this->setAssetPath('test-assets'); foreach (['og:image', 'og:image:url'] as $tag) { - $this->assertEquals('http://' . $this->getHostname() . '/test-assets/path/to/og-image.png', $this->proxyManager()->getAttributeValue($request, Selectors::get($tag), 'https://www.hel.fi/path/to/og-image.png')); + $this->assertEquals('http://' . $this->getHostname() . '/test-assets/themes/hdbt/og-image.png', $this->proxyManager()->getAttributeValue($request, Selectors::get($tag), 'https://www.hel.fi/themes/hdbt/og-image.png')); } - $this->assertEquals('http://' . $this->getHostname() . '/test-assets/path/to/og-image.png', $this->proxyManager()->getAttributeValue($request, Selectors::get('twitter:image'), 'https://www.hel.fi/path/to/og-image.png')); + $this->assertEquals('http://' . $this->getHostname() . '/test-assets/themes/hdbt/og-image.png', $this->proxyManager()->getAttributeValue($request, Selectors::get('twitter:image'), 'https://www.hel.fi/themes/hdbt/og-image.png')); } /** @@ -217,7 +217,7 @@ public function testSourceSrcSet() : void { foreach ($values as $value) { $this->assertEquals( - '/test-assets' . $value, + '//' . $this->getHostname() . $value, $this->proxyManager()->getAttributeValue($request, Selectors::get('source'), $value) ); }