Skip to content

Fixed gift preview redirect for invalid tokens#27645

Merged
sagzy merged 1 commit into
mainfrom
crazy-proskuriakova-857aa8
May 1, 2026
Merged

Fixed gift preview redirect for invalid tokens#27645
sagzy merged 1 commit into
mainfrom
crazy-proskuriakova-857aa8

Conversation

@sagzy
Copy link
Copy Markdown
Contributor

@sagzy sagzy commented May 1, 2026

closes https://linear.app/ghost/issue/BER-3524

  • Visiting /gift/<invalid-token> (e.g. a stale share link or mistyped URL) crashed the request with: ERROR Unhandled rejection: Cannot read properties of null (reading 'cadence')
  • giftService.getByToken resolves with null for an unknown token rather than throwing, so the controller's existing try/catch did not cover the missing-gift case. The handler then dereferenced gift.cadence and produced a 500 for visitors hitting any non-resolving gift URL.

Changes

  • ghost/core/core/server/web/gift-preview/controller.js — when getByToken returns null, throw a NotFoundError inside the existing try block so the same catch (warn + 302 to homepage) handles both the throw and the not-found paths. No new branch, no duplicated redirect logic.
  • ghost/core/test/unit/server/web/gift-preview/controller.test.js — added a regression test that mocks getByToken to resolve null and asserts the homepage redirect.

Test plan

  • New unit test covers the null return path
  • pnpm test:single test/unit/server/web/gift-preview/controller.test.js from ghost/core/
  • Manual: curl -i https://<host>/gift/abc returns 302 to /, no unhandled rejection in logs
  • Valid gift tokens still render the OG preview HTML (existing tests cover this)

no ref

`giftService.getByToken` resolves with `null` (not a thrown error) when
the token does not exist, so the gift preview controller's `try/catch`
never caught the missing-gift case. The handler then dereferenced
`gift.cadence`, producing an unhandled rejection and a 500 response for
visitors hitting a stale or mistyped `/gift/<token>` URL.

Throw a `NotFoundError` inside the existing `try` block when the lookup
returns `null` so the same catch logs and redirects to the homepage for
both the throw and the not-found paths. Added a sibling unit test for
the `null` return so we don't regress this again.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 1, 2026

Walkthrough

The gift preview controller is updated to explicitly handle the case where a gift lookup returns no result. The controller now imports the error utilities and throws a NotFoundError when a gift is not found by token. The existing error handling mechanism catches this error, logs a warning, and redirects the request to the homepage. A corresponding test case is added to verify that a null gift lookup result triggers the expected 302 redirect behavior to the site homepage.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately reflects the main change: handling invalid gift preview tokens by converting null returns into proper redirects instead of crashes.
Description check ✅ Passed The pull request description accurately describes the bug fix, explains the root cause, details the changes made to both production and test code, and includes a comprehensive test plan.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch crazy-proskuriakova-857aa8

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sagzy sagzy changed the title 🐛 Fixed crash when visiting an invalid gift redemption link Fixed gift preview redirect for invalid tokens May 1, 2026
@sagzy sagzy enabled auto-merge (squash) May 1, 2026 11:50
@sagzy sagzy merged commit a95ddb8 into main May 1, 2026
44 checks passed
@sagzy sagzy deleted the crazy-proskuriakova-857aa8 branch May 1, 2026 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant