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.Over at #605318: Add some garbage collection to the update manager there's been some discussion of verifying MD5 hashes. That's out of scope for #605318 but we should discuss it. hass wrote:
Aside, I guess we should also check the file size or MD5 hash. Sometimes things are rotten and than a file is only downloaded half or with 0kb. In this case a file exists is TRUE, but the TAR could be corrupt. Not sure how we can grab the MD5 generated with every release from d.o... I only say this as I do expect that such issues may also occur (e.g. if d.o goes down while downloading) at least in theory... You never know, anything can happen :-)
As part of #538660: Move update manager upgrade process into new authorize.php file (and make it actually work) we introduced hook_archive_verify() which is invoked after the Update manager downloads something before it tries to install it. So, in theory, there could be a contrib module that does this.
However, we should consider if this needs to be part of core. If so, we certainly don't need to grab the MD5 for all releases on d.o. If it's an update, we already have the release history for the project, which includes the MD5 hashes. If it's a new install, we could just fetch the release history for the one project we're dealing with and find the hash in there.
So, this isn't necessarily a huge or complicated change, there's already a good place for this code to live (update_manager_archive_verify()), and it would almost certainly help with hardening and better recovery in cases of failure (something the Update manager is currently pretty terrible about handling).
Comments
Comment #1
bfroehle CreditAttribution: bfroehle commentedKicking the can down the road...
Comment #2
Leeteq CreditAttribution: Leeteq commentedSubscribing.
Comment #15
smustgrave CreditAttribution: smustgrave at Mobomo commentedWonder with the current state of update manager on D10 if this is still needed?
Comment #16
dwwUpdate Manager (as such) is all but deprecated at this point. Package Manager has all of TUF to verify the release artifacts, so this wouldn’t be needed there. I don’t see anyone spending time improving Update Manager anymore, especially since it doesn’t handle composer dependencies which are more and more essential.
As subsystem maintainer, I’d call this “closed (outdated)”, and start to mark a bunch of other Update Manager (but not Update Status) issues the same or similar.
However, I think it’d be good to get at least the product managers (if not release managers) to sign off on treating it as if it’s deprecated, even if the replacement isn’t in core (yet).
Thanks!
-Derek
Comment #17
smustgrave CreditAttribution: smustgrave at Mobomo commentedBased on the slack this can be closed out.