Merge commit '16dd3f08c1e5396d5f9ff3f13417901bc4e4b8b9' into glitch-soc/merge-upstream

Conflicts:
- `app/views/settings/preferences/appearance/show.html.haml`:
  Upstream fixed a translation bug in the theme selector that is absent from
  glitch-soc due to our different theming system.
  Discarded upstream changes.
- `streaming/index.js`:
  Upstream changed the signature of a function to change its return type.
  This is not a real conflict, the conflict being caused by an extra
  argument in glitch-soc's code.
  Applied upstream's change while keeping our extra argument.

Gemfile.lock

@@ -882,3 +882,9 @@ DEPENDENCIES
   webpacker (~> 5.4)
   webpush!
   xorcist (~> 1.1)
+
+RUBY VERSION
+   ruby 3.2.2p53
+
+BUNDLED WITH
+   2.4.13

app/controllers/admin/dashboard_controller.rb

@@ -14,15 +14,5 @@ module Admin
       @pending_tags_count    = Tag.pending_review.count
       @pending_appeals_count = Appeal.pending.count
     end
-
-    private
-
-    def redis_info
-      @redis_info ||= if redis.is_a?(Redis::Namespace)
-                        redis.redis.info
-                      else
-                        redis.info
-                      end
-    end
   end
 end

spec/requests/api/v1/admin/email_domain_blocks_spec.rb

@@ -2,23 +2,20 @@
 
 require 'rails_helper'
 
-describe Api::V1::Admin::EmailDomainBlocksController do
+RSpec.describe 'Email Domain Blocks' do
-  render_views
-
   let(:role)    { UserRole.find_by(name: 'Admin') }
   let(:user)    { Fabricate(:user, role: role) }
   let(:token)   { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) }
   let(:account) { Fabricate(:account) }
   let(:scopes)  { 'admin:read:email_domain_blocks admin:write:email_domain_blocks' }
-
+  let(:headers) { { 'Authorization' => "Bearer #{token.token}" } }
-  before do
-    allow(controller).to receive(:doorkeeper_token) { token }
-  end
 
   shared_examples 'forbidden for wrong scope' do |wrong_scope|
     let(:scopes) { wrong_scope }
 
     it 'returns http forbidden' do
+      subject
+
       expect(response).to have_http_status(403)
     end
   end
@@ -27,65 +24,54 @@ describe Api::V1::Admin::EmailDomainBlocksController do
     let(:role) { UserRole.find_by(name: wrong_role) }
 
     it 'returns http forbidden' do
+      subject
+
       expect(response).to have_http_status(403)
     end
   end
 
-  describe 'GET #index' do
+  describe 'GET /api/v1/admin/email_domain_blocks' do
-    context 'with wrong scope' do
+    subject do
-      before do
+      get '/api/v1/admin/email_domain_blocks', headers: headers, params: params
-        get :index
-      end
-
-      it_behaves_like 'forbidden for wrong scope', 'read:statuses'
     end
 
-    context 'with wrong role' do
+    let(:params) { {} }
-      before do
-        get :index
-      end
 
-      it_behaves_like 'forbidden for wrong role', ''
+    it_behaves_like 'forbidden for wrong scope', 'read:statuses'
-      it_behaves_like 'forbidden for wrong role', 'Moderator'
+    it_behaves_like 'forbidden for wrong role', ''
-    end
+    it_behaves_like 'forbidden for wrong role', 'Moderator'
 
     it 'returns http success' do
-      get :index
+      subject
 
       expect(response).to have_http_status(200)
     end
 
     context 'when there is no email domain block' do
       it 'returns an empty list' do
-        get :index
+        subject
 
-        json = body_as_json
+        expect(body_as_json).to be_empty
-
-        expect(json).to be_empty
       end
     end
 
     context 'when there are email domain blocks' do
-      let!(:email_domain_blocks) { Fabricate.times(5, :email_domain_block) }
+      let!(:email_domain_blocks)  { Fabricate.times(5, :email_domain_block) }
       let(:blocked_email_domains) { email_domain_blocks.pluck(:domain) }
 
       it 'return the correct blocked email domains' do
-        get :index
+        subject
 
-        json = body_as_json
+        expect(body_as_json.pluck(:domain)).to match_array(blocked_email_domains)
-
-        expect(json.pluck(:domain)).to match_array(blocked_email_domains)
       end
 
       context 'with limit param' do
         let(:params) { { limit: 2 } }
 
         it 'returns only the requested number of email domain blocks' do
-          get :index, params: params
+          subject
 
-          json = body_as_json
+          expect(body_as_json.size).to eq(params[:limit])
-
-          expect(json.size).to eq(params[:limit])
         end
       end
 
@@ -93,12 +79,11 @@ describe Api::V1::Admin::EmailDomainBlocksController do
         let(:params) { { since_id: email_domain_blocks[1].id } }
 
         it 'returns only the email domain blocks after since_id' do
-          get :index, params: params
+          subject
 
           email_domain_blocks_ids = email_domain_blocks.pluck(:id).map(&:to_s)
-          json = body_as_json
 
-          expect(json.pluck(:id)).to match_array(email_domain_blocks_ids[2..])
+          expect(body_as_json.pluck(:id)).to match_array(email_domain_blocks_ids[2..])
         end
       end
 
@@ -106,102 +91,78 @@ describe Api::V1::Admin::EmailDomainBlocksController do
         let(:params) { { max_id: email_domain_blocks[3].id } }
 
         it 'returns only the email domain blocks before max_id' do
-          get :index, params: params
+          subject
 
           email_domain_blocks_ids = email_domain_blocks.pluck(:id).map(&:to_s)
-          json = body_as_json
 
-          expect(json.pluck(:id)).to match_array(email_domain_blocks_ids[..2])
+          expect(body_as_json.pluck(:id)).to match_array(email_domain_blocks_ids[..2])
         end
       end
     end
   end
 
-  describe 'GET #show' do
+  describe 'GET /api/v1/admin/email_domain_blocks/:id' do
+    subject do
+      get "/api/v1/admin/email_domain_blocks/#{email_domain_block.id}", headers: headers
+    end
+
     let!(:email_domain_block) { Fabricate(:email_domain_block) }
-    let(:params) { { id: email_domain_block.id } }
 
-    context 'with wrong scope' do
+    it_behaves_like 'forbidden for wrong scope', 'read:statuses'
-      before do
+    it_behaves_like 'forbidden for wrong role', ''
-        get :show, params: params
+    it_behaves_like 'forbidden for wrong role', 'Moderator'
-      end
-
-      it_behaves_like 'forbidden for wrong scope', 'read:statuses'
-    end
-
-    context 'with wrong role' do
-      before do
-        get :show, params: params
-      end
-
-      it_behaves_like 'forbidden for wrong role', ''
-      it_behaves_like 'forbidden for wrong role', 'Moderator'
-    end
 
     context 'when email domain block exists' do
       it 'returns http success' do
-        get :show, params: params
+        subject
 
         expect(response).to have_http_status(200)
       end
 
       it 'returns the correct blocked domain' do
-        get :show, params: params
+        subject
 
-        json = body_as_json
+        expect(body_as_json[:domain]).to eq(email_domain_block.domain)
-
-        expect(json[:domain]).to eq(email_domain_block.domain)
       end
     end
 
     context 'when email domain block does not exist' do
       it 'returns http not found' do
-        get :show, params: { id: 0 }
+        get '/api/v1/admin/email_domain_blocks/-1', headers: headers
 
         expect(response).to have_http_status(404)
       end
     end
   end
 
-  describe 'POST #create' do
+  describe 'POST /api/v1/admin/email_domain_blocks' do
+    subject do
+      post '/api/v1/admin/email_domain_blocks', headers: headers, params: params
+    end
+
     let(:params) { { domain: 'example.com' } }
 
-    context 'with wrong scope' do
+    it_behaves_like 'forbidden for wrong scope', 'read:statuses'
-      before do
+    it_behaves_like 'forbidden for wrong role', ''
-        post :create, params: params
+    it_behaves_like 'forbidden for wrong role', 'Moderator'
-      end
-
-      it_behaves_like 'forbidden for wrong scope', 'read:statuses'
-    end
-
-    context 'with wrong role' do
-      before do
-        post :create, params: params
-      end
-
-      it_behaves_like 'forbidden for wrong role', ''
-      it_behaves_like 'forbidden for wrong role', 'Moderator'
-    end
 
     it 'returns http success' do
-      post :create, params: params
+      subject
 
       expect(response).to have_http_status(200)
     end
 
     it 'returns the correct blocked email domain' do
-      post :create, params: params
+      subject
 
-      json = body_as_json
+      expect(body_as_json[:domain]).to eq(params[:domain])
-
-      expect(json[:domain]).to eq(params[:domain])
     end
 
     context 'when domain param is not provided' do
       let(:params) { { domain: '' } }
 
       it 'returns http unprocessable entity' do
-        post :create, params: params
+        subject
 
         expect(response).to have_http_status(422)
       end
@@ -211,7 +172,7 @@ describe Api::V1::Admin::EmailDomainBlocksController do
       let(:params) { { domain: 'do\uD800.com' } }
 
       it 'returns http unprocessable entity' do
-        post :create, params: params
+        subject
 
         expect(response).to have_http_status(422)
       end
@@ -223,59 +184,45 @@ describe Api::V1::Admin::EmailDomainBlocksController do
       end
 
       it 'returns http unprocessable entity' do
-        post :create, params: params
+        subject
 
         expect(response).to have_http_status(422)
       end
     end
   end
 
-  describe 'DELETE #destroy' do
+  describe 'DELETE /api/v1/admin/email_domain_blocks' do
+    subject do
+      delete "/api/v1/admin/email_domain_blocks/#{email_domain_block.id}", headers: headers
+    end
+
     let!(:email_domain_block) { Fabricate(:email_domain_block) }
-    let(:params) { { id: email_domain_block.id } }
 
-    context 'with wrong scope' do
+    it_behaves_like 'forbidden for wrong scope', 'read:statuses'
-      before do
+    it_behaves_like 'forbidden for wrong role', ''
-        delete :destroy, params: params
+    it_behaves_like 'forbidden for wrong role', 'Moderator'
-      end
-
-      it_behaves_like 'forbidden for wrong scope', 'read:statuses'
-    end
-
-    context 'with wrong role' do
-      before do
-        delete :destroy, params: params
-      end
-
-      it_behaves_like 'forbidden for wrong role', ''
-      it_behaves_like 'forbidden for wrong role', 'Moderator'
-    end
 
     it 'returns http success' do
-      delete :destroy, params: params
+      subject
 
       expect(response).to have_http_status(200)
     end
 
     it 'returns an empty body' do
-      delete :destroy, params: params
+      subject
 
-      json = body_as_json
+      expect(body_as_json).to be_empty
-
-      expect(json).to be_empty
     end
 
     it 'deletes email domain block' do
-      delete :destroy, params: params
+      subject
 
-      email_domain_block = EmailDomainBlock.find_by(id: params[:id])
+      expect(EmailDomainBlock.find_by(id: email_domain_block.id)).to be_nil
-
-      expect(email_domain_block).to be_nil
     end
 
     context 'when email domain block does not exist' do
       it 'returns http not found' do
-        delete :destroy, params: { id: 0 }
+        delete '/api/v1/admin/email_domain_blocks/-1', headers: headers
 
         expect(response).to have_http_status(404)
       end

streaming/index.js

@@ -52,18 +52,31 @@ const redisUrlToClient = async (defaultConfig, redisUrl) => {
 };
 
 /**
+ * Attempts to safely parse a string as JSON, used when both receiving a message
+ * from redis and when receiving a message from a client over a websocket
+ * connection, this is why it accepts a `req` argument.
  * @param {string} json
- * @param {any} req
+ * @param {any?} req
  * @returns {Object.<string, any>|null}
  */
 const parseJSON = (json, req) => {
   try {
     return JSON.parse(json);
   } catch (err) {
-    if (req.accountId) {
+    /* FIXME: This logging isn't great, and should probably be done at the
-      log.warn(req.requestId, `Error parsing message from user ${req.accountId}: ${err}`);
+     * call-site of parseJSON, not in the method, but this would require changing
+     * the signature of parseJSON to return something akin to a Result type:
+     * [Error|null, null|Object<string,any}], and then handling the error
+     * scenarios.
+     */
+    if (req) {
+      if (req.accountId) {
+        log.warn(req.requestId, `Error parsing message from user ${req.accountId}: ${err}`);
+      } else {
+        log.silly(req.requestId, `Error parsing message from ${req.remoteAddress}: ${err}`);
+      }
     } else {
-      log.silly(req.requestId, `Error parsing message from ${req.remoteAddress}: ${err}`);
+      log.warn(`Error parsing message from redis: ${err}`);
     }
     return null;
   }
@@ -163,7 +176,7 @@ const startServer = async () => {
   const { redisParams, redisUrl, redisPrefix } = redisConfigFromEnv(process.env);
 
   /**
-   * @type {Object.<string, Array.<function(string): void>>}
+   * @type {Object.<string, Array.<function(Object<string, any>): void>>}
    */
   const subs = {};
 
@@ -203,7 +216,10 @@ const startServer = async () => {
       return;
     }
 
-    callbacks.forEach(callback => callback(message));
+    const json = parseJSON(message, null);
+    if (!json) return;
+
+    callbacks.forEach(callback => callback(json));
   };
 
   /**
@@ -225,7 +241,7 @@ const startServer = async () => {
 
   /**
    * @param {string} channel
-   * @param {function(string): void} callback
+   * @param {function(Object<string, any>): void} callback
    */
   const unsubscribe = (channel, callback) => {
     log.silly(`Removing listener for ${channel}`);
@@ -369,7 +385,7 @@ const startServer = async () => {
 
   /**
    * @param {any} req
-   * @returns {string}
+   * @returns {string|undefined}
    */
   const channelNameFromPath = req => {
     const { path, query } = req;
@@ -478,15 +494,11 @@ const startServer = async () => {
   /**
    * @param {any} req
    * @param {SystemMessageHandlers} eventHandlers
-   * @returns {function(string): void}
+   * @returns {function(object): void}
    */
   const createSystemMessageListener = (req, eventHandlers) => {
     return message => {
-      const json = parseJSON(message, req);
+      const { event } = message;
-
-      if (!json) return;
-
-      const { event } = json;
 
       log.silly(req.requestId, `System message for ${req.accountId}: ${event}`);
 
@@ -604,19 +616,16 @@ const startServer = async () => {
    * @param {function(string[], function(string): void): void} attachCloseHandler
    * @param {boolean=} needsFiltering
    * @param {boolean=} allowLocalOnly
-   * @returns {function(string): void}
+   * @returns {function(object): void}
    */
   const streamFrom = (ids, req, output, attachCloseHandler, needsFiltering = false, allowLocalOnly = false) => {
     const accountId = req.accountId || req.remoteAddress;
 
     log.verbose(req.requestId, `Starting stream from ${ids.join(', ')} for ${accountId}`);
 
+    // Currently message is of type string, soon it'll be Record<string, any>
     const listener = message => {
-      const json = parseJSON(message, req);
+      const { event, payload, queued_at } = message;
-
-      if (!json) return;
-
-      const { event, payload, queued_at } = json;
 
       const transmit = () => {
         const now = new Date().getTime();
@@ -1219,8 +1228,15 @@ const startServer = async () => {
     ws.on('close', onEnd);
     ws.on('error', onEnd);
 
-    ws.on('message', data => {
+    ws.on('message', (data, isBinary) => {
-      const json = parseJSON(data, session.request);
+      if (isBinary) {
+        log.debug('Received binary data, closing connection');
+        ws.close(1003, 'The mastodon streaming server does not support binary messages');
+        return;
+      }
+      const message = data.toString('utf8');
+
+      const json = parseJSON(message, session.request);
 
       if (!json) return;