Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
In most cases, no security updates are provided for old minor versions of core, so any security release is created only on the latest minor version. However, there are a few scenarios where security releases might be issued for two branches:
- The next minor branch of Drupal 8 is in its release candidate phase. Core release candidates receive security coverage, so we would want to mark both (e.g.) 8.2.0-rc2 and 8.1.10 as security releases as part of the same security advisory in SA-CCORE-2016-004. Sites on 8.1.10 already have the fixes for the issues disclosed in the advisory, and they should not upgrade to 8.2.0-rc1 (as it is not yet stable and may include disruptive changes). The two releases are equivalent in terms of their security status.
- We have a one-month window following each minor release in which we do not disclose any security issues with the old minor release,
to give users time to update and adjust for any disruptions in the minor release. However, sometimes it is necessary to issue advisories within this window, so we provide a release for the old, unsupported minor as well as the new minor to ensure all users can update safely without breaking other things on their sites. This was the case with SA-CORE-2017-002, for which we released both Drupal 8.2.8 and Drupal 8.3.1. - In the future, we may adopt a shorter minor release cycle but provide security fix backports for the old minor, so it might be more common to have equivalent security releases on two branches. This is being discussed in #2945200: [policy, no patch] Four month minor release cycles.
Example of what the OP experienced when updating to 8.1.10, when 8.2.0-rc2 was also marked as a security release:
With the error message on admin pages:
There is a security update available for your version of Drupal. To ensure the security of your server, you should update immediately! See the available updates page for more information and to install your missing updates.
Fixing this issue will replace the _update_equivalent_security_releases()
temporary workaround.
Proposed resolution
- Normally, when a security advisory is issued for one minor branch, all previous releases (including previous security releases on earlier branches) should be marked insecure.
- In cases where releases are issued for two minor branches in the same advisory, the old minor branch should not be marked insecure and the user should not be told a security update is required.
- However, say 2017-002 had released 8.3.2 instead of 8.3.1. In that scenario, 8.2.8 would have been secure, 8.3.1 would have been insecure, and 8.3.2 woOuld have been secure
Comment | File | Size | Author |
---|---|---|---|
#109 | update-interdiff-109.txt | 947 bytes | xjm |
#109 | update-2804155-109.patch | 6.86 KB | xjm |
#106 | interdiff-2804155-103-106.txt | 1.14 KB | samuel.mortenson |
#106 | 2804155-106.patch | 6.86 KB | samuel.mortenson |
Comments
Comment #2
v7nguyen CreditAttribution: v7nguyen commentedSame issue, here. Ran "drush pm-update drupal" and received "Project drupal was updated successfully. Installed version is now 8.1.10." Also ran drush updb and drush cr.
Comment #3
drummhttps://www.drupal.org/project/drupal/releases/8.2.0-rc2 was also marked as a security release since it had the same fixes. We removed this and expect this issue to be fixed short-term for 8.1.10 installs.
I think core is looking to see if it has the latest security release in https://updates.drupal.org/release-history/drupal/8.x, without considering 8.2.x being different from 8.1.x.
Comment #4
drummIn core/modules/update/update.compare.inc,
update_calculate_project_update_status()
looks like it processes all the releases linearly. So a security release in 8.2.x makes it think all of 8.1.x is insecure.This might be a decent place to start digging into this:
Comment #5
geerlingguy CreditAttribution: geerlingguy commented@drumm - That seems to have fixed it, thanks! After another 'manually update' try, I get:
The question is, then... do versions that 'aren't-the-current-stable/security-supported-release' ever get 'security updates', or should we call this a bug in Drupal core's update mechanism? I'd lean towards the former... if you're using rc/beta/alpha releases of Drupal or any contrib projects, there's no shield next to the download link on the project page, and they're not 'officially' supported from a security perspective.
Comment #6
drummNon-full-releases do occasionally get marked as security releases. In this case, I think it makes sense. Alerting your 8.2.0-rc1 install that it isn't secure is a nice thing to do.
IIRC, there is some overlap when full releases of 8.2.x and 8.1.x are supported. If there happened to be a security release during that window, we'd hit this bug quite a bit harder.
And for future contrib semver support, we should have this. Contrib modules can and will make up their own schemes for what branches are supported.
Comment #7
mariagwyn CreditAttribution: mariagwyn commentedI can confirm that this is registering properly.
Comment #8
xjmOnce 8.2.0 ships in two weeks, any security release in the 8.2.x series will actually mean all of 8.1.x is insecure, because we will not make another 8.1.x. So the current behavior probably is actually better than the alternative. I guess we can avoid marking RCs as security releases rather than risking some site on 8.1.10 not know they need the security fixes that ship in the next security release of 8.2.x.
For 8.1.0, we had the final security release window of 8.0.0 on the same day, so if we had shipped a sec release that day the issue would indeed have been problematic. However, we've since changed this for a few reasons. In the future, minor support ends with the final security release window before the minor release (barring a zero-day or something that requires an emergency security release) and the minor is always scheduled on a patch release window.
All of this is specific release policy though that could change in the future, so it probably does not make sense to encode it for d.o or the update manager. What would make sense would be a way to indicate which SAs are included in a security release, because any release that includes 2016-004 is equivalent from a security perspective. They are valid now but will not be once there is a release that contains (say) a future 2016-005 or 2017-001 or whatnot.
Comment #9
drummAgreed, this sort of thing does get into surprising messiness. I added a couple related issues. They could use a bit more detail, so I’m not thinking about closing this as a duplicate yet.
#2461167: Create Security announcement content type will get us a bit closer to being able to provide which SA is for which release.
Comment #10
xjmRetitling since I forgot why this was still open after we made 8.2.0-rc1 not a security release anymore.
Comment #12
xjmTo actually support this, I think we would need a way of saying that two security release include the same SA's worth of security fixes. For example, using a scenario like:
So in October, having 8.6.1 should show you green cheer. Having 8.5.9 should be yellow neutral-face and indicate that there are updates available, but not that said updates are security update.
But after the November window, we absolutely need to make sure both 8.5.9 and 8.6.1 are marked as insecure angry-face.
The API-equivalent releases would hopefully have the same release date, but that's definitely a fragile thing to rely on (lol timezones). So we do need some added API or functionality to support this.
Comment #13
xjmKeep in mind that it's also possible for 8.5.9 to be released the last security window before 8.6.0, and become insecure with the release of 8.6.1, or going the other direction, for 8.5.10 to be released along with 8.6.3 and be equivalent in terms of security support even though very deficient in features and that's starting to look like security support for two minor branches which we do not want and not a part of any release proposal. However, there could be a scenario where we'd do that, even.
Comment #14
xjmTo support this, I think the core update feed would need to include information on what SAs are included in the core release. Otherwise, core has no meaningful way of comparing security releases other than which version number is higher, and if there is a security update with a higher version number, that one will be safest. It may just be equally safe to the one you're on if you were on the previous minor. You still should update because we don't support the minors concurrently; they're just the release-by-release evolution of one major version. The only difference is that you might install a security update, get the message again, and then do what is only a bugfix and feature update thereafter.
Comment #15
drummI’m leaning more toward putting something more-direct in update status XML than SAs included. This puts the logic burden on each Drupal installation; if we update it, we have to wait for a core update. If we add something to the XML, I’d rather have a boolean that says “this release is after all known SAs, at this time.” (Kinda like #2766491: Update status should indicate whether installed contributed projects receive security coverage does for security advisory coverage.)
Better yet, I think, would be marking branches as unsupported, like module maintainers can do for major versions. So a core maintainer can mark all 8.5.x as unsupported on Drupal.org at the appropriate time. IIRC, this is also quite red in update status, but says “unsupported” instead of security update required. This would have to go with core’s logic for a security updates, 8.6.1 being a security update should have no effect on 8.5.x releases.
Comment #16
xjmThe first paragraph of #15 sounds like the best approach for now. So it would just become part of security release procedures to mark the releases that contain the current SA, as well as to un-mark previous security releases that do not contain the current SA? Since not all security fixes will be relevant for different branches.
Comment #17
xjmThe second paragraph of #15 seems like a separate scope to me, since d.o currently (correctly) assumes that 8.4.0 supercedes 8.3.9, etc., but that is a policy rather than a technical requirement.
Comment #18
drummI did a bit more reading of
update.compare.inc
, and it looks like core already has a way to mark specific releases as insecure:This release type has never existed on www.drupal.org as far as I know. We should be able to:
Comment #19
xjmThis would be every core security release every time there is a core SA, unless we specifically released an EOL sec release or security-covered release candidate.
It could also mean having to pay attention to whether unsupported branches include the vulnerabilities or not, which I'd rather not have to even think about. What we don't want to do though is give anyone the impression that 8.1.10 is just "unsupported" rather than "SECURITY UPDATE REQUIRED!".
Comment #20
xjmTo clarify, it could theoretically be possible that the first 8.3.x security release include issues that only affect 8.3.x. But eventually there will most likely be one in there that includes 8.2.x. I would not have to go back and figure that out each time.
Comment #21
drummThis patch adds minor version parsing to
update_process_project_info()
, and for core only, awareness of that toupdate_calculate_project_update_status()
.For testing, I faked the version number by editing
const VERSION = …
incore/lib/Drupal.php
. Setting that to something completely insecure, 8.1.2, looks like:All future 8.x.x security updates showing is existing behavior. I’m keeping that.
Without a change to Drupal.org, if I fake my core version number to 8.1.10, the final 8.1.x security release:
So it is now possible to have a future security release in the next minor version without saying what you are running is insecure. The security update is still mentioned as a possible upgrade.
I faked an update status server by:
8.x.txt
to serve athttp://drupalvm.dev/release-history/drupal/8.x
project status url: 'http://drupalvm.dev/release-history'
tocore/modules/automated_cron/automated_cron.info.yml
(the update status url is populated from the first.info.yml
file processed)<term><name>Release type</name><value>Insecure</value></term>
to 8.1.10.Now, the same fake core version is labelled as insecure. Once an old minor branch becomes completely insecure, we would set this term on the patch releases after the final security release, of that minor version’s branch.
Comment #22
drummKinda guessing at how to fix the coding standards messages - I added
use \Drupal\update\UpdateManagerInterface;
and now reference the constants likeUpdateManagerInterface::NOT_SECURE;
Comment #23
xjmWow those reports are really overwhelming. Does the update status normally list every security update rather than just the most recent one?
Comment #24
xjmLike, even on 8.1.2, should definitely not offer 8.1.3, 8.1.7, or 8.2.3 when 8.1.10 and 8.2.7 are available. Does that already happen?
Comment #25
drummYep, I was surprised too. People working on core must not see many really outdated sites.
Comment #26
drummI made a separate issue for slimming down the security updates offered: #2865920: When a site is multiple security releases behind for a given project, they are all listed in a paralyzing wall of terror
Comment #27
drummI missed the security updates from before 8.1.10 in my last screenshot.
Otherwise, ignore unpublished, insecure, or unsupported releases
was being hit beforeStop searching once we hit the currently installed version
. This patch rearranges the code to do those in a better order, along with theLook for the …
sections.Comment #30
xjmSo this bit is the bit that's problematic. A security release within that minor branch might be secure... but it also might not be if the branch is EOL. So at a certain point the release is insecure.
I think that rather than magic quessing and boolean, we should consider just marking releases with the latest SA they include. If we release a 2018-SA-CORE-002 that fixes both branches, the update module should know that and not display a warning.
But if we then release 2018-SA-CORE-003 and that fix is not backported to the previous branch, then that branch is not marked for the latest SA and should be shown as insecure.
Comment #31
xjmThe boolean in #15 is a similar idea I guess. We could add some process for that on d.o:
All of this would be totally agnostic of which actual releases, branches, etc., since those are things that change and differ from project to project.
Thoughts?
Bumping this to major since it's important to solve for our security release processes, especially for core.
Comment #32
larowlanComment #33
xjm@larowlan and I think this should maybe be critical, since affects how site owners receive security updates.
Comment #34
drummSpecifically, this is a priority to solve because there’s a good chance of simultaneous 8.x security releases when pre-releases are happening next to supported releases. Those pre-releases aren’t technically covered by security advisory policy, but it would still be good to mark them as security updates.
Comment #35
drummI filed a followup for Drupal.org based on #15 and later comments (yes we do want some automation/options to mass-mark releases as insecure) at #2942591: Start reporting specific releases as insecure in update status XML.
#21 has some details on testing, as far as I can tell from what I wrote, it worked. But there is no core test coverage for update status XML containing
<term><name>Release type</name><value>Insecure</value></term>
Comment #36
xjmUpdating the title a bit and rewriting the IS to describe the actual requirements/background info.
Comment #37
drummThis is the same patch as #27, it still applies cleanly to 8.6.x and tests pass. Just fixing the coding standards message.
#2942591: Start reporting specific releases as insecure in update status XML has been deployed to Drupal.org, so some releases are now being reported as Insecure, like https://updates.drupal.org/release-history/jsonapi/8.x
Comment #38
drummMentioning
_update_equivalent_security_releases()
in case someone searches for that function.Comment #40
xjmSo the current patch on this issue is out of date, because of a couple improvements to Drupal.org:
So all we need to do now for core is change it to only pay attention to whether Drupal.org reports the release as insecure or not, not whether there's a security release with a higher version. Then
_update_equivalent_security_releases()
can be removed.Comment #41
xjmThe simplest way to test this patch during development is to install HEAD but hack the version string in
core/lib/Drupal.php
, and change_update_equivalent_security_releases()
to return an empty array.Two interesting releases currently are Drupal 8.5.3 and 8.4.8. Expected results on the status report:
(Note: this is valid until the next core security release which will change what d.o reports.)
Comment #42
drummThe insecure release label was set on only past core releases on Drupal.org. The data hasn’t been populated on all contrib projects with security releases. We should verify it is being used correctly for recent contrib security releases; then we can backfill the rest.
In the meantime, the hybrid approach in the last patch is best.
Comment #43
xjmAh, I didn't know that contrib hadn't been backfilled yet. Maybe we can have a followup for contrib, and ensure that we are dealing with core only in this issue?
This is the logic that is problematic. It's changing the logic from "A new security release means all previous ones are insecure" to "A new security release means all previous security releases on this minor branch are insecure", which is wrong. Let's say there was never a security release on 8.4.x at all (there almost wasn't). When the first security release of 8.5.x came out, if there isn't an equivalent secure release of 8.4.x, all 8.4.x and earlier releases should still say "security update required!" Having logic for both is dangerous.
Meanwhile, some manual tests and testing will help. The goal of this issue is to get rid of
_update_equivalent_security_releases()
, make the "insecure release" data on d.o the sole arbiter of core secureness, and get it in sooner rather than later because even once we fix it the manual process around_update_equivalent_security_releases()
will have to stick around for an entire minor release afterward since a site on an old release will not yet have this code.Comment #44
xjm@tedbow is currently working on tests for this issue.
Comment #45
tedbowOk. I have add more test cases for security updates. The only test case that currently fails is the case where the current minor version has a security update it also shows the next versions security update in the list.
Comment #48
xjmHere are the screenshots @tedbow provided me WRT the case that's failing in #45. Before the patch, with 8.4.8, as of today when 8.4.8 includes the latest security advisory:
(Edit: See the following comment for the "after".)
Comment #49
xjmAdding an annotation to make the issue clearer:
Comment #50
xjmEdit: Just noticed that it's actually two fixtures, but same idea.
Can we add inline comments for each case in the provider explaining the condition and expected result? (So that the reader doesn't need to sift through the fixture to see what's what.)
expected_security_release
also seems incorrect to me as a provider key, or at least incomplete. I would have expected a boolean to indicate whether the given release is considered secure or not.Also, I think we shouldn't use the word "current" minor version, or shouldn't frame the test in that way. One of the reasons I want to not add special logic about whether it's a "current" minor or not is that this hardcodes release policy in the Update module. We're planning to adopt a policy where there's one minor that's in active support and one that's in security support only, but we could theoretically have a policy where there were two minors in active support, or something else entirely. So I don't think Drupal should try to guess about that. That's why I think we should only rely on the metadata from d.o to tell us whether the releases are secure following #2942591: Start reporting specific releases as insecure in update status XML.
Is the naming of the fixtures with numbers part of the existing test, or could we give them descriptive names?
Comment #51
tedbow@xjm thanks for the review!
1. there are more fixtures now. I have added a new parameter
$fixture
so you can easily find the fixture file.2. Added inline comments.
3. I like
expected_security_release
It is the expected security release for the test case.I changed
current_minor
tosite_minor
because it is really what minor the site is on.4. We could extend the fixture names but they would get very long. I think because now the test cases explicitly point to the fixture the names will matter a little less.
Comment #53
drummThe last 10 contrib security releases on Drupal.org look like they've all properly labeled previous releases as insecure. So I'm comfortable with writing a script to back-fill all contrib security releases. This should be straightforward, but edge cases surface when looking this far back, and updating the release history XML might take some time. I'd like to help avoid separate core and contrib logic if possible.
Comment #54
xjmThanks @tedbow and @drumm!
site_minor
is much clearer, +1.I am not sure what that means though. :)
So there are a few different things we are checking (some of which are in scope here):
NULL
forexpected_security_release
mean this message is not displayed, and any non-null value mean that it is?recommended_security_release
orsecurity_release_displayed
or something?In a followup issue we will also probably want to list both the latest security release for the site minor and the latest security release overall. Not in scope here, but we should
recommended_latest_release
or something (not in scope for this issue, but maybe for our followups WRT the other test cases).Maybe we can at least change them to something that's at least more than a integer? The reason is that it's hard to tell at a glance what the fixture name is versus the Drupal release in the fixture. We could have something like
scenario_security_2a
maybe. Like in the actual filename also.Comment #55
tedbowRemoves test for out of scope issues:
I think this should be green. Is there other test cases we need to cover in this issue?
Comment #56
drummFor posterity, and as a backup, here is the main round of releases being marked as insecure. Format is
The indented lines are the ones being marked as insecure. This does not cover pre-releases (alpha/beta/etc) labeled as security releases, because the logic I’m hijacking for this wasn’t written with that in mind.
Comment #57
tedbowSpelling verision
We need 1 more test case where:
adding this test case which emulates being a minor that is not longer receiving security updates and the next minor security update is needed.
The test should pass.
Should we make a follow to remove this separate logic when Insecure tag is propagated to all projects?
Comment #58
drummThe Insecure tag should be applied to all applicable releases sometime tomorrow. I’d go ahead and take out the separate core and contrib logic.
Comment #59
tedbow@drumm ok thanks. this patch removes the core specific logic in the patch.
I don't think there is any test for contrib security updates. I work on making the ones for contrib that I add for core.
Comment #61
tedbowWhoops
Comment #63
drummThe releases from #56 have all been backfilled and marked as insecure. The remainder is attached and running now.
Comment #64
drummThe releases from #63 have updated now. Now update status XML simply reports “Insecure” if insecure; and all other security-update-related logic can be discarded.
Comment #65
xjmI think the patch we want starts like this:
There is probably more dead code around it.
Comment #66
tedbow@xjm yes I figured out yesterday that could remove that logic. All the new tests pass.
I just add to update on of the contrib test files to add the insecure tag.
Re #61
I am working on this now.
Comment #67
xjmOh, let's also add this:
Comment #68
xjmI also think we can now remove the changes for parsing the minor version and moving the logic for it.
Comment #69
xjmThe docblock for
update_calculate_project_update_status()
also has some docs that will need to be updated. This bit:Comment #70
xjmOh also. I'd really like to commit a patch that includes all the test cases that are passing in HEAD, including the previously removed ones, before we make this change. Can we get a separate issue for that? Review and commit should go fairly quickly, and then this issue can just add the test cases relevant for the change.
Comment #71
xjmComment #72
tedbowHere are tests for contrib. Some are failing
This regex doesn't actually work for contrib.
So for
8.x-1.3
it won't find 3 as the minor version. This may be causing the test failure.Also in this sometimes the last character in the the contrib version is called "patch" and here it is called "minor"
Comment #74
tedbowRemoved debug line left in.
Comment #76
tedbow#2990511: Add comprehensive test coverage for update status for security releases is now RTBC. Rebuilding this patch on top of it.
update.compare.inc
that was moved dow the file unchanged. This is no longer necessary now that theInsecure
tag has been propagated to all projects.UPDATE: I should have made 2804155-76.patch a
do-not-test
patch file. canceled test.Comment #78
tedbowComment #79
tedbowupdate.compare.inc
in this patch I was trying figure what are part actually is needed for this fix.Re #72 the regex changes don't actually work for contrib.
So I removed them.
This part seems to be the only part that is necessary.
Comment #80
samuel.mortensonDo #67 and #69 still need addressed? Could you upload a test only patch as well to show what doesn't work in HEAD?
Comment #81
tedbow@samuel.mortenson good point
_update_equivalent_security_releases()
and not just deprecated it. looking at #2989243: _update_equivalent_security_releases() should not diverge per branch I don't understand why it just needs to be deprecate because it not called anywhere and since it start with "_" I think by policy it is an internal function.I am probably missing something though.
I think this
refers to this bug(or feature) #2865920: When a site is multiple security releases behind for a given project, they are all listed in a paralyzing wall of terror
I don't think anything in this patch addressed that problem so changing seems out of scope.
That being said I can't not reproduce #2865920: When a site is multiple security releases behind for a given project, they are all listed in a paralyzing wall of terror by checking out 8.7.x and changing
\Drupal::VERSION
to 8.1.10I have manually tested and with or without this current patch in 8.7.x if change
\Drupal::VERSION
to 8.1.10 afterupdate_calculate_project_update_status()
is run called in\update_calculate_project_data()
(it's only call).$projects["drupal"]["security updates"]
only has 1 security release 8.5.6.Well.... I see that @xjm already made a comment to that effect
Since that issue will now be used to add test coverage it seems reasonable to make the comment change there.
Comment #83
samuel.mortensonNit: some extra indentation here.
Comment #84
tedbowFixing #83, not uploading combined patch.
Comment #85
samuel.mortensonWe're waiting for #2990511: Add comprehensive test coverage for update status for security releases still but once that's in I consider this RTBC.
Comment #86
xjmThis is indeed internal, but: https://www.drupal.org/core/deprecation#internal
So let's make it throw a deprecation warning instead.
Since the function will no longer be maintained once we land this, we should also consider: https://www.drupal.org/core/deprecation#how-unintended
Comment #87
tedbow_update_equivalent_security_releases()
as per #86The new test cases that are being added now have case where there are security releases available but not required.
So changing 'update_available' to 'expected_update_message_type'. This will have the possible values
Comment #88
tedbowwhoops heres the patch.
If you want to run the test locally combine with #2990511-47: Add comprehensive test coverage for update status for security releases
Comment #89
tedbowThis todo is for this issue. Fixing
Whoops this has no security updates but also
static::SECURITY_UPDATE_REQUIRED
Let make sure a test like this would fail and fix this.
Comment #90
tedbowRerolled after #2990511: Add comprehensive test coverage for update status for security releases
Comment #91
samuel.mortensonLooks good to me! Nice work.
Comment #92
drummLooks good to me too.
Comment #93
tedbowHere is a test only patch to prove the new cases would not pass with the fix.
Comment #95
tedbowExpected fail in test only patch.
Comment #96
xjm+1 for this; I was thinking after I committed the last issue that something like this would be good. However, maybe we should fix it in a separate issue first to avoid confusion with what's the key fix here?
For the code comment here (and possibly elsewhere) it matters whether the security release for the previous minor is marked insecure or not. Probably we should mention that.
I checked whether the "b" fixture was still in use. It's used for the foreach loop for pre-release versions still.
Can we add this fixture to our list on the data provider?
So changing this from sec.0.2-rc-2b to sec-0.2-rc2 as the effect of switching it to a schema where there is a security release for an RC of the 8.2.0 version. That's one of the bugs we're trying to solve here. 👍
However, this calls my attention to the fact that the site fixtures seem to be named incorrectly. Shouldn't they be (e.g.)
sec.2.0-rc2
rather thansec.0.2-rc2
? Edit: Note that this is not in scope for this issue.Comment #97
xjmWe filed #2995076: Let UpdateTestBase check for multiple status messages to split the test improvements out into their own issue.
Comment #98
samuel.mortensonI applied the latest patch from #2995076: Let UpdateTestBase check for multiple status messages in reverse on top of #90, deleted
drupal.sec.1.2_secure.xml
as it was identical todrupal.sec.1.2.xml
, and updated documentation per #96.2Comment #100
samuel.mortensonHad a hard time converting the new tests back to the old data format.
Comment #101
xjmRetesting now that #2995076: Let UpdateTestBase check for multiple status messages is in.
Comment #103
samuel.mortensonRe-roll.
Comment #105
xjmOh, there's an error in the message here. It should be the "Insecure" release type rather than the "Internal" release type.
Also, the canonical name for d.o is "Drupal.org" (with a capital "D").
Comment #106
samuel.mortensonAddressed #105.
Comment #107
xjmOkay, this looks good to me! (Assuming tests don't unexpectedly fail).
Thanks @tedbow and @samuel.mortenson for the perseverance on this issue! This is the one we definitely want to have in prior to 8.6.0 for extended minor security coverage to be viable.
Comment #108
larowlanany reason not to use "" here and avoid \' ? bit neater
Comment #109
xjmYep, seems sensible.
Leaving at RTBC since this is a small change.
Comment #110
xjmNote for other reviewers (I forgot to repeat it here after discussing it with @samuel.mortenson yesterday). It's a full
trigger_error()
because the release list will not be maintained going forward, so we want users to get the deprecation warning about it without it being suppressed. This is along the lines of: https://www.drupal.org/core/deprecation#how-unintendedComment #113
larowlanCommitted as adf3293 and pushed to 8.7.x.
Cherry picked as 49c3ed2 and pushed to 8.6.x.
Thanks everyone
Comment #115
larowlanBackported to 8.5.x also