Skip to content

Commit b111e0c

Browse files
ralyodioclaude
andcommitted
fix(ads): refuse stale ad clicks outright instead of redirecting them
#264 marked a click that trails its impression past the device ceiling as invalid, but still wrote a row and still 302'd to the advertiser. That is the reward the feed-link harvester keeps coming back for: it fetches feeds with a Chrome UA, keeps the /a/<code> links, and replays them from fresh IPs days to weeks later. Last 7 days on prod: 13,178 of 13,513 clicks were exactly that, 0 of them valid. A stale click now gets a 410 with no link, no-store and noindex/nofollow, and nothing is recorded or charged. Feed-reader impressions keep no ceiling, so a subscriber opening an old item is untouched; no browser click in 30 days has trailed its impression past 6h. Every other invalid click (duplicate, cooldown, forged) is still recorded and redirected as before. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent a21cd1f commit b111e0c

5 files changed

Lines changed: 112 additions & 4 deletions

File tree

‎app/a/[id]/route.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@
1010
// impression row) redirect to the site rather than dead-ending.
1111

1212
import { NextRequest, NextResponse } from "next/server";
13-
import { resolveClick } from "@/lib/ads/serve";
13+
import { BLOCKED_CLICK, resolveClick } from "@/lib/ads/serve";
14+
import { blockedClickResponse } from "@/lib/ads/blocked-click";
1415
import { serviceClient } from "@/lib/supabase/service";
1516
import { lookupGeo } from "@/lib/tracker/geo";
1617
import { adClickIp } from "@/lib/ads/client-ip";
@@ -107,6 +108,7 @@ export async function GET(request: NextRequest, ctx: { params: Promise<{ id: str
107108
},
108109
});
109110

111+
if (dest === BLOCKED_CLICK) return blockedClickResponse();
110112
if (!dest) return NextResponse.redirect(fallback, { status: 302 });
111113
if (crawler) return NextResponse.redirect(dest, { status: 302, headers: { "cache-control": "no-store", "x-robots-tag": "noindex, nofollow" } });
112114

‎app/api/ads/click/route.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
// still redirect somewhere safe rather than dead-ending the user.
44

55
import { NextRequest, NextResponse } from "next/server";
6-
import { resolveClick } from "@/lib/ads/serve";
6+
import { BLOCKED_CLICK, resolveClick } from "@/lib/ads/serve";
7+
import { blockedClickResponse } from "@/lib/ads/blocked-click";
78
import { lookupGeo } from "@/lib/tracker/geo";
89
import { adClickIp } from "@/lib/ads/client-ip";
910
import { parseDevice } from "@/lib/tracker/device";
@@ -39,6 +40,7 @@ export async function GET(request: NextRequest) {
3940
ctx: { visitorId, ip, country: geo?.countryCode ?? null, device },
4041
});
4142

43+
if (dest === BLOCKED_CLICK) return blockedClickResponse();
4244
return NextResponse.redirect(dest ?? fallback, { status: 302 });
4345
} catch {
4446
return NextResponse.redirect(fallback, { status: 302 });

‎lib/ads/blocked-click.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// What a refused click gets back (see BLOCKED_CLICK in ./serve).
2+
//
3+
// 410 rather than a redirect to the homepage: a redirect is a reward, and the
4+
// harvesting crawler would simply follow it and crawl us instead. The body
5+
// carries no link to the advertiser for the same reason. noindex/nofollow and
6+
// no-store so nothing between us and it keeps the answer or the URL.
7+
8+
import { NextResponse } from "next/server";
9+
10+
export function blockedClickResponse(): NextResponse {
11+
return new NextResponse(
12+
"<!doctype html><meta charset=utf-8><title>Link expired</title><p>This ad link has expired.</p>",
13+
{
14+
status: 410,
15+
headers: {
16+
"content-type": "text/html; charset=utf-8",
17+
"cache-control": "no-store",
18+
"x-robots-tag": "noindex, nofollow",
19+
},
20+
},
21+
);
22+
}

‎lib/ads/serve.ts‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -717,15 +717,28 @@ async function resolveClickVisitor(
717717
}
718718
}
719719

720+
/**
721+
* resolveClick's answer for a click that is refused outright: no row, no
722+
* redirect. Today that is exactly one case, a click trailing an impression
723+
* past its device's ceiling (see maxClickAgeMs). Those were recorded as
724+
* invalid and still sent to the advertiser, which is what kept the harvesting
725+
* crawler coming back — ~37k a month, every one of them replaying a feed-item
726+
* link days to weeks after it was fetched. Nobody real is refused by it: no
727+
* browser click in 30 days trailed its impression past 6h, and feed-reader
728+
* impressions have no ceiling at all.
729+
*/
730+
export const BLOCKED_CLICK = Symbol("blocked-click");
731+
720732
// Resolve a click: record it, return the destination URL (with ?ref=) to
721-
// redirect to. Returns null if the campaign/creative can't be resolved.
733+
// redirect to. Returns null if the campaign/creative can't be resolved, and
734+
// BLOCKED_CLICK if the click is refused.
722735
export async function resolveClick(input: {
723736
impressionId?: string | null;
724737
slotId?: string | null;
725738
campaignId?: string | null;
726739
creativeId?: string | null;
727740
ctx?: ServeContext;
728-
}): Promise<string | null> {
741+
}): Promise<string | null | typeof BLOCKED_CLICK> {
729742
const sb = serviceClient();
730743
if (!input.campaignId) return null;
731744

@@ -753,6 +766,7 @@ export async function resolveClick(input: {
753766
ipHashes: rotatingIpHashCandidates(input.ctx?.ip ?? null, CLICK_DEDUPE_WINDOW_MS),
754767
device: input.ctx?.device,
755768
});
769+
if (validity.reason === "stale_impression") return BLOCKED_CLICK;
756770
if (validity.valid) {
757771
const admission = await claimClickCooldown({ visitorId, ip: input.ctx?.ip });
758772
if (!admission.allowed) validity = { valid: false, reason: admission.reason };
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
3+
// A click trailing its impression past the device ceiling is refused outright:
4+
// no ad_clicks row, no redirect to the advertiser. Recording it as invalid and
5+
// still redirecting is what kept the feed-link harvester coming back.
6+
7+
const state = vi.hoisted(() => ({
8+
validity: { valid: true } as { valid: boolean; reason?: string },
9+
inserts: [] as Record<string, unknown>[],
10+
rpc: vi.fn(),
11+
}));
12+
vi.mock("@/lib/supabase/service", () => ({ serviceClient: () => ({
13+
rpc: state.rpc,
14+
from(table: string) {
15+
const q = {
16+
select: () => q, eq: () => q,
17+
insert(row: Record<string, unknown>) { state.inserts.push(row); return q; },
18+
maybeSingle: async () => ({ data: table === "ad_campaigns"
19+
? { id: "campaign", destination_url: "https://example.com/offer", ref_slug: "ad-test", bid_credits: 4 }
20+
: { visitor_id: "visitor" } }),
21+
};
22+
return q;
23+
},
24+
}) }));
25+
vi.mock("@/lib/ads/fraud", async (original) => ({
26+
...await original<object>(), assessClickValidity: async () => state.validity,
27+
}));
28+
vi.mock("@/lib/ads/click-cooldown", () => ({ claimClickCooldown: async () => ({ allowed: true }) }));
29+
vi.mock("@/lib/ads/promos", () => ({ promoForCampaign: async () => null }));
30+
vi.mock("@/lib/ads/bids", () => ({ paperCharge: vi.fn() }));
31+
import { BLOCKED_CLICK, resolveClick } from "@/lib/ads/serve";
32+
import { blockedClickResponse } from "@/lib/ads/blocked-click";
33+
34+
beforeEach(() => {
35+
state.validity = { valid: true };
36+
state.inserts = [];
37+
state.rpc.mockReset();
38+
state.rpc.mockResolvedValue({ data: [{ click_id: "click", valid: true, charged_cents: 20 }] });
39+
});
40+
41+
const click = () => resolveClick({
42+
campaignId: "campaign", slotId: "slot", impressionId: "imp",
43+
ctx: { ip: "8.8.8.8", visitorId: "visitor", device: "desktop" },
44+
});
45+
46+
describe("stale ad clicks", () => {
47+
it("are refused: no row, no charge, no destination", async () => {
48+
state.validity = { valid: false, reason: "stale_impression" };
49+
expect(await click()).toBe(BLOCKED_CLICK);
50+
expect(state.inserts).toHaveLength(0);
51+
expect(state.rpc).not.toHaveBeenCalled();
52+
});
53+
54+
it("leave every other invalid click recorded and redirected", async () => {
55+
state.validity = { valid: false, reason: "duplicate" };
56+
expect(await click()).toBe("https://example.com/offer?ref=ad-test");
57+
expect(state.inserts).toHaveLength(1);
58+
});
59+
60+
it("get a 410 that links nowhere and is kept by nothing", async () => {
61+
const res = blockedClickResponse();
62+
expect(res.status).toBe(410);
63+
expect(res.headers.get("location")).toBeNull();
64+
expect(res.headers.get("cache-control")).toBe("no-store");
65+
expect(res.headers.get("x-robots-tag")).toContain("nofollow");
66+
expect(await res.text()).not.toContain("example.com");
67+
});
68+
});

0 commit comments

Comments
 (0)