Modx 3.0.4-pl issue with image upload

[prbt2016]

Bug report

Summary

Quick summary what's this issue about.

I was in the process of manual installation of Modx 3.0.4-pl on PHP 7.4, MYSQL 5.6, Apache 2.2

However , after uploading image , it is been shown as a placeholder and not actual image , while fresh installation.

I had an old installation of Modx 3.0.3-pl and also while upgrading from 3.0.3-pl which has some images and post upgrading , they too are shown as placeholders.

Could you please replicate this at your end and fix this issue?.

Normal view :

Detail view :

Step to reproduce

Go to media -> media browser , and upload an image.

Image shown as placeholder as shown in above screenshot.

Observed behavior

Image is shown as placeholder.

Expected behavior

Actual image should be shown.

Environment

MODX version - 3.0.4-pl Apache - 2.2.34 MYSQL - 5.6.46 PHP Version - 7.4.27 Browser - Mozilla Firefox 115.3.1esr (64-bit)

For your kind information following is the error in Apache logs :

[{{DATE}}] [error] [client {{IP-ADDRESS}}] PHP Fatal error:  Uncaught TypeError: Argument 2 passed to phpthumb::__set() must be an instance of mixed, bool given in /{{PATH}}/{{TO}}/{{MODX}}/core/vendor/james-heinrich/phpthumb/phpthumb.class.php:317\nStack trace:\n#0 /{{PATH}}/{{TO}}/{{MODX}}/core/vendor/james-heinrich/phpthumb/phpthumb.class.php(395): phpthumb->__set('cache_source_en...', false)\n#1 /{{PATH}}/{{TO}}/{{MODX}}/core/src/Revolution/modPhpThumb.php(100): phpthumb->setParameter('cache_source_en...', false)\n#2 /{{PATH}}/{{TO}}/{{MODX}}/core/src/Revolution/Processors/System/PhpThumb.php(137): MODX\\Revolution\\modPhpThumb->initialize()\n#3 /{{PATH}}/{{TO}}/{{MODX}}/core/src/Revolution/Processors/System/PhpThumb.php(84): MODX\\Revolution\\Processors\\System\\PhpThumb->loadPhpThumb()\n#4 /{{PATH}}/{{TO}}/{{MODX}}/core/src/Revolution/Processors/Processor.php(189): MODX\\Revolution\\Processors\\System\\PhpThumb->process()\n#5 /{{PATH}}/{{TO}}/{{MODX}}/core/src/Revolution/modX.php(177 in /{{PATH}}/{{TO}}/{{MODX}}/core/vendor/james-heinrich/phpthumb/phpthumb.class.php on line 317, referer: http://{{DOMAIN.COM}}/{{MODX-FOLDER}}/manager/?a=media/browser

[MaticSulc]

Possibly related to #16468? Try updating your PHP to 8.x.

[prbt2016]

Hello ,

I cannot upgrade my PHP due to certain restrictions.

Is it possible that a backward compatibility fix be added for the same for PHP 7.4 which will be great as users working with PHP 7.4 won't face an issue?

Kindly let me know.

[MaticSulc]

There's a solution in the issue I linked: https://github.com/modxcms/revolution/issues/16468#issuecomment-1708335830

[halftrainedharry]

In the file core/vendor/james-heinrich/phpthumb/phpthumb.class.php, remove the word mixed on line 317 to fix the issue.

[opengeek]

@halftrainedharry — did you report this in the phpthumb repository? If this does not get fixed upstream, there's not a lot we can do about it.

[halftrainedharry]

did you report this in the phpthumb repository?

I did create a pull request 3 days ago. https://github.com/JamesHeinrich/phpThumb/pull/218

If this does not get fixed upstream, there's not a lot we can do about it.

I looked at the code (of MODX 2.x) and the problem occurs here in the MODX code.

https://github.com/modxcms/revolution/blob/3f887dc6494fe32d3b2a52da20ec76a63bc8fb7d/core/model/phpthumb/modphpthumb.class.php#L84-L89

where a few properties are set that don't actually exist.

For example: The code tries to set the property cache_source_enabled, but the property is actually called config_cache_source_enabled (with the prefix config_).

https://github.com/modxcms/revolution/blob/3f887dc6494fe32d3b2a52da20ec76a63bc8fb7d/core/model/phpthumb/phpthumb.class.php#L123

Do you have an idea, why the MODX code sets parameters that don't exist in phpThumb (and are named differently for at least 10 years)? I guess the problem could actually be fixed in MODX, but I have no idea why the current MODX code is the way it is (and changing it may create new problems).

[opengeek]

did you report this in the phpthumb repository?

I did create a pull request 3 days ago. JamesHeinrich/phpThumb#218

Oh, great! I was searching issues instead of PRs and missed it!

I looked at the code (of MODX 2.x) and the problem occurs here in the MODX code.

https://github.com/modxcms/revolution/blob/3f887dc6494fe32d3b2a52da20ec76a63bc8fb7d/core/model/phpthumb/modphpthumb.class.php#L84-L89

where a few properties are set that don't actually exist.

For example: The code tries to set the property cache_source_enabled, but the property is actually called config_cache_source_enabled (with the prefix config_).

https://github.com/modxcms/revolution/blob/3f887dc6494fe32d3b2a52da20ec76a63bc8fb7d/core/model/phpthumb/phpthumb.class.php#L123

Do you have an idea, why the MODX code sets parameters that don't exist in phpThumb (and are named differently for at least 10 years)? I guess the problem could actually be fixed in MODX, but I have no idea why the current MODX code is the way it is (and changing it may create new problems).

I certainly do not remember, if I ever knew.

[sonicpunk]

We also just discovered this issue and manually patched the file as described here https://github.com/JamesHeinrich/phpThumb/pull/218

And we also noted that the variable names in the modphpthumb.class.php are wrongly named, as mentioned above.

[smg6511]

I believe this can be closed, as:

1. The main issue was resolved with the vendor's phpthumb Dec 2023 release
2. The incorrectly-named config params should be either fixed or even removed (doing so is trivial though). The reason those have not caused any problems in a decade is because the member methods that utilize the cache_source config params (namely setSourceData) are used by neither the core modPhpThumb class, nor the Extras that are phpthumb-related.