-
Notifications
You must be signed in to change notification settings - Fork 16
feat(redirect): serve a friendly 410 page for expired links #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,6 +136,52 @@ function generateInterstitialHTML(schemeUrl: string, fallbackUrl: string, title? | |
| </body></html>`; | ||
| } | ||
|
|
||
| /** | ||
| * Whether a link's expiration timestamp has passed. Links without expires_at | ||
| * never expire. An unparseable timestamp is treated as not expired (fail open) | ||
| * so a malformed value can't take a live link down. | ||
| */ | ||
| export function isLinkExpired(expiresAt: string | Date | null | undefined, now: Date = new Date()): boolean { | ||
| if (!expiresAt) return false; | ||
| const t = new Date(expiresAt).getTime(); | ||
| return Number.isFinite(t) && t <= now.getTime(); | ||
| } | ||
|
|
||
| /** | ||
| * Friendly page served (with HTTP 410 Gone) when someone opens a link past its | ||
| * expiration date. Deliberately unbranded: core is white-label and the page is | ||
| * served on customers' own short-link domains. | ||
| */ | ||
| export function generateExpiredLinkHTML(): string { | ||
| return `<!DOCTYPE html> | ||
| <html><head> | ||
| <meta charset="utf-8"> | ||
| <meta name="viewport" content="width=device-width,initial-scale=1"> | ||
| <meta name="robots" content="noindex"> | ||
| <title>Link expired</title> | ||
| <style> | ||
| body { font-family: -apple-system, system-ui, sans-serif; display: flex; align-items: center; justify-content: center; min-height: 100vh; margin: 0; background: #f9fafb; color: #111827; text-align: center; } | ||
| .container { padding: 2rem; max-width: 26rem; } | ||
| .clock { width: 48px; height: 48px; margin: 0 auto 1.5rem; color: #9ca3af; } | ||
| h1 { font-size: 1.25rem; font-weight: 600; margin: 0 0 0.5rem; } | ||
| p { font-size: 0.875rem; color: #6b7280; margin: 0; line-height: 1.5; } | ||
| .powered { position: fixed; bottom: 1.25rem; left: 0; right: 0; font-size: 0.75rem; color: #9ca3af; } | ||
| .powered a { color: #6b7280; text-decoration: none; font-weight: 500; } | ||
| .powered a:hover { text-decoration: underline; } | ||
| </style> | ||
| </head><body> | ||
| <div class="container"> | ||
| <svg class="clock" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="1.5" stroke-linecap="round" aria-hidden="true"> | ||
| <circle cx="12" cy="12" r="9"></circle> | ||
| <path d="M12 7v5l3 2"></path> | ||
| </svg> | ||
| <h1>This link has expired</h1> | ||
| <p>The link you followed is no longer active. If you were expecting to find something here, ask whoever shared it for an up-to-date link.</p> | ||
| </div> | ||
| <div class="powered">Powered by <a href="https://linkforty.com" rel="noopener">LinkForty</a></div> | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The doc comment 26 lines up says the page is "deliberately unbranded: core is white-label and the page is served on customers' own short-link domains" — and then this line renders a link to linkforty.com. The comment and the code want opposite things, so it's worth picking one deliberately. Self-hosters serving this on their own domain get an outbound backlink they didn't opt into, which is a slightly awkward default for an OSS package. If the branding stays, making it opt-in via a route option would be cleaner — #37 sets up exactly that pattern with export function generateExpiredLinkHTML(options: { poweredByUrl?: string } = {}): stringEither way the comment should match whatever we land on. |
||
| </body></html>`; | ||
| } | ||
|
|
||
| export async function redirectRoutes(fastify: FastifyInstance) { | ||
| // Helper function to handle the actual redirect logic | ||
| async function handleRedirect(request: any, reply: any, shortCode: string, templateSlug?: string) { | ||
|
|
@@ -188,6 +234,22 @@ export async function redirectRoutes(fastify: FastifyInstance) { | |
| const result = await db.query(query, params); | ||
|
|
||
| if (result.rows.length === 0) { | ||
| // Distinguish "expired" from "never existed": an expired link keeps its | ||
| // expires_at even after the hourly expiration job flips is_active off, | ||
| // so this lookup stays accurate long after expiry. | ||
| const expiredCheck = templateSlug | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth flagging the cost: every miss now runs two queries instead of one. On a link shortener the 404 path is mostly scanners probing random paths, so this doubles the cost of exactly the traffic least worth paying for. It's an indexed lookup on a unique column so it's cheap in absolute terms — not blocking. The better fix is the direction #37 is already heading: drop the |
||
| ? await db.query( | ||
| `SELECT 1 FROM links l JOIN link_templates t ON l.template_id = t.id | ||
| WHERE l.short_code = $1 AND t.slug = $2 AND l.expires_at IS NOT NULL AND l.expires_at <= NOW()`, | ||
| [shortCode, templateSlug] | ||
| ) | ||
| : await db.query( | ||
| 'SELECT 1 FROM links WHERE short_code = $1 AND expires_at IS NOT NULL AND expires_at <= NOW()', | ||
| [shortCode] | ||
| ); | ||
| if (expiredCheck.rows.length > 0) { | ||
| return reply.status(410).type('text/html').send(generateExpiredLinkHTML()); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: 410 is cacheable by default under HTTP semantics, so intermediaries may store this without an explicit directive. If an expiry later gets extended, clients and CDNs can keep serving the gone page after the link is live again. return reply
.status(410)
.header('Cache-Control', 'no-store')
.type('text/html')
.send(generateExpiredLinkHTML());#37 does this on its interstitial, so it'd be consistent. Applies to the cache-hit branch below too. |
||
| } | ||
| return reply.status(404).send({ error: 'Link not found' }); | ||
| } | ||
|
|
||
|
|
@@ -205,6 +267,12 @@ export async function redirectRoutes(fastify: FastifyInstance) { | |
|
|
||
| const link = JSON.parse(linkData); | ||
|
|
||
| // Enforce expiry on cache hits too — a link cached shortly before expiring | ||
| // would otherwise keep redirecting for up to the cache TTL (5 minutes). | ||
| if (isLinkExpired(link.expires_at)) { | ||
| return reply.status(410).type('text/html').send(generateExpiredLinkHTML()); | ||
| } | ||
|
|
||
| // Check targeting rules BEFORE redirecting | ||
| if (link.targeting_rules) { | ||
| const userAgent = request.headers['user-agent'] || ''; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Helper coverage is fine. The gap is that nothing drives the route.
No test asserts that an expired short code actually returns 410, that an unknown one still returns 404 (the regression risk of adding a second lookup), or that the cache-hit path enforces expiry — which is the most valuable behaviour here and the easiest to break later.
redirect.safety.test.tsin #37 is the template to copy: register the real plugin, mock only the data layer, assert status codes throughfastify.inject(). Three tests in that style would lock this down.