Hoe ik de Backup and Migrate-module voor Drupal weer aan de praat kreeg

In 2017 verhuisde ik deze (Drupal-)website naar een nieuwe provider. Na enige tijd viel me op dat ik geen back-ups meer ontving. Dat was vreemd, want ik wist vrij zeker dat ik de standaard module voor back-ups, Backup and Migrate, had geïnstalleerd. Tijdens een korte blik op de logs maakte ik kennis met de foutmelding "Multiple or malformed newlines found in additional_header in mime_mail->send()" en het bijbehorend bugreport in de issuequeue van de voornoemde module.

Wat bleek? Het mailen van back-ups faalde op sites die op moderne versies van PHP draaiden. De reden hiervoor was mede door de heldere foutmelding al snel gevonden en had te maken met een veiligheidsgat dat in recente PHP-versies was gedicht. In comment numero 8 plaatste iemand al een werkende patch.

"Probleem opgelost!" zou je zeggen, maar bij free and open source software (FOSS) leidt het bestaan van een oplossing niet altijd automatisch tot een update van de software. De beheerders zijn vrijwilligers die ook maar net de tijd en de zin moeten hebben om een stukje van wildvreemden afkomstige code op de juiste manier met hun applicatie te verweven.

Als gebruiker kun je er dan voor kiezen om de software met de hand op je eigen websites en die van je klanten te patchen, maar dat resulteert erin dat je vanaf dat moment zelf verantwoordelijk bent voor het bijhouden van deze eigen versie van de module. Drupal laat je vanaf dat moment niet meer automatisch weten dat er updates zijn, en als je zo'n update toch installeert, overschrijft deze je eigen versie, zodat je telkens opnieuw moet patchen. Als het probleem wat je had niet in de update wordt opgelost, ben je dus weer terug bij af.

Een patch die op de plank blijft liggen, verandert niet, maar veroudert wel. De software waartegen hij gemaakt is, kan namelijk zelf in de tussentijd veranderen. Dat was hier ook het geval. De patch moet in dat geval opnieuw worden aangemaakt; dat heet een reroll. Daarnaast bleven mensen nieuwe problemen met de patch zelf melden, waarvan het me vijf jaar na dato niet (meer) duidelijk is of dit nou volledig ongerelateerde problemen waren of dat de patch gebreken had.

Hosting providers verzorgen vaak zelf al back-ups. Ik maakte me daarom niet superdruk om functionaliteit die het plotseling niet meer deed. Er was immers een work-around. (Je kunt je anno 2021 überhaupt afvragen hoe veilig en verantwoord het is om database-back-ups per e-mail te versturen.)

In januari 2018, toen ik toch al bezig was met een in een vorige blogpost besproken Drupal 8-feature, besloot ik toch eens naar de laatste patch voor deze bug te kijken en mijn verslag postte ik in commentaar #50. Voordat ik het doorhad, vroeg een van de beheerders van de module me om naar een volgende patch te kijken en zo werd ik langzaamaan binnengehengeld, totdat een groot deel van de patch door mij herschreven was en van circa 8 tot 18 kilobyte was uitgegroeid.

Een belangrijke reden hiervoor was dat telkens als ik dacht klaar te zijn, de beheerder weer een nieuw aspect meldde waarvan hij vond dat iemand ernaar moest kijken voordat de patch gecommit kon worden.

(Enigszins korzelig, maar waarschijnlijk onterecht, denk ik op zo'n moment: je software doet het niet, vader, zou je hem niet eens eerst weer aan de praat krijgen voordat je je zorgen gaat maken over punten en komma's? Zoals Joel Spolsky zegt, shipping is a feature.)

Een aantal zaken waar ik uiteindelijk voor verantwoordelijk was:

- rerolls,
- doorvoeren van coding standards,
- het schrijven van tests,
- het testbaar maken van de code (dit was waarom ik een deel van de oorspronkelijke patch moest herschrijven),
- security,
- installatie-/updatescripts, en
- refactoring,

en dat allemaal voor een patch die er voor zorgt dat er geen overbodige regelafbrekingen voorkomen in de header van een e-mail. Sterker, dat laatste aspect is zo'n beetje het enige waar ik niet naar heb gekeken. Ik ging ervan uit dat de oorspronkelijke patch werkte en heb me alleen met randzaken bemoeid.

In de tussentijd heb ik tests leren schrijven (in Simpletest, het standaard testframework van Drupal 7), want dat kon ik nog niet toen ik voor het eerst naar dit issue keek, en heel veel geleerd over de module. Het gevolg daarvan was dat ik meer issues ging oppakken en met name versie 3.7 van de Backup and Migrate-module is bezaaid met mijn vingerafdrukken.

In maart 2019 was het dan zo ver. De laatste versie van de patch werd toegevoegd aan de repository van de module. Goed voor mijn ego om ook eens een resultaat te kunnen zien. Na vier lange jaren ... wacht eens, vier jaar?

Iets kriebelde nog. Backups zijn van die dingen die op de achtergrond draaien. Feitelijk zou je regelmatig moeten kijken of je backupsysteem werkt, maar voor veel sitebeheerders is dat zelden een prioriteit. Er zouden sites kunnen zijn die al vier jaar zonder het door te hebben een kapotte back-up-module draaiden. E-mailadressen die ooit aan een sitebeheerder hadden behoord, kwamen inmiddels mogelijk uit bij hun bitterste concurrenten. Wilden we daar werkelijk database-back-ups naar gaan sturen?

Ik deelde mijn zorgen via e-mail met een van de beheerders van de module (toevallig ook een van de leden van het securityteam van Drupal) en die was het met me eens dat als onderdeel van de eerstvolgende moduleupdate we ook alle e-mail-back-ups moesten uitschakelen. Ook daarvoor schreef ik de PHP-code. Frapant dat de code die haperende functionaliteit herstelt, meteen die hele functionaliteit ook uitschakelt, maar het was de enige manier om ervoor te zorgen dat die functionaliteit veilig opnieuw kon worden ingevoerd.

Een door het updatescript aangemaakte korte melding hierover zou dan hopelijk de webmaster onder ogen komen, zodat die de e-mail-back-ups weer aan kon zetten.

Mijn issuecomments

#50: ik vraag me hardop af of de recentste patch niet te veel probeert op te lossen. Dit trekt onmiddellijk een maintainer aan die een verwante geest meent te herkennen.

#56 - 62: ik review de recentste patch. Mijn review laat een andere maintainer concluderen dat er blijkbaar geautomatiseerde tests moeten worden toegevoegd. Er volgt wat discussie.

#64: ik review een nieuwe patch en let nu ook goed op of de coding standards van Drupal correct zijn toegepast. In #66 vraag ik wat er nu nog moet worden gedaan voordat we een release kunnen verwachten, en omdat er inmiddels een half jaar voorbij is gegaan, is het weinig verrassende antwoord: "de patch moet opnieuw worden gerold".

#70 en 71: ik reroll de patch (en lever een interdiff mee, waarschijnlijk omdat me op dat moment nog niet helemaal duidelijk is wat interdiffs zijn).

#73: nadat een van de maintainers heeft aangegeven dat er geen release komt als er niet eerst automatische tests worden toegevoegd, haak ik af: ik heb geen verstand van testen.

#81: ik krijg het op mijn heupen en schrijf toch Mijn Eerste Simpletest. Daarmee schiet ik mezelf ook in de voet, want in de tussentijd heeft iemands iets slims bedacht: als hij een eenvoudigere, elegantere patch schrijft (en dat doet hij), dan zijn automatische tests wellicht overbodig. En de maintainer is het daar mee eens!

#83, 84 en 86: om te kunnen testen, moet ik wel die nieuwe, elegante patch testbaar maken. Een van de dingen die ik ontdek is dat alle coole modules in plaats van de standaard PHP-functie mail(), de Drupal-wrapper drupal_mail() gebruiken. Deze heeft een ingebouwde mailcatcher die tijdens automatische tests in plaats van de mailserver wordt gebruikt.

Ik maak een nieuw patch met daarin wat eenvoudige testfunctionaliteit.

#88, 90 en 92: ik voeg nog wat tests toe, maar ondanks dat deze op mijn eigen pc met vlag en wimpel slagen, heeft de testrunner van drupal.org er zichtbaar problemen mee.

#97: een van de maintainers begint zich nu actiever op dit issue in te zetten en begint zelf code aan te passen. Mijn testfunctie assertMailStringCount() blijkt een zooitje te zijn en ik verander hem vrijwel volledig. Daarbij heb ik veel aan de suggestie van Webchick om code achterstevoren te lezen.

This is a great way to catch things like some_function(NULL, TRUE, array(), $booger, 'aardvark'). Without benefit of having seen the PHPDoc for some_function() ahead of time, if the calling code makes no sense, we have some work to do. This work could involve renaming some_function() to something that makes it more obvious what it's for, or it could involve refactoring some_function() into several functions that each take parameters specific to their functionality, or it could just involve re-formatting the way options are passed in. At the very least, it means we need some inline comments to explain what it does.

In issue #100 volgt de commit naar de shared repository! Met andere woorden, bij de eerstvolgende release gaat mijn patch mee. Dat ging nog best soepel als je bedenkt dat ik 10 dagen eerder besloot de dingen moeilijker te maken dan nodig was.

En dat is het moment dat ik me bedenk dat een e-mail-issue van 4 jaar oud een veiligheidsprobleem kan zijn als je het oplost. Terug naar de werkbank.

#103: ik schakel middels de update-functie van Drupal alle e-mailbestemmingen voor de module uit.

#106: ik antwoord op een review.

#109: mijn update-script bevatte nog een fout waar ik in het kader van een ander issue achterkwam.

#112: een reroll, want inmiddels zijn we een half jaar verder.

Op 28 februari 2020, ongeveer een jaar na de eerste commit, komt dan eindelijk een officiële release uit waarin deze patch zit verwerkt.

Dit was het tweede en laatste deel van een kleine serie over Drupal-issues waar ik aan heb meegewerkt. Het eerste deel ging over een dropzone-feature voor Drupal 8.

Haperen jouw modules en plug-ins? Bel of mail me en we kijken samen hoe we jouw site weer zo snel en goed mogelijk op de rails krijgen.