From d5ea4215ebd39e4097b25daaab86f227fc45529e Mon Sep 17 00:00:00 2001 From: Jeff Turner Date: Mon, 4 Nov 2024 11:55:26 +1100 Subject: [PATCH 1/5] Remove whitespace from mobile number before using it in the regex to validate the SMS gateway response. Fixes #1093 --- include/sms_sender.class.php | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/include/sms_sender.class.php b/include/sms_sender.class.php index 2dc66668..2e10329b 100644 --- a/include/sms_sender.class.php +++ b/include/sms_sender.class.php @@ -272,8 +272,8 @@ public static function sendMessage($message, $recips, $saveAsNote=FALSE) $response = str_replace("\r", '', $response); if ($okReg = self::_getSetting('RESPONSE_OK_REGEX')) { foreach ($recips as $id => $recip) { - $reps['_RECIPIENT_INTERNATIONAL_'] = self::internationaliseNumber($recip['mobile_tel']); - $reps['_RECIPIENT_'] = $recip['mobile_tel']; + $reps['_RECIPIENT_INTERNATIONAL_'] = self::internationaliseNumber(self::normalizeNumber($recip['mobile_tel'])); + $reps['_RECIPIENT_'] = self::normalizeNumber($recip['mobile_tel']); $pattern = '/' . str_replace(array_keys($reps), array_values($reps), $okReg) . '/m'; if (preg_match($pattern, $response)) { $successes[$id] = $recip; @@ -315,6 +315,18 @@ private static function internationaliseNumber($number) return $number; } + /** + * Remove any whitespace or non-digits from telephone number. + * https://github.com/tbar0970/jethro-pmm/issues/1093 + */ + private static function normalizeNumber($number) + { + if ($number !== null) { + return preg_replace('/\D/', '', $number); + } + return null; + } + private static function logSuccess($recip_count, $message) { if (self::$configPrefix !== self::DEFAULT_CONFIG_PREFIX) return; // Log doesn't apply when using dedicated 2FA settings. @@ -403,4 +415,5 @@ private static function saveAsNote($recipients, $message) } } + } From 1b1e1d28afa0cc255c0fcef2ad01df6340ab74ad Mon Sep 17 00:00:00 2001 From: Jeff Turner Date: Sat, 9 Nov 2024 12:34:25 +1100 Subject: [PATCH 2/5] Fixes the effects of #1086, where person Age Brackets may have inadvertently be changed. --- ...-2.36-report-if-agebrackets-are-broken.php | 59 +++ .../AgeBracketChangesFixer.php | 345 ++++++++++++++++++ ...dmin__10_fix_broken_age_brackets.class.php | 176 +++++++++ 3 files changed, 580 insertions(+) create mode 100644 upgrades/2024-upgrade-to-2.36-report-if-agebrackets-are-broken.php create mode 100644 upgrades/upgradefixes/2.5.1_fix_agebrackets/AgeBracketChangesFixer.php create mode 100644 views/view_10_admin__10_fix_broken_age_brackets.class.php diff --git a/upgrades/2024-upgrade-to-2.36-report-if-agebrackets-are-broken.php b/upgrades/2024-upgrade-to-2.36-report-if-agebrackets-are-broken.php new file mode 100644 index 00000000..2fc26ebd --- /dev/null +++ b/upgrades/2024-upgrade-to-2.36-report-if-agebrackets-are-broken.php @@ -0,0 +1,59 @@ +setCLIScript(); + +require_once JETHRO_ROOT.'/include/system_controller.class.php'; +$GLOBALS['system'] = System_Controller::get(); + +$badchangegroups = AgeBracketChangesFixer::getBadChangeGroups(); + +// We're interested if any of our BadChangeGroups affect >1 user. If so that's a very likely indication of problems. +// Partition our BadChangeGroups into $groupedchanges[1] (single-person affected) and $groupedchanges['n'] (multiple persons affected). +$groupedchanges = array_reduce($badchangegroups, function ($carry, $cg) { + if ($cg->isBulkEdit()) { + $carry[MULTIPLE_PERSONS][] = $cg; + } else { + $carry[SINGLE_PERSON][] = $cg; + } + return $carry; +}, []); + +if (!empty($groupedchanges)) { + if (array_key_exists(MULTIPLE_PERSONS, $groupedchanges)) { + // Bulk edit definitely caused problems. Report affected persons by name. + $allaffected = array_map(fn($gc) => $gc->getAffectedPersons(), $groupedchanges[MULTIPLE_PERSONS]); + $allaffected = array_unique(array_merge(...$allaffected)); + print("Warning: ".count($allaffected)." people have been incorrectly turned into Adults by bug https://github.com/tbar0970/jethro-pmm/issues/1086:".PHP_EOL); + } elseif (array_key_exists(SINGLE_PERSON, $groupedchanges)) { + $allaffected = array_map(fn($gc) => $gc->getAffectedPersons(), $groupedchanges[SINGLE_PERSON]); + $allaffected = array_unique(array_merge(...$allaffected)); + print("Warning: ".count($allaffected)." people MAY have been incorrectly turned into Adults by bug https://github.com/tbar0970/jethro-pmm/issues/1086:".PHP_EOL); + } + foreach ($allaffected as $affected) { + print($affected.PHP_EOL); + } + print("Please go to Admin -> Fix Broken Age Brackets to fix this".PHP_EOL); +} +//AgeBracketChangesFixer::printBadChanges($badchanges); +//AgeBracketChangesFixer::fix($badChanges); +exit(0); diff --git a/upgrades/upgradefixes/2.5.1_fix_agebrackets/AgeBracketChangesFixer.php b/upgrades/upgradefixes/2.5.1_fix_agebrackets/AgeBracketChangesFixer.php new file mode 100644 index 00000000..bbc03ffd --- /dev/null +++ b/upgrades/upgradefixes/2.5.1_fix_agebrackets/AgeBracketChangesFixer.php @@ -0,0 +1,345 @@ +queryAll($sql), + function ($all, $row) { + $all[$row['id']] = $row; + return $all; + }, + []); + $dateBuggyJethroReleased = strtotime("2024-05-30"); + + // The fact that N changes were made 'together' is an important indicator that bulk change (and hence the bug) + // was involved. So our first step is to group likely problems by time. + // $tstamp_to_affectedpersons is an array of [quantizedtimestamp -> [personid -> BadChange]] + $tstamp_to_affectedpersons = []; + foreach ($persons as $personid => $persondetails) { + $hist = unserialize($persondetails['history']); + if (!$hist) { + echo "Could not unserialize person $personid history"; + continue; + } + foreach (array_reverse($hist, true) as $time => $histrecord) { + if ($time < $dateBuggyJethroReleased) break; // Ignore history record that happened before the buggy Jethro was released + $histlines = explode(PHP_EOL, $histrecord); // Separate lines of history record + if (count($histlines) <= 2) continue; // Besides the 'Updated by' and 'Age bracket changed from ..' line, there must be some other change for the Age bracket change to have been accidental + $oldAgeBrackets = array_filter(array_map(function ($line) use ($adult) { + if (preg_match('/Age bracket changed from "(.*)" to "'.$adult.'"/', $line, $match)) { + return $match[1]; + } else return null; + }, $histlines)); + if (empty($oldAgeBrackets)) continue; // This change doesn't change 'Age Bracket'. + $oldagebracket = array_values($oldAgeBrackets)[0]; + $badchange = new BadChange($time, $personid, $oldagebracket, $adult, $persondetails, $histlines); + $quantized_timestamp = intval($time / 10) * 10; // Group by 10s intervals in case a bulk edit took more than 1s + $tstamp_to_affectedpersons[$quantized_timestamp][$personid] = $badchange; + break; // We've found the most recent 'Age bracket changed' history item; older ones are irrelevant, so finish with this person. + } + } + + // Iterate through person changes grouped by time. Confirm the grouped changes were bulk edits, i.e. the changes were made by the same person + $badchangegroups = []; + foreach ($tstamp_to_affectedpersons as $time => $badchange) { + $firstlines = array_map(function ($changeinfo) { + return $changeinfo->getHistLines()[0]; + }, $badchange); + if (count(array_unique($firstlines)) != 1) trigger_error("First lines in history are expected to always be 'Updated by ...", E_USER_ERROR); + $oldagebracket = array_values(array_unique($firstlines))[0]; + if (preg_match('/Updated by ([\w\s]+) \(#(\d+)\)/', $oldagebracket, $matches)) { + $updater = $matches[1]; + $updaterid = $matches[2]; + } else { + trigger_error('First line of change, '.$firstlines[1].' does not match expected /Updated by ... (#...)/ regex.', E_USER_ERROR); + } + // Get the other fields changed (e.g. 'Status'), that the user was trying to set, when they accidentally set 'Age bracket' + $other_changed_fieldnames = array_values(array_map(function ($personinfo) use ($adult) { + return array_values(array_map(function ($histline) { + return preg_replace('/(.+) changed from "(.*)" to "(.*)"/', '\1', $histline); + }, array_filter($personinfo->getHistLines(), function ($histline) use ($adult) { + return !preg_match('/^Updated by /', $histline) && + !preg_match('/Age bracket changed from ".+" to "'.$adult.'"/', $histline); + }))); + }, $badchange)); + $other_changed_fieldnames = array_unique(array_merge(...$other_changed_fieldnames)); + + $badchangegroup = new BadChangeGroup($time, $updater, $updaterid, $other_changed_fieldnames, $badchange); + $badchangegroups[$time] = $badchangegroup; + } + // Sort by the number of persons affected, ascending. The low-count changes are more likely to not be buggy. + uasort($badchangegroups, fn($a, $b) => ($a->count() <=> $b->count())); + return $badchangegroups; + } + + /** + * @param BadChangeGroup[] $badchangegroups + * @return BadChangeFixInfo[] + * + */ + public static function fix($badchangegroups): array + { + $results = []; + foreach ($badchangegroups as $id => $cg) { + foreach ($cg->getBadChanges() as $badchange) { + $results[] = self::fixBadChange($badchange); + } + } + return $results; + } + + /** + * Fix the 'Age Bracket' of all persons in a BadChange. The change will be recorded in the person's history as done by user 'system'. + * @param BadChange $badchange + * @return BadChangeFixInfo + */ + public static function fixBadChange(BadChange $badchange): BadChangeFixInfo + { + $GLOBALS['system']->includeDBClass('person'); + $person = new Person(); + $person->load($badchange->getPersonId()); + $person->setValue('age_bracketid', self::_getAgeBracketIdByLabel($badchange->getOldagebracket())); + + // We don't want the current user recorded in the issue history as the 'fixer', as this is conceptually something Jethro would fix itself. + $_SESSION['user']['first_name'] = 'system'; + $_SESSION['user']['last_name'] = ''; + $_SESSION['id']['id'] = -1; + $person->save(); + return new BadChangeFixInfo($person->id, $person->toString(), $person->getValue('history'), $badchange->getOldAgebracket()); +} + + /** + * Return the default Age Bracket name, usually 'Adult'. + * @return string + */ + static function getDefaultAgeBracketLabel(): string + { + return $GLOBALS['db']->queryOne("select label from age_bracket where is_default=1;"); + } + + /** + * Return the database id of the given Age Bracket. If $label isn't found an error is triggered. + * @param $label e.g. 'Adult' + * @return int e.g. 1 + */ + private static function _getAgeBracketIdByLabel($label) : int + { + $id = $GLOBALS['db']->queryOne("select id from age_bracket where label=".$GLOBALS['db']->quote($label)); + if (is_null($id)) trigger_error("No age bracket '$label'."); + return $id; + } +} + +/** + * A collection of BadChanges that happened at (roughly) the same time, by the same person. + * If a BadChangeGroup contains more than one BadChange, that indicates a Bulk Edit happened and the Age Bracket changes are likely wrong. + * If the BadChangeGroup contains just one BadChange, it might be a bulk edit (affected by the bug) or a regular edit (not affected). + **/ +class BadChangeGroup +{ + private int $quantized_time; + private string $updater; + private int $updaterid; + /** + * @var array + */ + private array $other_changed_fieldnames; + /** @var BadChange[] $changes */ + private array $changes; + + public function __construct(int $quantized_time, string $updater, int $updaterid, array $other_changed_fieldnames, array $agebracket_change_info) + { + $this->quantized_time = $quantized_time; + $this->updater = $updater; + $this->updaterid = $updaterid; + $this->other_changed_fieldnames = $other_changed_fieldnames; + $this->changes = $agebracket_change_info; + } + + public function getId(): int + { + return $this->quantized_time; + } + + public function getUpdater(): string + { + return $this->updater; + } + + public function getUpdaterid(): int + { + return $this->updaterid; + } + + public function getQuantizedTimestamp(): string + { + return $this->quantized_time; + } + + public function getQuantizedTime(): string + { + return format_datetime($this->quantized_time); + } + + /** + * @return string[] + */ + public function getOtherChangedFieldNames(): array + { + return $this->other_changed_fieldnames; + } + + /** More than one change at the same time, i.e. a bulk edit. */ + public function isBulkEdit(): bool + { + return count($this->changes) > 1; + } + /** + * Number of persons changed. If only one, this might not have been a bulk edit affected by the bug! + * @return int + */ + public function count(): int + { + return count($this->changes); + } + + /** + * @return BadChange[] + */ + public function getBadChanges(): array + { + return $this->changes; + } + + public function getAffectedPersons(): array + { + return array_unique(array_map(fn($c) => $c->getPersonName(), $this->changes)); + } + + public function toString(): string + { + return "On ".$this->getQuantizedTime().", ".$this->getUpdater()." changed ".$this->count()." ".($this->count() == 1 ? "person" : "people")." to '".AgeBracketChangesFixer::getDefaultAgeBracketLabel()."', when changing other fields ".(implode(', ', array_map(fn($s) => "'".$s."'", $this->other_changed_fieldnames))).PHP_EOL; + } +} + +/** + * Information about a single point in time when Age Bracket changed (possibly incorrectly) to Adult. + */ +class BadChange +{ + /** + * @param int $time + * @param int $personid + * @param string $oldagebracket + * @param array $persondetail + * @param array $histlines + */ + public function __construct(int $time, int $personid, string $oldagebracket, string $newagebracket, array $persondetail, array $histlines) + { + $this->time = $time; + $this->personid = $personid; + $this->oldagebracket = $oldagebracket; + $this->newagebracket = $newagebracket; + $this->histlines = $histlines; + $this->persondetail = $persondetail; + } + + public function getTime(): int + { + return $this->time; + } + + public function getPersonid(): int + { + return $this->personid; + } + + public function getPersonName(): string + { + return $this->persondetail['first_name'].' '.$this->persondetail['last_name']; + } + + public function getHistlines(): array + { + return $this->histlines; + } + + public function getPersondetail(): array + { + return $this->persondetail; + } + + public function getNewagebracket(): string + { + return $this->newagebracket; + } + + public function getOldagebracket(): string + { + return $this->oldagebracket; + } +} +class BadChangeFixInfo +{ + private $personid; + private $personname; + private $history; + private $agebracket; + + /** + * @return mixed + */ + public function getPersonid() + { + return $this->personid; + } + + /** + * @return mixed + */ + public function getPersonName() + { + return $this->personname; + } + + /** + * @return mixed + */ + public function getHistory() + { + return $this->history; + } + + /** + * @return mixed + */ + public function getAgebracket() + { + return $this->agebracket; + } + + public function __construct($personid, $personname, $history, $agebracket) + { + + $this->personid = $personid; + $this->personname = $personname; + $this->history = $history; + $this->agebracket = $agebracket; + } +} \ No newline at end of file diff --git a/views/view_10_admin__10_fix_broken_age_brackets.class.php b/views/view_10_admin__10_fix_broken_age_brackets.class.php new file mode 100644 index 00000000..c42d4eb7 --- /dev/null +++ b/views/view_10_admin__10_fix_broken_age_brackets.class.php @@ -0,0 +1,176 @@ +_stage) { + case 'begin': + $this->_printBeginView(); + break; + + case 'done': + $this->_printDoneView(); + break; + } + } + + function processView() + { + if (!empty($_REQUEST['done'])) { + $this->_stage = 'done'; + } else if (!empty($_POST['confirm_fix'])) { + $this->_processFix(); + } else { + $this->_findAffectedPersons(); + } + } + + + private function _printBeginView() + { + $text = "Jethro 2.35.1 had a bug (#1086) where bulk editing persons would reset people's Age Bracket to '".AgeBracketChangesFixer::getDefaultAgeBracketLabel()."'. This page lets you revert 'Age Bracket' to the original value for affected persons. + Note: If a change affected just one person, we can't tell if it was a bulk edit, or a regular edit where Age Bracket was deliberately changed. Please review the single-person changes carefully. The multi-person changes are more likely to be incorrect, and these ticked by default."; + $text = '

'.str_replace("\n", '

', $text); + print_message($text, 'info', true); + ?> +

+ + _affectedpersons as $changegroup) { + ?> + + + + + + + + + _affectedpersons) > 0) { ?> + + + + + + + + + +
toString() ?> + isBulkEdit() ? "" : "
Warning: this may have been deliberate, and not be from a bulk change!") ?> +
+ isBulkEdit() ? "checked" : "unchecked") ?> + + + + + + + + + + + + + getBadChanges() as $change) { + ?> + + + + + + + + + +
PersonPrevious Age BracketCurrent Age BracketRelated ChangesProposed Fix
+ getPersonName() ?> + getOldagebracket() ?>getNewagebracket() ?>getHistlines(), fn($s, $line) => $s .= $line.'
', '') ?>		getNewagebracket() ?> → getOldagebracket() ?>
+
 
No affected persons!
+
+

Fix completed

+ _fixresults)) { + echo ''; + echo ''; + foreach ($this->_fixresults as $fixresult) { + echo ""; + echo ""; + echo ""; + echo ""; + } + echo "
PersonAge Bracket
".$fixresult->getPersonName()."".$fixresult->getAgebracket(); + echo "
"; + } else { + echo "No fix results??"; + } + } + + function _processFix() + { + $GLOBALS['system']->doTransaction('BEGIN'); + $changes = []; + foreach (AgeBracketChangesFixer::getBadChangeGroups() as $id => $change) { + if (in_array($id, $_REQUEST['time'])) { + $changes[$id] = $change; + } + } + $this->_fixresults = AgeBracketChangesFixer::fix($changes); + $GLOBALS['system']->doTransaction('COMMIT'); + $this->_stage = 'done'; + } + + /** + * @return stdClass + */ + + private + function _getAffectedPersons() + { + } + + private + function _findAffectedPersons() + { + $this->_affectedpersons = AgeBracketChangesFixer::getBadChangeGroups(); + } +} \ No newline at end of file From 2f2c0323d61374aaaa9c4737c61166a153634fbc Mon Sep 17 00:00:00 2001 From: Jeff Turner Date: Sat, 9 Nov 2024 12:41:40 +1100 Subject: [PATCH 3/5] Bugfix - allow names to contain apostrophes ("O'Leary") --- .../2.5.1_fix_agebrackets/AgeBracketChangesFixer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/upgrades/upgradefixes/2.5.1_fix_agebrackets/AgeBracketChangesFixer.php b/upgrades/upgradefixes/2.5.1_fix_agebrackets/AgeBracketChangesFixer.php index bbc03ffd..47356331 100644 --- a/upgrades/upgradefixes/2.5.1_fix_agebrackets/AgeBracketChangesFixer.php +++ b/upgrades/upgradefixes/2.5.1_fix_agebrackets/AgeBracketChangesFixer.php @@ -63,7 +63,7 @@ function ($all, $row) { }, $badchange); if (count(array_unique($firstlines)) != 1) trigger_error("First lines in history are expected to always be 'Updated by ...", E_USER_ERROR); $oldagebracket = array_values(array_unique($firstlines))[0]; - if (preg_match('/Updated by ([\w\s]+) \(#(\d+)\)/', $oldagebracket, $matches)) { + if (preg_match('/Updated by (.+) \(#(\d+)\)/', $oldagebracket, $matches)) { $updater = $matches[1]; $updaterid = $matches[2]; } else { From 71a2efc681abe71949b30974ecc366fb0849eabf Mon Sep 17 00:00:00 2001 From: Jeff Turner Date: Wed, 13 Nov 2024 21:55:32 +1100 Subject: [PATCH 4/5] Fix #1054 - our tempfile must have a .docx extension for later filetype inferral. --- views/view_2_families__4_contact_list.class.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/views/view_2_families__4_contact_list.class.php b/views/view_2_families__4_contact_list.class.php index 09c6b20b..c95ea856 100644 --- a/views/view_2_families__4_contact_list.class.php +++ b/views/view_2_families__4_contact_list.class.php @@ -500,7 +500,9 @@ public function printDOCX() } if (file_exists($templateFilename)) { require_once 'include/odf_tools.class.php'; - $outname = tempnam(sys_get_temp_dir(), 'contactlist-docx-'); + $outname = tempnam(sys_get_temp_dir(), 'contactlist'); + rename($outname, $outname.".docx"); // replaceKeywords() relies on the extension to know the filetype. + $outname = $outname.".docx"; copy($templateFilename, $outname); ODF_Tools::insertFileIntoFile($tempname, $outname, '%CONTACT_LIST%'); $replacements = Array('SYSTEM_NAME' => SYSTEM_NAME, 'MONTH' => date('F Y')); From a4645e721792a40d4845207d7307049c88f7d450 Mon Sep 17 00:00:00 2001 From: Jeff Turner Date: Wed, 13 Nov 2024 22:09:17 +1100 Subject: [PATCH 5/5] Prevent zero-byte /tmp/jethrozipXXXX files being created. --- views/view_9_documents.class.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/views/view_9_documents.class.php b/views/view_9_documents.class.php index f0242f7a..a612fe6e 100644 --- a/views/view_9_documents.class.php +++ b/views/view_9_documents.class.php @@ -546,7 +546,9 @@ function serveZip() $downloadFilename = array_get($_REQUEST, 'zipname', 'JethroFiles'); if (substr($downloadFilename, -4) != '.zip') $downloadFilename .= '.zip'; $zip = new ZipArchive(); - $zipFilename = tempnam(sys_get_temp_dir(), 'jethrozip').'.zip'; + $zipFilename = tempnam(sys_get_temp_dir(), 'jethrozip'); + rename($zipFilename, $zipFilename.".zip"); + $zipFilename .= ".zip"; if ($zip->open($zipFilename, ZipArchive::CREATE)!==TRUE) { exit("cannot open <$zipFilename>\n"); }