Bug with ClassicPress Automatic update filters?

Support
1

We develop the Shield Security plugin and we’ve been offering support for ClassicPress for a while now and we recently got contacted by a customer who is seeing some inconsistencies with our display of Automatic Updates for WP/CP Core.

Our plugin handles the switch between WP/CP automatically, so this was odd. So we went digging and have an issue with the way CP handles automatic updates.

I’ve done some searching, and it seems that the general consensus has been to honour the pre-existing WP filters and hooks for Background Updates. This simplifies things greatly. If this is the case, then I’ve found some rather significant bugs with the implementation. If it’s not the case, you can completely ignore me.

To understand this, we need to clarify what we mean by “major” and “minor” updates. WordPress’ definitions are as follows:

‘Major’ - this is any update where the 2nd digit in the version string changes. i.e 1.0 → 1.1; 1.0 → 1.2
‘Minor’ - this is any update where the 3rd digit in the version string changes: i.e. 1.0.1 → 1.0.2; 5.3->5.3.1

To reflect these, WP provides 2x filters:
major: allow_major_auto_core_updates
minor: allow_minor_auto_core_updates

This doesn’t quite align with semantic versioning as used by many developers, but that’s just the way it currently is.

And it seems that the development of CP automatic updates has adopted this approach. Because the code clearly uses the WP filters in different places.

For example, the CP update code uses the WordPress “major” filter to modify behaviour if upgrade is a “semantically major” version, not a “WordPress major version”.

In CP, the WordPress “minor” filter modifies behaviour for upgrades that are “semantically minor”, but that should be “WordPress major”.

And then the CP developer has introduced a new filter called “allow_patch_auto_core_updates” to account for what were essentially “WordPress minor” upgrades.

Again, if the behaviour I’ve outlined here is fully intended, this is not a bug, of course. But what it basically does is break compatibility with any other plugin or implementation that uses the previously standard WordPress filters. Is there any definitive documentation somewhere around these adjustments of hooks and the new hooks? I can adjust Shield to compensate, but I’d like to do that on the basis of some sort of official doc on it.

What might something to consider in the future for a case like this where CP reuses the WP API but changes its behaviour, is to introduce new ClassicPress filters and then tie-in the older, WordPress filters to modify behaviour based on CP’s functionality.

Related bug:

This issue is because of CP’s new implementation of automatic updates checking that doesn’t take into account how WordPress was previously testing using strings like “‘.1.next.minor’”

6 Likes
2

Thanks for posting this, @Paul – your follow-up is top-notch!

3 Likes
3

This isn’t quite correct. The first and second digit together comprise the major version. So if either is changed, that is a major update. And the first can be more than one digit.

And the 3rd digit can be more than one digit (4.9.13).

+100 on this.

4

CP uses semver; I’m sure you know how that works, but since I don’t think this is documented anywhere else I’ll go through it here for reference.

If we change or add something that breaks backward compatibility we bump the major version number. Unlike WP, a major version update will never happen automatically; allow_major_auto_core_updates allows you to force major updates to install automatically, but core doesn’t. Use with caution.

New features go into minor releases. It’s the responsibility of plugins and themes to check for features they’re using, so there should be no breakage there. By default minor versions will be installed automatically, and allow_minor_auto_core_updates lets you prevent that.

Bug fixes go into patch releases. Unlike WP these will only ever be bug fixes, not new features, so they should always be safe to apply automatically. Again, you can prevent this if you really want to - allow_patch_auto_core_updates - but it’s almost certainly a really bad idea.

You say this breaks compatibility - could you give an example? In any practical use I don’t see how it can, but maybe we missed something.

1 Like
5

No, CP follows semver: only the first number is the major version number.

6

Right, but he was describing WordPress.

7

Sorry, it wasn’t clear to me that you were still talking about WP not CP. Nothing to see here, move along, move along…

8

Thanks for taking the time to lay this out for having a definitive reference on it, before docs are made available, I appreciate that.

What you have stated corresponds with the CP “should update?” code I’ve read.

The breakages comes with the understanding that was established with WP where .X releases (patches for CP / minor for WP) were understood to be security and patch releases, while everything else is “major”. You could say that WP just skipped the “CP minor” altogether, insofar as the semantic versioning principle goes, but, current convention is that the WP patch releases are notated using “minor” language.

This means that CP admin still appear to operate under that existing paradigm and the use of the WP filters is misaligned. When WP says “minor” they really mean “patch” (as far as CP goes), so their filter names actually mean something different, but they’ve been adopted into the CP paradigm. And this creates confusion.

Now that we have a definitive statement on the automatic update structure for CP, we can work around with it and it’s a non-issue, as far as understanding goes.

But I think if CP is going to re-appropriate WP API components and change their meaning, there needs to be docs to outline it, at least for devs. But ideally, as I mentioned, it’s far better instead to create new CP hooks/API components for such changes, and integrate the legacy WP API into these new hooks, as appropriate.

Completely aside from all of this, I personally question the legitimacy of defaulting to automatic CP minor upgrades. But that’s just my opinion and has no bearing on the core topic of this post.

Thanks again @invisnet for taking the time to clarify the official position on this.

3 Likes
9

The fundamental issue is that, until 5.0, WP never released what we’d call a major version; everything from early 2.x through 4.9 was backward compatible (we’ll ignore the db for now to keep things simple). The “major” part of the version number was simply bumped when they felt like it - it had no actual meaning. That all changed with 5.0.

So, pre-5.0 we could have mapped everything to CP minor, but post-5.0 there’s a genuine major when coming from <5.0.

The way the filters work now simply reflects the mess WP created; thinking about this again after ~16 months I don’t see a better way to handle it.

1 Like
10

It is misaligned if you look at the specific version number components, but each filter (and the corresponding values of the WP_AUTO_UPDATE_CORE constant) still means basically the same thing conceptually:

  • major updates - in WP this is 4.8 → 4.9, but in CP this is 1.0 → 2.0. Automatically applying these updates is disabled by default but can be enabled via the allow_major_auto_core_updates filter.
  • minor updates - in WP this is 4.9.1 → 4.9.2, but in CP this is 1.0 → 1.1. Automatically applying these updates is enabled by default but can be disabled via the allow_minor_auto_core_updates filter.
  • patch updates - do not exist in WP, but in CP this is 1.0.0 → 1.0.1. Automatically applying these updates is enabled by default but can be disabled via the allow_patch_auto_core_updates filter, which only exists in CP.

You can see from the above list that conceptually, each filter behaves the same way as it did in WP, and we also added one new filter.

Since ClassicPress follows semver, minor updates may introduce new features but they will not break backwards compatibility (new features will be disabled by default, for example).

If we didn’t auto-upgrade across minor versions, it would increase our maintenance burden because we’d have to release each security patch not just against the latest 1.x version, but against all of 1.0, 1.1 etc. This behavior is arguably inconsistent with WP, but because we’re using semver we can do this in a way that won’t break sites.

I definitely agree we need docs for this.

I also agree that we shouldn’t re-use existing mechanisms from WP and change their meaning, but in this case, we wanted sites to behave generally the same way as they were already configured regarding automatic updates.

A related issue that we have not fixed yet: "Future security updates will be applied automatically" message is not shown · Issue #387 · ClassicPress/ClassicPress · GitHub

Other than fixing this remaining issue and documenting the way this works, is there anything else we can do to make this system easier to work with?

4 Likes