From 32386c9e9f4bd0e63b23efe0a02e6f4cdf151ccc Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Mon, 2 Dec 2024 16:25:59 -0400 Subject: [PATCH 1/8] Rework away from `LocalReadOnlyStream`. "Local" storage has some other expectations that we do not want to deal with: - the idea of a local managed directory - being `realpath()`-able --- src/StreamWrapper/Foxml.php | 158 ++++++++++++++++++++++++++++++++---- 1 file changed, 143 insertions(+), 15 deletions(-) diff --git a/src/StreamWrapper/Foxml.php b/src/StreamWrapper/Foxml.php index 5eecf75..d6c6f77 100644 --- a/src/StreamWrapper/Foxml.php +++ b/src/StreamWrapper/Foxml.php @@ -3,7 +3,8 @@ namespace Drupal\foxml\StreamWrapper; use Drupal\Core\File\FileSystem; -use Drupal\Core\StreamWrapper\LocalReadOnlyStream; +use Drupal\Core\StreamWrapper\ReadOnlyStream; +use Drupal\Core\StreamWrapper\StreamWrapperInterface; use Drupal\Core\Url; use Drupal\foxml\Utility\Fedora3\DatastreamLowLevelAdapterInterface; @@ -12,7 +13,7 @@ /** * FOXML stream wrapper. */ -class Foxml extends LocalReadOnlyStream { +class Foxml extends ReadOnlyStream { use NotWritableTrait; @@ -53,14 +54,7 @@ public function __construct() { } /** - * {@inheritdoc} - */ - public function getDirectoryPath() { - throw new \Exception('Overides ::getLocalPath(), so this is not necessary.'); - } - - /** - * {@inheritdoc} + * {@inheritDoc} */ protected function getLocalPath($uri = NULL) { // XXX: Adapted from LocalReadOnlyStream::getLocalPath(). @@ -110,8 +104,7 @@ protected function getLocalPath($uri = NULL) { public function getExternalUrl() { // XXX: Copypasta from // \Drupal\Core\StreamWrapper\PrivateStream::getExternalUrl(). - $path = str_replace('\\', '/', $this - ->getTarget()); + $path = str_replace('\\', '/', $this->getTarget()); return Url::fromRoute('foxml.download', [ 'filepath' => $path, ], [ @@ -121,6 +114,32 @@ public function getExternalUrl() { ->toString(); } + /** + * Returns the local target of the resource within the stream. + * + * This function should be used in place of calls to realpath() or similar + * functions when attempting to determine the location of a file. While + * functions like realpath() may return the location of a read-only file, this + * method may return a URI or path suitable for writing that is completely + * separate from the URI used for reading. + * + * @param null|string $uri + * Optional URI. + * + * @return string + * Returns a string representing a location suitable for writing of a file. + */ + protected function getTarget(?string $uri = NULL) : string { + if (!isset($uri)) { + $uri = $this->uri; + } + + [, $target] = explode('://', $uri, 2); + + // Remove erroneous leading or trailing, forward-slashes and backslashes. + return trim($target, '\/'); + } + /** * {@inheritdoc} */ @@ -139,15 +158,124 @@ public function getDescription() { * {@inheritDoc} * phpcs:disable Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps */ - public function url_stat($uri, $flags) { - return static::applyMask(parent::url_stat($uri, $flags)); + public function url_stat($path, $flags) { + return ($flags & STREAM_URL_STAT_QUIET) ? + @static::applyMask(stat($this->getLocalPath($path))) : + static::applyMask(stat($this->getLocalPath($path))); } /** * {@inheritDoc} */ public function stream_stat() { - return static::applyMask(parent::stream_stat()); + return static::applyMask(fstat($this->handle)); + } + + /** + * {@inheritDoc} + */ + public function dir_closedir() { + trigger_error(__FUNCTION__ . '() not supported for foxml stream wrapper.', E_USER_WARNING); + return TRUE; + } + + /** + * {@inheritDoc} + */ + public function dir_opendir($path, $options) { + trigger_error(__FUNCTION__ . '() not supported for foxml stream wrapper.', E_USER_WARNING); + return FALSE; + } + + /** + * {@inheritDoc} + */ + public function dir_readdir() { + trigger_error(__FUNCTION__ . '() not supported for foxml stream wrapper.', E_USER_WARNING); + return FALSE; + } + + /** + * {@inheritDoc} + */ + public function dir_rewinddir() { + trigger_error(__FUNCTION__ . '() not supported for foxml stream wrapper.', E_USER_WARNING); + return FALSE; + } + + /** + * {@inheritDoc} + */ + public function stream_cast($cast_as) { + trigger_error(__FUNCTION__ . '() not supported for foxml stream wrapper.', E_USER_WARNING); + return FALSE; + } + + /** + * {@inheritDoc} + */ + public function stream_close() { + $result = fclose($this->handle); + unset($this->handle); + return $result; + } + + /** + * {@inheritDoc} + */ + public function stream_eof() { + return feof($this->handle); + } + + /** + * {@inheritDoc} + */ + public function stream_read($count) { + return fread($this->handle, $count); + } + + /** + * {@inheritDoc} + */ + public function stream_seek($offset, $whence = SEEK_SET) { + return fseek($this->handle, $offset, $whence); + } + + /** + * {@inheritDoc} + */ + public function stream_set_option($option, $arg1, $arg2) { + return stream_context_set_option($this->handle, $option, $arg1, $arg2); + } + + /** + * {@inheritDoc} + */ + public function stream_tell() { + return ftell($this->handle); + } + + /** + * {@inheritDoc} + */ + public static function getType() { + return StreamWrapperInterface::HIDDEN; + } + + /** + * {@inheritDoc} + */ + public function realpath() { + trigger_error(__FUNCTION__ . '() not supported for foxml stream wrapper.', E_USER_WARNING); + return FALSE; + } + + /** + * {@inheritDoc} + */ + public function dirname($uri = NULL) { + trigger_error(__FUNCTION__ . '() not supported for foxml stream wrapper.', E_USER_WARNING); + return FALSE; } // phpcs:enable Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps } From 34d3e468493a33b63ba346c4dc4454320be5164a Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Mon, 2 Dec 2024 16:39:06 -0400 Subject: [PATCH 2/8] Adjust range of things under sniff exclusion. --- src/StreamWrapper/Foxml.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/StreamWrapper/Foxml.php b/src/StreamWrapper/Foxml.php index d6c6f77..01d72bc 100644 --- a/src/StreamWrapper/Foxml.php +++ b/src/StreamWrapper/Foxml.php @@ -253,7 +253,7 @@ public function stream_set_option($option, $arg1, $arg2) { */ public function stream_tell() { return ftell($this->handle); - } + } // phpcs:enable Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps /** * {@inheritDoc} @@ -276,6 +276,6 @@ public function realpath() { public function dirname($uri = NULL) { trigger_error(__FUNCTION__ . '() not supported for foxml stream wrapper.', E_USER_WARNING); return FALSE; - } // phpcs:enable Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps + } } From 0a0de6fcf7aa3ae406985b584c79f8c100e21a2c Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Mon, 2 Dec 2024 16:52:04 -0400 Subject: [PATCH 3/8] Move error over to logger. --- src/StreamWrapper/Foxml.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/StreamWrapper/Foxml.php b/src/StreamWrapper/Foxml.php index 01d72bc..26829f7 100644 --- a/src/StreamWrapper/Foxml.php +++ b/src/StreamWrapper/Foxml.php @@ -9,6 +9,7 @@ use Drupal\foxml\Utility\Fedora3\DatastreamLowLevelAdapterInterface; use Drupal\foxml\Utility\Fedora3\ObjectLowLevelAdapterInterface; +use Psr\Log\LoggerInterface; /** * FOXML stream wrapper. @@ -38,6 +39,13 @@ class Foxml extends ReadOnlyStream { */ protected FileSystem $fileSystem; + /** + * Logging channel. + * + * @var \Psr\Log\LoggerInterface + */ + protected LoggerInterface $logger; + /** * Constructor. * @@ -50,6 +58,7 @@ public function __construct() { $this->datastreamAdapter = \Drupal::service('foxml.parser.datastream_lowlevel_storage'); $this->objectAdapter = \Drupal::service('foxml.parser.object_lowlevel_storage'); $this->fileSystem = \Drupal::service('file_system'); + $this->logger = \Drupal::service('logger.channel.foxml'); // phpcs:enable } @@ -76,7 +85,11 @@ protected function getLocalPath($uri = NULL) { assert(is_string($path), 'Dereferenced path.'); } catch (\Exception $e) { - trigger_error('Failed to dereference URI.', E_USER_WARNING); + $this->logger->warning('Failed to dereference URI "{uri}". Exception info: {message}, {trace}', [ + 'uri' => $uri, + 'message' => $e->getMessage(), + 'trace' => $e->getTraceAsString(), + ]); return FALSE; } From 36466454b583b76242e332c063218dfb3935b2dd Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Mon, 2 Dec 2024 16:59:06 -0400 Subject: [PATCH 4/8] Adjust comment to point at implementation from which it was derived. --- src/StreamWrapper/Foxml.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/StreamWrapper/Foxml.php b/src/StreamWrapper/Foxml.php index 26829f7..a7f30af 100644 --- a/src/StreamWrapper/Foxml.php +++ b/src/StreamWrapper/Foxml.php @@ -128,19 +128,19 @@ public function getExternalUrl() { } /** - * Returns the local target of the resource within the stream. + * Returns the target of the resource within the stream. * - * This function should be used in place of calls to realpath() or similar - * functions when attempting to determine the location of a file. While - * functions like realpath() may return the location of a read-only file, this - * method may return a URI or path suitable for writing that is completely - * separate from the URI used for reading. + * XXX: Effectively copypasta from + * \Drupal\Core\StreamWrapper\LocalStream::getTarget(), to facilitate parsing + * of the URI. * * @param null|string $uri * Optional URI. * * @return string * Returns a string representing a location suitable for writing of a file. + * + * @see \Drupal\Core\StreamWrapper\LocalStream::getTarget() */ protected function getTarget(?string $uri = NULL) : string { if (!isset($uri)) { From 79f48a420b794507f6692175e9d105da795b4233 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Mon, 2 Dec 2024 17:09:11 -0400 Subject: [PATCH 5/8] Saner property usage. --- src/StreamWrapper/Foxml.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/StreamWrapper/Foxml.php b/src/StreamWrapper/Foxml.php index a7f30af..68030e5 100644 --- a/src/StreamWrapper/Foxml.php +++ b/src/StreamWrapper/Foxml.php @@ -6,7 +6,6 @@ use Drupal\Core\StreamWrapper\ReadOnlyStream; use Drupal\Core\StreamWrapper\StreamWrapperInterface; use Drupal\Core\Url; - use Drupal\foxml\Utility\Fedora3\DatastreamLowLevelAdapterInterface; use Drupal\foxml\Utility\Fedora3\ObjectLowLevelAdapterInterface; use Psr\Log\LoggerInterface; @@ -78,10 +77,18 @@ protected function getLocalPath($uri = NULL) { return FALSE; } [$subtype, $target_actual] = $exploded; + // XXX: If you see this assert() complaining about "styles", you need some + // form of image style remapping, to have image style derivatives to be + // created under a different, writeable scheme. assert(in_array($subtype, ['object', 'datastream']), 'Valid URI.'); try { - $path = $this->{"{$subtype}Adapter"}->dereference($target_actual); + /** @var \Drupal\foxml\Utility\Fedora3\LowLevelAdapterInterface $adapter */ + $adapter = match($subtype) { + 'object' => $this->objectAdapter, + 'datastream' => $this->datastreamAdapter, + }; + $path = $adapter->dereference($target_actual); assert(is_string($path), 'Dereferenced path.'); } catch (\Exception $e) { From 00442424bc0d9813c9fdbd935d73e6383e0c9448 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Mon, 2 Dec 2024 17:17:23 -0400 Subject: [PATCH 6/8] Drop mention of "writing". Given things are read-only, didn't really make sense. Hurray, copypasta! --- src/StreamWrapper/Foxml.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/StreamWrapper/Foxml.php b/src/StreamWrapper/Foxml.php index 68030e5..d59b4ff 100644 --- a/src/StreamWrapper/Foxml.php +++ b/src/StreamWrapper/Foxml.php @@ -145,7 +145,7 @@ public function getExternalUrl() { * Optional URI. * * @return string - * Returns a string representing a location suitable for writing of a file. + * The coordinates of the resource. * * @see \Drupal\Core\StreamWrapper\LocalStream::getTarget() */ From 876bb84cfb0025ed1860848ce3d20ee3f8fafcf5 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 3 Dec 2024 09:53:34 -0400 Subject: [PATCH 7/8] Drop `trigger_error()` inside `::realpath()` method. --- src/StreamWrapper/Foxml.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/StreamWrapper/Foxml.php b/src/StreamWrapper/Foxml.php index d59b4ff..ecdf0fe 100644 --- a/src/StreamWrapper/Foxml.php +++ b/src/StreamWrapper/Foxml.php @@ -286,7 +286,8 @@ public static function getType() { * {@inheritDoc} */ public function realpath() { - trigger_error(__FUNCTION__ . '() not supported for foxml stream wrapper.', E_USER_WARNING); + // XXX: Avoiding spamming via trigger_error() here, as we expect this to be + // called from Drupal. return FALSE; } From f4637315b9e231213aa4fdce8e724e915c0a2cb8 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 3 Dec 2024 10:09:14 -0400 Subject: [PATCH 8/8] Avoid some log spam. --- src/StreamWrapper/Foxml.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/StreamWrapper/Foxml.php b/src/StreamWrapper/Foxml.php index ecdf0fe..3181c4c 100644 --- a/src/StreamWrapper/Foxml.php +++ b/src/StreamWrapper/Foxml.php @@ -287,7 +287,8 @@ public static function getType() { */ public function realpath() { // XXX: Avoiding spamming via trigger_error() here, as we expect this to be - // called from Drupal. + // called legitimately, such as from: + // - https://git.drupalcode.org/project/imagemagick/-/blob/218b8bb59225232eb502862c202a7a3849cf0e6e/src/EventSubscriber/ImagemagickEventSubscriber.php#L136 return FALSE; } @@ -295,7 +296,9 @@ public function realpath() { * {@inheritDoc} */ public function dirname($uri = NULL) { - trigger_error(__FUNCTION__ . '() not supported for foxml stream wrapper.', E_USER_WARNING); + // XXX: Avoiding spamming via trigger_error() here, as we expect this to be + // called legitimately, such as from: + // - https://github.com/discoverygarden/dgi_migrate/blob/a93b0ce642db6f0ecdde4d184a501c280fb9d2ed/src/Plugin/migrate/process/EnsureNonWritableTrait.php#L52-L56 return FALSE; }