Skip to content

Commit a40c1d3

Browse files
iMattProiMattPro
committed
Remove permanently gone endpoints
1 parent ab15164 commit a40c1d3

2 files changed

Lines changed: 133 additions & 3 deletions

File tree

‎notification/method/webpush.php‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -281,9 +281,11 @@ protected function notify_using_webpush(): void
281281
{
282282
if (!$report->isSuccess())
283283
{
284-
// Fill array of endpoints to remove if subscription has expired
285-
// Library checks for 404/410; we also check for 401 (Unauthorized)
286-
if ($report->isSubscriptionExpired() || $this->is_subscription_unauthorized($report))
284+
// Fill array of endpoints to remove if subscription has expired or is permanently gone.
285+
// Library checks for 404/410; we also check for 401 (Unauthorized) and endpoints
286+
// using the .invalid TLD (e.g. permanently-removed.invalid), which per RFC 6761 are
287+
// guaranteed to never resolve and are used as a sentinel for dead subscriptions.
288+
if ($report->isSubscriptionExpired() || $this->is_subscription_unauthorized($report) || $this->is_endpoint_permanently_removed($report->getEndpoint()))
287289
{
288290
$expired_endpoints[] = $report->getEndpoint();
289291
}
@@ -512,4 +514,22 @@ protected function is_subscription_unauthorized(\Minishlink\WebPush\MessageSentR
512514
$response = $report->getResponse();
513515
return $response && $response->getStatusCode() === 401;
514516
}
517+
518+
/**
519+
* Check if a push endpoint uses the .invalid TLD, meaning it can never resolve.
520+
*
521+
* Per RFC 6761, the .invalid TLD is reserved and guaranteed to never resolve in DNS.
522+
* It is commonly used as a sentinel value for dead/permanently-removed push subscriptions
523+
* (e.g. permanently-removed.invalid) where the push service has indicated the endpoint
524+
* is gone but no HTTP response was returned (e.g. cURL error 6: could not resolve host).
525+
*
526+
* @param string $endpoint
527+
*
528+
* @return bool True if the endpoint host ends with .invalid
529+
*/
530+
protected function is_endpoint_permanently_removed(string $endpoint): bool
531+
{
532+
$host = parse_url($endpoint, PHP_URL_HOST);
533+
return $host !== null && substr($host, -strlen('.invalid')) === '.invalid';
534+
}
515535
}

‎tests/notification/notification_method_webpush_test.php‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,47 @@ public function test_expired_subscriptions_deleted($notification_type, $post_dat
602602
}
603603
}
604604

605+
/**
606+
* @dataProvider data_notification_webpush
607+
*/
608+
public function test_permanently_removed_subscriptions_deleted($notification_type, $post_data, $expected_users): void
609+
{
610+
// Skip test if no expected users
611+
if (empty($expected_users))
612+
{
613+
$this->assertTrue(true);
614+
return;
615+
}
616+
617+
// Insert a permanently-removed.invalid subscription for the first user.
618+
// This simulates a dead subscription whose endpoint can never resolve (RFC 6761).
619+
$first_user_id = array_key_first($expected_users);
620+
$dead_endpoint = 'https://permanently-removed.invalid/fcm/send/test_dead_subscription';
621+
$this->insert_subscription_for_user($first_user_id, $dead_endpoint);
622+
623+
$this->assertEquals(1, $this->get_subscription_count(), 'Expected 1 subscription before notification');
624+
625+
$post_data = array_merge([
626+
'post_time' => 1349413322,
627+
'poster_id' => 1,
628+
'topic_title' => '',
629+
'post_subject' => '',
630+
'post_username' => '',
631+
'forum_name' => '',
632+
], $post_data);
633+
634+
// Send notifications — should trigger cleanup of the permanently-removed subscription
635+
$this->notifications->add_notifications($notification_type, $post_data);
636+
637+
// The dead subscription should have been silently deleted
638+
$this->assertEquals(0, $this->get_subscription_count(), 'Expected permanently-removed subscription to be deleted');
639+
640+
// Verify no admin log was written — unlike real delivery failures (which log errors),
641+
// permanently-removed endpoints should be silently cleaned up without noise.
642+
$admin_logs = $this->log->get_logs('admin');
643+
$this->assertEmpty($admin_logs, 'Expected no admin log entry for a permanently-removed subscription');
644+
}
645+
605646
public function test_get_type(): void
606647
{
607648
$this->assertEquals('notification.method.phpbb.wpn.webpush', $this->notification_method_webpush->get_type());
@@ -688,6 +729,54 @@ protected function createMockRequest(): \Psr\Http\Message\RequestInterface
688729
return $request;
689730
}
690731

732+
/**
733+
* Create a mock PSR-7 RequestInterface with a custom endpoint URL
734+
*/
735+
/**
736+
* Test is_endpoint_permanently_removed method
737+
*/
738+
public function test_is_endpoint_permanently_removed(): void
739+
{
740+
$reflection = new \ReflectionMethod($this->notification_method_webpush, 'is_endpoint_permanently_removed');
741+
$reflection->setAccessible(true);
742+
743+
// .invalid TLD sentinel — should return true
744+
$this->assertTrue(
745+
$reflection->invoke($this->notification_method_webpush, 'https://permanently-removed.invalid/fcm/send/abc123'),
746+
'Expected permanently-removed.invalid to be treated as permanently removed'
747+
);
748+
749+
// Any .invalid host — should return true
750+
$this->assertTrue(
751+
$reflection->invoke($this->notification_method_webpush, 'https://some-other.invalid/push/endpoint'),
752+
'Expected any .invalid host to be treated as permanently removed'
753+
);
754+
755+
// Valid FCM endpoint — should return false
756+
$this->assertFalse(
757+
$reflection->invoke($this->notification_method_webpush, 'https://fcm.googleapis.com/fcm/send/abc123'),
758+
'Expected valid FCM endpoint to not be treated as permanently removed'
759+
);
760+
761+
// Valid Mozilla endpoint — should return false
762+
$this->assertFalse(
763+
$reflection->invoke($this->notification_method_webpush, 'https://updates.push.services.mozilla.com/push/v1/abc123'),
764+
'Expected valid Mozilla endpoint to not be treated as permanently removed'
765+
);
766+
767+
// Subdomain spoofing attempt (host ends in .invalid.attacker.com, not .invalid) — should return false
768+
$this->assertFalse(
769+
$reflection->invoke($this->notification_method_webpush, 'https://permanently-removed.invalid.attacker.com/push'),
770+
'Expected .invalid.attacker.com to not be treated as permanently removed'
771+
);
772+
773+
// Empty/invalid URL — should return false
774+
$this->assertFalse(
775+
$reflection->invoke($this->notification_method_webpush, 'not_a_url'),
776+
'Expected unparseable URL to not be treated as permanently removed'
777+
);
778+
}
779+
691780
/**
692781
* @dataProvider data_notification_webpush
693782
*/
@@ -905,6 +994,27 @@ protected function get_all_subscriptions(): array
905994
return $sql_ary;
906995
}
907996

997+
/**
998+
* Create a real subscription via the push testing service for the given user, then overwrite
999+
* its endpoint with the specified value. This gives a subscription with valid encryption keys
1000+
* (required for payload encryption) but an endpoint that will never resolve — used for testing
1001+
* dead/sentinel endpoints such as permanently-removed.invalid.
1002+
*/
1003+
protected function insert_subscription_for_user(int $user_id, string $endpoint): void
1004+
{
1005+
// Get a real subscription from the push testing service so the p256dh/auth keys are
1006+
// valid base64url-encoded EC keys that the library can actually encrypt against.
1007+
$subscription_data = $this->create_subscription_for_user($user_id);
1008+
1009+
// Overwrite the endpoint to the dead one we want to test with.
1010+
$push_subscriptions_table = $this->container->getParameter('tables.phpbb.wpn.push_subscriptions');
1011+
$sql = 'UPDATE ' . $push_subscriptions_table . "
1012+
SET endpoint = '" . $this->db->sql_escape($endpoint) . "'
1013+
WHERE user_id = " . (int) $user_id . "
1014+
AND endpoint = '" . $this->db->sql_escape($subscription_data['endpoint']) . "'";
1015+
$this->db->sql_query($sql);
1016+
}
1017+
9081018
/**
9091019
* @depends test_get_subscription
9101020
*/

0 commit comments

Comments
 (0)