Happy Birthday Emails 1.0.3

[Alpha1]

Thanks.

 

[Alpha1]

After the fix, I'm no longer getting 1000 bounces per day, but only a few.

 

[stromb0li]

but only a few.

Are the bounces expected (e.g. user is active, part of scoped group, and outside the exclude group)?

 

[Alpha1]

The bounces I have received are expected.

 

[Alpha1]

I had Claude Code audit the code. It found 4 low level issues. No significant security issues.

## 1. The "test users" feature never sends, and enabling it silently stops all birthday emails [Medium]

• Type: Functional bug (broken feature + footgun)
• File: XF/Cron/HappyBirthdayEmail.php:11-25 (test branch), :19 (lookup)
• Expected: with one or more test users configured, the cron sends each of them a test birthday email.
• Actual: the jack_happy_birthday_test_users option uses edit_format="username" (_data/options.xml:27), so it stores usernames (XenForo's username-option convention stores the entered name and resolves it later by name — cf. core XF\Option\RegistrationWelcome doing findOne(User::class, ['username' => $name])). But the cron resolves each entry with:

php
  $user = $finder->where('user_id', $testUser)->fetchOne();   // :19  — $testUser is a username

user_id is an integer column, so a non-numeric username is coerced to 0 and matches no one → $user is null → no email. And because the test branch `return`s after the loop (:24), while any test user is configured no real birthday emails are sent either — an admin trying to test the add-on sees nothing work, in both test and production paths.

• Disprove attempt: where('user_id', $name) would only work if the option stored numeric IDs. Confirmed on the live install (dev ACP, Options → Happy Birthday configuration): the "Test users" field renders as a username token-input (Tagify, autocompleting against admin.php?users/find), so it stores usernames — which the cron then queries as user_id, coercing the name to 0 and matching no one.
• Fix: resolve test users by username — $finder->where('username', $testUser) (or where('Profile...') / findByUsername) — or store user IDs in the option.

## 2. Birthday email injects user values into a raw template without escaping [Low]

• Type: Output escaping (HTML injection, self-scoped)
• File: XF/Cron/HappyBirthdayEmail.php:96-114; template jack_happy_birthday_email ({$subject|raw}, {{$message|raw}})
• Expected: user-supplied values placed into the email are HTML-escaped (admin formatting in the body can stay HTML).
• Actual: replacePlaceholders() substitutes {$user.<path>} with raw user values (no escaping) into $subject/$message, and the email template emits both with |raw. A {$user.<field>} placeholder resolving to a free-text field containing HTML would render unescaped in the email.
• Why Low: the email goes to the birthday user containing their own data (not cross-user), XenForo usernames can't contain HTML, and the default templates only use {$user.username}/{$boardTitle}. Impact is limited to self-HTML in one's own birthday email; email clients don't execute script. Worth fixing as hardening.
• Fix: htmlspecialchars() each substituted user value before insertion (leave the admin body HTML intact).

## 3. Birthday date is evaluated in the board timezone, not the user's [Low]

• Type: Functional (mismatch with the code's own intent)
• File: XF/Cron/HappyBirthdayEmail.php:35-38
• Expected: the comment says "Consider user's time zone for sending birthday email."
• Actual: the day/month are computed from new \DateTimeZone($options->guestTimeZone) — the board default timezone, the same for every user — so birthdays fire on the board's date, not each recipient's local date. Either the comment is misleading or per-user timezone was intended and not implemented.
• Fix: if per-user timing is intended, compute dob_day/dob_month per user from their timezone; otherwise drop the comment.

## 4. Same schema column added by two setup steps with no existence guard [Low — code quality]

• Type: Upgrade robustness / code quality
• File: Setup.php:18-24 (installStep1) and :26-33 (upgrade1000100Step1)
• Expected: a column is added once, by a single path, with no risk of a duplicate ADD COLUMN.
• Actual: both steps call addColumn('jack_happy_birthday_receive_email', …) with no existence guard.
• Disprove attempt: XF does not swallow such an error — SchemaManager::handleException (2.3.10) only decorates and re-throws DDL exceptions, so a duplicate ADD COLUMN would be fatal. But I could not confirm a real triggering path: a fresh install runs only installStep1, and upgrade1000100Step1 runs only when upgrading from < 1.0.1 — it errors only if that older version's install had already added the column (its Setup.php wasn't available to check). So this is a genuine fragility, not a confirmed live failure.
• Fix: guard the add (skip if the column already exists), or add the column in only one path.

---

I hope this is useful to you. Please let me know if any of these are BS or are unclear.