Merge commit '0530ce5e9575c09464847412f43852f438b0494e' into glitch-soc/merge-upstream

app/controllers/api/v2/search_controller.rb

@@ -8,6 +8,11 @@ class Api::V2::SearchController < Api::BaseController
   before_action -> { authorize_if_got_token! :read, :'read:search' }
   before_action :validate_search_params!
 
+  with_options unless: :user_signed_in? do
+    before_action :query_pagination_error, if: :pagination_requested?
+    before_action :remote_resolve_error, if: :remote_resolve_requested?
+  end
+
   def index
     @search = Search.new(search_results)
     render json: @search, serializer: REST::SearchSerializer
@@ -21,12 +26,22 @@ class Api::V2::SearchController < Api::BaseController
 
   def validate_search_params!
     params.require(:q)
+  end
 
-    return if user_signed_in?
+  def query_pagination_error
+    render json: { error: 'Search queries pagination is not supported without authentication' }, status: 401
+  end
 
-    return render json: { error: 'Search queries pagination is not supported without authentication' }, status: 401 if params[:offset].present?
+  def remote_resolve_error
+    render json: { error: 'Search queries that resolve remote resources are not supported without authentication' }, status: 401
+  end
 
-    render json: { error: 'Search queries that resolve remote resources are not supported without authentication' }, status: 401 if truthy_param?(:resolve)
+  def remote_resolve_requested?
+    truthy_param?(:resolve)
+  end
+
+  def pagination_requested?
+    params[:offset].present?
   end
 
   def search_results
@@ -34,7 +49,15 @@ class Api::V2::SearchController < Api::BaseController
       params[:q],
       current_account,
       limit_param(RESULTS_LIMIT),
-      search_params.merge(resolve: truthy_param?(:resolve), exclude_unreviewed: truthy_param?(:exclude_unreviewed), following: truthy_param?(:following))
+      combined_search_params
+    )
+  end
+
+  def combined_search_params
+    search_params.merge(
+      resolve: truthy_param?(:resolve),
+      exclude_unreviewed: truthy_param?(:exclude_unreviewed),
+      following: truthy_param?(:following)
     )
   end
 

app/javascript/styles/mastodon/components.scss

@@ -2732,22 +2732,16 @@ $ui-header-height: 55px;
     &__description {
       flex: 1 1 auto;
       line-height: 20px;
-      white-space: nowrap;
-      text-overflow: ellipsis;
-      overflow: hidden;
 
       h6 {
         color: $highlight-text-color;
         font-weight: 500;
         font-size: 14px;
-        overflow: hidden;
-        text-overflow: ellipsis;
       }
 
       p {
         color: $darker-text-color;
         overflow: hidden;
-        text-overflow: ellipsis;
       }
     }
   }

config/initializers/inflections.rb

@@ -13,21 +13,22 @@
 # end
 
 ActiveSupport::Inflector.inflections(:en) do |inflect|
-  inflect.acronym 'StatsD'
-  inflect.acronym 'OEmbed'
-  inflect.acronym 'OStatus'
   inflect.acronym 'ActivityPub'
-  inflect.acronym 'PubSubHubbub'
   inflect.acronym 'ActivityStreams'
-  inflect.acronym 'JsonLd'
-  inflect.acronym 'Ed25519'
-  inflect.acronym 'TOC'
-  inflect.acronym 'RSS'
-  inflect.acronym 'REST'
-  inflect.acronym 'URL'
   inflect.acronym 'ASCII'
+  inflect.acronym 'CLI'
   inflect.acronym 'DeepL'
   inflect.acronym 'DSL'
+  inflect.acronym 'Ed25519'
+  inflect.acronym 'JsonLd'
+  inflect.acronym 'OEmbed'
+  inflect.acronym 'OStatus'
+  inflect.acronym 'PubSubHubbub'
+  inflect.acronym 'REST'
+  inflect.acronym 'RSS'
+  inflect.acronym 'StatsD'
+  inflect.acronym 'TOC'
+  inflect.acronym 'URL'
 
   inflect.singular 'data', 'data'
 end

spec/controllers/api/v2/search_controller_spec.rb

@@ -34,6 +34,26 @@ RSpec.describe Api::V2::SearchController do
           expect(body_as_json[:accounts].pluck(:id)).to contain_exactly(bob.id.to_s, ana.id.to_s, tom.id.to_s)
         end
 
+        context 'with truthy `resolve`' do
+          let(:params) { { q: 'test1', resolve: '1' } }
+
+          it 'returns http unauthorized' do
+            get :index, params: params
+
+            expect(response).to have_http_status(200)
+          end
+        end
+
+        context 'with `offset`' do
+          let(:params) { { q: 'test1', offset: 1 } }
+
+          it 'returns http unauthorized' do
+            get :index, params: params
+
+            expect(response).to have_http_status(200)
+          end
+        end
+
         context 'with following=true' do
           let(:params) { { q: 'test', type: 'accounts', following: 'true' } }
 
@@ -48,6 +68,26 @@ RSpec.describe Api::V2::SearchController do
           end
         end
       end
+
+      context 'when search raises syntax error' do
+        before { allow(Search).to receive(:new).and_raise(Mastodon::SyntaxError) }
+
+        it 'returns http unprocessable_entity' do
+          get :index, params: params
+
+          expect(response).to have_http_status(422)
+        end
+      end
+
+      context 'when search raises not found error' do
+        before { allow(Search).to receive(:new).and_raise(ActiveRecord::RecordNotFound) }
+
+        it 'returns http not_found' do
+          get :index, params: params
+
+          expect(response).to have_http_status(404)
+        end
+      end
     end
   end
 
@@ -59,6 +99,12 @@ RSpec.describe Api::V2::SearchController do
         get :index, params: search_params
       end
 
+      context 'without a `q` param' do
+        it 'returns http bad_request' do
+          expect(response).to have_http_status(400)
+        end
+      end
+
       context 'with a `q` shorter than 5 characters' do
         let(:search_params) { { q: 'test' } }
 
@@ -79,6 +125,7 @@ RSpec.describe Api::V2::SearchController do
 
           it 'returns http unauthorized' do
             expect(response).to have_http_status(401)
+            expect(response.body).to match('resolve remote resources')
           end
         end
 
@@ -87,6 +134,7 @@ RSpec.describe Api::V2::SearchController do
 
           it 'returns http unauthorized' do
             expect(response).to have_http_status(401)
+            expect(response.body).to match('pagination is not supported')
           end
         end
       end

spec/requests/accounts_spec.rb

@@ -2,23 +2,22 @@
 
 require 'rails_helper'
 
-RSpec.describe AccountsController do
-  render_views
-
+describe 'Accounts show response' do
   let(:account) { Fabricate(:account) }
 
-  describe 'unapproved account check' do
+  context 'with an unapproved account' do
     before { account.user.update(approved: false) }
 
     it 'returns http not found' do
       %w(html json rss).each do |format|
-        get :show, params: { username: account.username, format: format }
+        get short_account_path(username: account.username), as: format
+
         expect(response).to have_http_status(404)
       end
     end
   end
 
-  describe 'permanently suspended account check' do
+  context 'with a permanently suspended account' do
     before do
       account.suspend!
       account.deletion_request.destroy
@@ -26,25 +25,26 @@ RSpec.describe AccountsController do
 
     it 'returns http gone' do
       %w(html json rss).each do |format|
-        get :show, params: { username: account.username, format: format }
+        get short_account_path(username: account.username), as: format
+
         expect(response).to have_http_status(410)
       end
     end
   end
 
-  describe 'temporarily suspended account check' do
+  context 'with a temporarily suspended account' do
     before { account.suspend! }
 
     it 'returns appropriate http response code' do
       { html: 403, json: 200, rss: 403 }.each do |format, code|
-        get :show, params: { username: account.username, format: format }
+        get short_account_path(username: account.username), as: format
 
         expect(response).to have_http_status(code)
       end
     end
   end
 
-  describe 'GET #show' do
+  describe 'GET to short username paths' do
     context 'with existing statuses' do
       let!(:status) { Fabricate(:status, account: account) }
       let!(:status_reply) { Fabricate(:status, account: account, thread: Fabricate(:status)) }
@@ -66,17 +66,17 @@ RSpec.describe AccountsController do
 
         shared_examples 'common HTML response' do
           it 'returns a standard HTML response', :aggregate_failures do
-            expect(response).to have_http_status(200)
+            expect(response)
+              .to have_http_status(200)
+              .and render_template(:show)
 
             expect(response.headers['Link'].to_s).to include ActivityPub::TagManager.instance.uri_for(account)
-
-            expect(response).to render_template(:show)
           end
         end
 
         context 'with a normal account in an HTML request' do
           before do
-            get :show, params: { username: account.username, format: format }
+            get short_account_path(username: account.username), as: format
           end
 
           it_behaves_like 'common HTML response'
@@ -84,8 +84,7 @@ RSpec.describe AccountsController do
 
         context 'with replies' do
           before do
-            allow(controller).to receive(:replies_requested?).and_return(true)
-            get :show, params: { username: account.username, format: format }
+            get short_account_with_replies_path(username: account.username), as: format
           end
 
           it_behaves_like 'common HTML response'
@@ -93,8 +92,7 @@ RSpec.describe AccountsController do
 
         context 'with media' do
           before do
-            allow(controller).to receive(:media_requested?).and_return(true)
-            get :show, params: { username: account.username, format: format }
+            get short_account_media_path(username: account.username), as: format
           end
 
           it_behaves_like 'common HTML response'
@@ -106,9 +104,8 @@ RSpec.describe AccountsController do
           let!(:status_tag) { Fabricate(:status, account: account) }
 
           before do
-            allow(controller).to receive(:tag_requested?).and_return(true)
             status_tag.tags << tag
-            get :show, params: { username: account.username, format: format, tag: tag.to_param }
+            get short_account_tag_path(username: account.username, tag: tag), as: format
           end
 
           it_behaves_like 'common HTML response'
@@ -117,21 +114,25 @@ RSpec.describe AccountsController do
 
       context 'with JSON' do
         let(:authorized_fetch_mode) { false }
-        let(:format) { 'json' }
+        let(:headers) { { 'ACCEPT' => 'application/json' } }
 
-        before do
-          allow(controller).to receive(:authorized_fetch_mode?).and_return(authorized_fetch_mode)
+        around do |example|
+          ClimateControl.modify AUTHORIZED_FETCH: authorized_fetch_mode.to_s do
+            example.run
+          end
         end
 
         context 'with a normal account in a JSON request' do
           before do
-            get :show, params: { username: account.username, format: format }
+            get short_account_path(username: account.username), headers: headers
           end
 
           it 'returns a JSON version of the account', :aggregate_failures do
-            expect(response).to have_http_status(200)
-
-            expect(response.media_type).to eq 'application/activity+json'
+            expect(response)
+              .to have_http_status(200)
+              .and have_attributes(
+                media_type: eq('application/activity+json')
+              )
 
             expect(body_as_json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary)
           end
@@ -152,13 +153,15 @@ RSpec.describe AccountsController do
 
           before do
             sign_in(user)
-            get :show, params: { username: account.username, format: format }
+            get short_account_path(username: account.username), headers: headers.merge({ 'Cookie' => '123' })
           end
 
           it 'returns a private JSON version of the account', :aggregate_failures do
-            expect(response).to have_http_status(200)
-
-            expect(response.media_type).to eq 'application/activity+json'
+            expect(response)
+              .to have_http_status(200)
+              .and have_attributes(
+                media_type: eq('application/activity+json')
+              )
 
             expect(response.headers['Cache-Control']).to include 'private'
 
@@ -170,14 +173,15 @@ RSpec.describe AccountsController do
           let(:remote_account) { Fabricate(:account, domain: 'example.com') }
 
           before do
-            allow(controller).to receive(:signed_request_actor).and_return(remote_account)
-            get :show, params: { username: account.username, format: format }
+            get short_account_path(username: account.username), headers: headers, sign_with: remote_account
           end
 
           it 'returns a JSON version of the account', :aggregate_failures do
-            expect(response).to have_http_status(200)
-
-            expect(response.media_type).to eq 'application/activity+json'
+            expect(response)
+              .to have_http_status(200)
+              .and have_attributes(
+                media_type: eq('application/activity+json')
+              )
 
             expect(body_as_json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary)
           end
@@ -188,12 +192,13 @@ RSpec.describe AccountsController do
             let(:authorized_fetch_mode) { true }
 
             it 'returns a private signature JSON version of the account', :aggregate_failures do
-              expect(response).to have_http_status(200)
-
-              expect(response.media_type).to eq 'application/activity+json'
+              expect(response)
+                .to have_http_status(200)
+                .and have_attributes(
+                  media_type: eq('application/activity+json')
+                )
 
               expect(response.headers['Cache-Control']).to include 'private'
-
               expect(response.headers['Vary']).to include 'Signature'
 
               expect(body_as_json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary)
@@ -207,60 +212,58 @@ RSpec.describe AccountsController do
 
         context 'with a normal account in an RSS request' do
           before do
-            get :show, params: { username: account.username, format: format }
+            get short_account_path(username: account.username, format: format)
           end
 
           it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie'
 
           it 'responds with correct statuses', :aggregate_failures do
             expect(response).to have_http_status(200)
-            expect(response.body).to include_status_tag(status_media)
-            expect(response.body).to include_status_tag(status_self_reply)
-            expect(response.body).to include_status_tag(status)
-            expect(response.body).to_not include_status_tag(status_direct)
-            expect(response.body).to_not include_status_tag(status_private)
-            expect(response.body).to_not include_status_tag(status_reblog.reblog)
-            expect(response.body).to_not include_status_tag(status_reply)
+            expect(response.body).to include(status_tag_for(status_media))
+            expect(response.body).to include(status_tag_for(status_self_reply))
+            expect(response.body).to include(status_tag_for(status))
+            expect(response.body).to_not include(status_tag_for(status_direct))
+            expect(response.body).to_not include(status_tag_for(status_private))
+            expect(response.body).to_not include(status_tag_for(status_reblog.reblog))
+            expect(response.body).to_not include(status_tag_for(status_reply))
           end
         end
 
         context 'with replies' do
           before do
-            allow(controller).to receive(:replies_requested?).and_return(true)
-            get :show, params: { username: account.username, format: format }
+            get short_account_with_replies_path(username: account.username, format: format)
           end
 
           it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie'
 
           it 'responds with correct statuses with replies', :aggregate_failures do
             expect(response).to have_http_status(200)
-            expect(response.body).to include_status_tag(status_media)
-            expect(response.body).to include_status_tag(status_reply)
-            expect(response.body).to include_status_tag(status_self_reply)
-            expect(response.body).to include_status_tag(status)
-            expect(response.body).to_not include_status_tag(status_direct)
-            expect(response.body).to_not include_status_tag(status_private)
-            expect(response.body).to_not include_status_tag(status_reblog.reblog)
+            expect(response.body).to include(status_tag_for(status_media))
+            expect(response.body).to include(status_tag_for(status_reply))
+            expect(response.body).to include(status_tag_for(status_self_reply))
+            expect(response.body).to include(status_tag_for(status))
+            expect(response.body).to_not include(status_tag_for(status_direct))
+            expect(response.body).to_not include(status_tag_for(status_private))
+            expect(response.body).to_not include(status_tag_for(status_reblog.reblog))
           end
         end
 
         context 'with media' do
           before do
-            allow(controller).to receive(:media_requested?).and_return(true)
-            get :show, params: { username: account.username, format: format }
+            get short_account_media_path(username: account.username, format: format)
           end
 
           it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie'
 
           it 'responds with correct statuses with media', :aggregate_failures do
             expect(response).to have_http_status(200)
-            expect(response.body).to include_status_tag(status_media)
-            expect(response.body).to_not include_status_tag(status_direct)
-            expect(response.body).to_not include_status_tag(status_private)
-            expect(response.body).to_not include_status_tag(status_reblog.reblog)
-            expect(response.body).to_not include_status_tag(status_reply)
-            expect(response.body).to_not include_status_tag(status_self_reply)
-            expect(response.body).to_not include_status_tag(status)
+            expect(response.body).to include(status_tag_for(status_media))
+            expect(response.body).to_not include(status_tag_for(status_direct))
+            expect(response.body).to_not include(status_tag_for(status_private))
+            expect(response.body).to_not include(status_tag_for(status_reblog.reblog))
+            expect(response.body).to_not include(status_tag_for(status_reply))
+            expect(response.body).to_not include(status_tag_for(status_self_reply))
+            expect(response.body).to_not include(status_tag_for(status))
           end
         end
 
@@ -270,30 +273,29 @@ RSpec.describe AccountsController do
           let!(:status_tag) { Fabricate(:status, account: account) }
 
           before do
-            allow(controller).to receive(:tag_requested?).and_return(true)
             status_tag.tags << tag
-            get :show, params: { username: account.username, format: format, tag: tag.to_param }
+            get short_account_tag_path(username: account.username, tag: tag, format: format)
           end
 
           it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie'
 
           it 'responds with correct statuses with a tag', :aggregate_failures do
             expect(response).to have_http_status(200)
-            expect(response.body).to include_status_tag(status_tag)
-            expect(response.body).to_not include_status_tag(status_direct)
-            expect(response.body).to_not include_status_tag(status_media)
-            expect(response.body).to_not include_status_tag(status_private)
-            expect(response.body).to_not include_status_tag(status_reblog.reblog)
-            expect(response.body).to_not include_status_tag(status_reply)
-            expect(response.body).to_not include_status_tag(status_self_reply)
-            expect(response.body).to_not include_status_tag(status)
+            expect(response.body).to include(status_tag_for(status_tag))
+            expect(response.body).to_not include(status_tag_for(status_direct))
+            expect(response.body).to_not include(status_tag_for(status_media))
+            expect(response.body).to_not include(status_tag_for(status_private))
+            expect(response.body).to_not include(status_tag_for(status_reblog.reblog))
+            expect(response.body).to_not include(status_tag_for(status_reply))
+            expect(response.body).to_not include(status_tag_for(status_self_reply))
+            expect(response.body).to_not include(status_tag_for(status))
           end
         end
       end
     end
   end
 
-  def include_status_tag(status)
-    include ActivityPub::TagManager.instance.url_for(status)
+  def status_tag_for(status)
+    ActivityPub::TagManager.instance.url_for(status)
   end
 end