Skip to content

Clean up permanently removed endpoints #127

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.

Sign up for GitHub

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

Open
iMattPro wants to merge 1 commit into
base: main
Choose a base branch
from
+130 −3
Open

Clean up permanently removed endpoints #127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions notification/method/webpush.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,11 @@ protected function notify_using_webpush(): void
{
if (!$report->isSuccess())
{
// Fill array of endpoints to remove if subscription has expired
// Library checks for 404/410; we also check for 401 (Unauthorized)
if ($report->isSubscriptionExpired() || $this->is_subscription_unauthorized($report))
// Fill array of endpoints to remove if subscription has expired or is permanently gone.
// Library checks for 404/410; we also check for 401 (Unauthorized) and endpoints
// using the .invalid TLD (e.g. permanently-removed.invalid), which per RFC 6761 are
// guaranteed to never resolve and are used as a sentinel for dead subscriptions.
if ($report->isSubscriptionExpired() || $this->is_subscription_unauthorized($report) || $this->is_endpoint_permanently_removed($report->getEndpoint()))
{
$expired_endpoints[] = $report->getEndpoint();
}
Expand Down Expand Up @@ -512,4 +514,22 @@ protected function is_subscription_unauthorized(\Minishlink\WebPush\MessageSentR
$response = $report->getResponse();
return $response && $response->getStatusCode() === 401;
}

/**
* Check if a push endpoint uses the .invalid TLD, meaning it can never resolve.
*
* Per RFC 6761, the .invalid TLD is reserved and guaranteed to never resolve in DNS.
* It is commonly used as a sentinel value for dead/permanently-removed push subscriptions
* (e.g. permanently-removed.invalid) where the push service has indicated the endpoint
* is gone but no HTTP response was returned (e.g. cURL error 6: could not resolve host).
*
* @param string $endpoint
*
* @return bool True if the endpoint host ends with .invalid
*/
protected function is_endpoint_permanently_removed(string $endpoint): bool
{
$host = parse_url($endpoint, PHP_URL_HOST);
return $host !== null && substr($host, -strlen('.invalid')) === '.invalid';
}
}
107 changes: 107 additions & 0 deletions tests/notification/notification_method_webpush_test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,47 @@ public function test_expired_subscriptions_deleted($notification_type, $post_dat
}
}

/**
* @dataProvider data_notification_webpush
*/
public function test_permanently_removed_subscriptions_deleted($notification_type, $post_data, $expected_users): void
{
// Skip test if no expected users
if (empty($expected_users))
{
$this->assertTrue(true);
return;
}

// Insert a permanently-removed.invalid subscription for the first user.
// This simulates a dead subscription whose endpoint can never resolve (RFC 6761).
$first_user_id = array_key_first($expected_users);
$dead_endpoint = 'https://permanently-removed.invalid/fcm/send/test_dead_subscription';
$this->insert_subscription_for_user($first_user_id, $dead_endpoint);

$this->assertEquals(1, $this->get_subscription_count(), 'Expected 1 subscription before notification');

$post_data = array_merge([
'post_time' => 1349413322,
'poster_id' => 1,
'topic_title' => '',
'post_subject' => '',
'post_username' => '',
'forum_name' => '',
], $post_data);

// Send notifications — should trigger cleanup of the permanently-removed subscription
$this->notifications->add_notifications($notification_type, $post_data);

// The dead subscription should have been silently deleted
$this->assertEquals(0, $this->get_subscription_count(), 'Expected permanently-removed subscription to be deleted');

// Verify no admin log was written — unlike real delivery failures (which log errors),
// permanently-removed endpoints should be silently cleaned up without noise.
$admin_logs = $this->log->get_logs('admin');
$this->assertEmpty($admin_logs, 'Expected no admin log entry for a permanently-removed subscription');
}

public function test_get_type(): void
{
$this->assertEquals('notification.method.phpbb.wpn.webpush', $this->notification_method_webpush->get_type());
Expand Down Expand Up @@ -688,6 +729,51 @@ protected function createMockRequest(): \Psr\Http\Message\RequestInterface
return $request;
}

/**
* Test is_endpoint_permanently_removed method
*/
public function test_is_endpoint_permanently_removed(): void
{
$reflection = new \ReflectionMethod($this->notification_method_webpush, 'is_endpoint_permanently_removed');
$reflection->setAccessible(true);

// .invalid TLD sentinel — should return true
$this->assertTrue(
$reflection->invoke($this->notification_method_webpush, 'https://permanently-removed.invalid/fcm/send/abc123'),
'Expected permanently-removed.invalid to be treated as permanently removed'
);

// Any .invalid host — should return true
$this->assertTrue(
$reflection->invoke($this->notification_method_webpush, 'https://some-other.invalid/push/endpoint'),
'Expected any .invalid host to be treated as permanently removed'
);

// Valid FCM endpoint — should return false
$this->assertFalse(
$reflection->invoke($this->notification_method_webpush, 'https://fcm.googleapis.com/fcm/send/abc123'),
'Expected valid FCM endpoint to not be treated as permanently removed'
);

// Valid Mozilla endpoint — should return false
$this->assertFalse(
$reflection->invoke($this->notification_method_webpush, 'https://updates.push.services.mozilla.com/push/v1/abc123'),
'Expected valid Mozilla endpoint to not be treated as permanently removed'
);

// Subdomain spoofing attempt (host ends in .invalid.attacker.com, not .invalid) — should return false
$this->assertFalse(
$reflection->invoke($this->notification_method_webpush, 'https://permanently-removed.invalid.attacker.com/push'),
'Expected .invalid.attacker.com to not be treated as permanently removed'
);

// Empty/invalid URL — should return false
$this->assertFalse(
$reflection->invoke($this->notification_method_webpush, 'not_a_url'),
'Expected unparseable URL to not be treated as permanently removed'
);
}

/**
* @dataProvider data_notification_webpush
*/
Expand Down Expand Up @@ -905,6 +991,27 @@ protected function get_all_subscriptions(): array
return $sql_ary;
}

/**
* Create a real subscription via the push testing service for the given user, then overwrite
* its endpoint with the specified value. This gives a subscription with valid encryption keys
* (required for payload encryption) but an endpoint that will never resolve — used for testing
* dead/sentinel endpoints such as permanently-removed.invalid.
*/
protected function insert_subscription_for_user(int $user_id, string $endpoint): void
{
// Get a real subscription from the push testing service so the p256dh/auth keys are
// valid base64url-encoded EC keys that the library can actually encrypt against.
$subscription_data = $this->create_subscription_for_user($user_id);

// Overwrite the endpoint to the dead one we want to test with.
$push_subscriptions_table = $this->container->getParameter('tables.phpbb.wpn.push_subscriptions');
$sql = 'UPDATE ' . $push_subscriptions_table . "
SET endpoint = '" . $this->db->sql_escape($endpoint) . "'
WHERE user_id = " . (int) $user_id . "
AND endpoint = '" . $this->db->sql_escape($subscription_data['endpoint']) . "'";
$this->db->sql_query($sql);
}

/**
* @depends test_get_subscription
*/
Expand Down
Loading