Repository: php-imagine/Imagine (language: php)

Issue:
ExifMetadataReader class fails due to unnecessary check for 'allow_url_fopen()'
### Issue description
A new Imagine instance can be generated by loading the content of the image file directly.
If you try and add ExifMetadataReader to an instance created this way, and the server environment has `allow_url_fopen` set to 0, the attempt fails: even though in this instance there is no need for remote access to anything to generate the image or read the metadata.

The issue is being felt on a server that has a 'locked down' security status (i.e. allow_url_fopen is set to 0, but curl is available) - even though in the specific use case no remote file access is required, if it was Curl is available and could be used. But the check carried out by ExifMetadataReader in `getUnsupportedReason()` doesn't consider either of these contexts, and throws the exception simply because of the value of `allow_url_fopen` which appears to be inappropriate.
...


### What version of Imagine are you using?

1.3.5


### What's the PHP version you are using?

8.3


### What's the imaging library you are using [gd/imagick/gmagick/any]?

gd2

### Minimal PHP code to reproduce the error: 

Where `$source_image_raw` is the content of an image file (obtained from reading a local disk file, or from remote location).

```php

$source_image = (new Imagine())->setMetadataReader(new \Imagine\Image\Metadata\ExifMetadataReader())->load($source_image_raw);
```



Discussion / hints:
The `ExifMetadataReader` uses `data://` URLs to read the data. 
This only works if `allow_url_fopen` is enabled.
You can find more details about that here: https://github.com/php-imagine/Imagine/issues/728
Well thanks for the additional information. 

Looks like this was causing problems in 2019, and is still causing problems now.

This is a pity - as this choice (to use `data://` - and this as far as I can tell is the **only** use of data:// in the entire Imagine library) prevents code that uses Imagine to read metadata from running on any secured php server (I don't have stats, but it appears that disabling `allow_url_fopen` is a standard security change for production server and has been for a long time). 

In my actual use case, suggesting to a sysadmin that they make their server less secure so that an image utility that has no actual need to contact a remote server can correct the rotation of an iPhone image (that was itself read from a local disk) seems like a request that is unlikely to be satisfied.

The use of the `data://` wrapper is solely to accommodate `exif_read_data()`'s requirement that its source be a "file". Where `allow_url_fopen` is available, using `data://` is the solution for reading image data that exists as a string value. However where `allow_url_fopen` is not available it is a poor choice, as it doesn't work - as the two issues relating to this on this library indicate.

I think the solution is, where `enable_url_fopen` is not available, to instead write the image data to a temporary file, use that to get the exif data and then delete the temporary file. I have written a demonstration of this principle working in a [PR](https://github.com/php-imagine/Imagine/pull/860) to the file ExifMetadataReader.php - but since I don't have great familiarity with how Imagine handles file I/O, and since the code's primary purpose is to demonstrate that alternatives that fix this issue can exist, I have written it using GD2 functions (which I can test to ensure the idea works) rather than provide a library independent version.

I hope the PR might encourage a more tolerant view of this issue: moving away from the view that it is some kind of "absolute" issue that cannot be addressed. Maybe also the PR might inspire someone who can write better code to fix this issue more generically.
> I don't have stats, but it appears that disabling `allow_url_fopen` is a standard security change for production server

I don’t think this is true. If a production server should not be able to make network connections, this should be prevented from the operating system side as `allow_url_fopen` is not sufficient to do that.

But I agree that if possible, this library should not require `allow_url_fopen` to be enabled.

> moving away from the view that it is some kind of "absolute" issue that cannot be addressed

I don’t see this as “absolute” in any way, this can totally be addressed I think. But I also think it would be great if this gets fixed in PHP itself as well, so maybe you could add a comment with your use case there: https://bugs.php.net/bug.php?id=47336

> I think the solution is, where enable_url_fopen is not available, to instead write the image data to a temporary file

There might be an even better solution using streams instead of `data:`-URIs. This probably also leads to a better performance. I’ll try to work on a pull request for that.
> If a production server should not be able to make network connections, this should be prevented from the operating system side as allow_url_fopen is not sufficient to do that.

I completely agree it is not the only thing that is, could, or should be done. Nonetheless it does appear to be widely used as part of steps to secure php servers.

> it would be great if this gets fixed in PHP itself as well, so maybe you could add a comment with your use case

Of course - but the issue has been open for 15 years and is cast as a documentation issue rather than a code issue, so I'm not expecting that it would lead to a useful code change any time soon.

> There might be an even better solution using streams instead of data:-URIs. This probably also leads to a better performance. I’ll try to work on a pull request for that.

That would be a very good outcome - let me know if there is anything I can do to help.
Comment added ... https://bugs.php.net/bug.php?id=47336

Fix patch:
diff --git a/src/Image/Metadata/ExifMetadataReader.php b/src/Image/Metadata/ExifMetadataReader.php
index 4f15e6bd6..b5bf3c46a 100644
--- a/src/Image/Metadata/ExifMetadataReader.php
+++ b/src/Image/Metadata/ExifMetadataReader.php
@@ -42,11 +42,14 @@ public static function getUnsupportedReason()
         if (!function_exists('exif_read_data')) {
             return 'The PHP EXIF extension is required to use the ExifMetadataReader';
         }
-        if (!in_array('data', stream_get_wrappers(), true)) {
-            return 'The data:// stream wrapper must be enabled';
-        }
-        if (in_array(ini_get('allow_url_fopen'), array('', '0', 0), true)) {
-            return 'The allow_url_fopen php.ini configuration key must be set to 1';
+
+        if (PHP_VERSION_ID < 70200) {
+            if (!in_array('data', stream_get_wrappers(), true)) {
+                return 'The data:// stream wrapper must be enabled';
+            }
+            if (in_array(ini_get('allow_url_fopen'), array('', '0', 0), true)) {
+                return 'The allow_url_fopen php.ini configuration key must be set to 1';
+            }
         }
 
         return '';
@@ -107,6 +110,14 @@ protected function extractFromStream($resource)
      */
     private function doReadData($data)
     {
+        if (PHP_VERSION_ID >= 70200) {
+            $stream = fopen('php://memory', 'r+');
+            fwrite($stream, $data);
+            rewind($stream);
+
+            return $this->extract($stream);
+        }
+
         if (substr($data, 0, 2) === 'II') {
             $mime = 'image/tiff';
         } else {
@@ -117,9 +128,9 @@ private function doReadData($data)
     }
 
     /**
-     * Performs the exif data extraction given a path or data-URI representation.
+     * Performs the exif data extraction given a path, data-URI representation or stream.
      *
-     * @param string $path the path to the file or the data-URI representation
+     * @param string|resource $path the path to the file, the data-URI representation or a stream
      *
      * @return array
      */


Test patch: