Add route constraint to only allow numeric ids (#39523)

app/controllers/activitypub/featured_collections_controller.rb

@@ -44,30 +44,30 @@ class ActivityPub::FeaturedCollectionsController < ApplicationController
   end
 
   def next_page_url
-    ap_account_featured_collections_url(@account, page: @collections.next_page) if @collections.respond_to?(:next_page)
+    ap_account_featured_collections_url(@account.id, page: @collections.next_page) if @collections.respond_to?(:next_page)
   end
 
   def prev_page_url
-    ap_account_featured_collections_url(@account, page: @collections.prev_page) if @collections.respond_to?(:prev_page)
+    ap_account_featured_collections_url(@account.id, page: @collections.prev_page) if @collections.respond_to?(:prev_page)
   end
 
   def collection_presenter
     if page_requested?
       ActivityPub::CollectionPresenter.new(
-        id: ap_account_featured_collections_url(@account, page: params.fetch(:page, 1)),
+        id: ap_account_featured_collections_url(@account.id, page: params.fetch(:page, 1)),
         type: :unordered,
         size: @account.collections.count,
         items: @collections,
-        part_of: ap_account_featured_collections_url(@account),
+        part_of: ap_account_featured_collections_url(@account.id),
         next: next_page_url,
         prev: prev_page_url
       )
     else
       ActivityPub::CollectionPresenter.new(
-        id: ap_account_featured_collections_url(@account),
+        id: ap_account_featured_collections_url(@account.id),
         type: :unordered,
         size: @account.collections.count,
-        first: ap_account_featured_collections_url(@account, page: 1)
+        first: ap_account_featured_collections_url(@account.id, page: 1)
       )
     end
   end

config/routes.rb

@@ -124,7 +124,7 @@ Rails.application.routes.draw do
   end
 
   scope path: 'ap', as: 'ap' do
-    resources :accounts, path: 'users', only: [:show], param: :id, concerns: :account_resources do
+    resources :accounts, path: 'users', only: [:show], param: :id, concerns: :account_resources, constraints: { id: /-?\d+/ } do
       resources :collections, only: [:show], constraints: { id: /\d+/ }
       resources :collection_items, only: [:show]
       resources :feature_authorizations, only: [:show], module: :activitypub

spec/routing/activitypub_routing_spec.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'Routes under ap/' do
+  it 'correctly handles numeric account ids' do
+    expect(get('/ap/users/1234/statuses/5678')).to route_to('statuses#show', account_id: '1234', id: '5678')
+  end
+
+  it 'correctly handles the instance actor id' do
+    expect(get('/ap/users/-99/statuses/5678')).to route_to('statuses#show', account_id: '-99', id: '5678')
+  end
+
+  it 'does not accept usernames' do
+    expect(get('/ap/users/john/statuses/5678')).to_not route_to('statuses#show', account_id: 'john', id: '5678')
+  end
+end