cedaro / satispress

Expose installed WordPress plugins and themes as Composer packages.
508 stars 51 forks source link

Invalid plugin version string causes error 500 #160

Closed DavidSingh3 closed 3 years ago

DavidSingh3 commented 3 years ago

Both of those situations will cause SatisPress to throw an Uncaught UnexpectedValueException error when generating the packages.json, which results in a 500 error during composer install.

The culprit seems to be /src/Transformer/ComposerRepositoryTransformer.php:L142, which contains the following code: $this->version_parser->normalize( $version )

Since this line is wrapped in a try/catch structure, and only FileNotFound are handled by the catch, this causes an Uncaught UnexpectedValueException error when the normalize method throws an UnexpectedValueException exception.

bradyvercher commented 3 years ago

Hi @DavidSingh3, this sounds reasonable and the PR looks good. Thanks for submitting it!

Before committing, do you have examples of the types of version strings that are causing issues? If it's every version for specific plugins, then I would think managing those with SatisPress wouldn't be ideal and ensuring the error is visible would be preferred.

DavidSingh3 commented 3 years ago

Hi @bradyvercher

This has been the most recent example we dealt with for our plugin repository.

image

I reproduced locally with SatisPress 1.0.1 (plus this commit: https://github.com/cedaro/satispress/commit/a5e7afb3cf027260aa0644c171ecba71cca76bbb)

Modal Survey is at version 2.0.1.2, which is a valid semver string. However, the updates for Modal Survey are handled by the Envato Market plugin, and this detects an available update of Modal Survey 2.0.1.8.6. This version string has 4 periods, which is one too many for the version_parser's normalize function, which will throw the exception.

Below is the result after applying this PR and here it is properly formatted: https://pastebin.com/dF0y21F8 image

Notice that the package for Modal Survey 2.0.1.2 is still working, but the one for 2.0.1.8.6 is skipped from packages.json.


Now I wonder why SatisPress will attempt to package and serve available plugin updates. I don't think it's intentional nor necessary. It appears to have something to do with the _site_transient_plugin_updates row from wp_options.

However, this PR also fixes a fatal error that happens similarly to the above scenario, but when the offending plugin is actually installed, and not just an available update. I've seen this with some TagDiv plugins, where the version number would start with a period (eg: .5.0), and also get rejected by the version_parser, causing the same fatal error.

bradyvercher commented 3 years ago

@DavidSingh3 Thanks for the detailed explanation. I went ahead and merged this.

Now I wonder why SatisPress will attempt to package and serve available plugin updates. I don't think it's intentional nor necessary.

I'm not quite sure what you mean here, but packaging and serving available updates is one of the major benefits of SatisPress. It allows all (valid) versions to be captured as soon as they're available in an automated way, otherwise new releases would only be available after a manual update.