diff --git a/plugins/chat/app/controllers/incoming_chat_webhooks_controller.rb b/plugins/chat/app/controllers/incoming_chat_webhooks_controller.rb index 1cf4963621e..f528e59513a 100644 --- a/plugins/chat/app/controllers/incoming_chat_webhooks_controller.rb +++ b/plugins/chat/app/controllers/incoming_chat_webhooks_controller.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true class Chat::IncomingChatWebhooksController < ApplicationController - WEBHOOK_MAX_MESSAGE_LENGTH = 2000 WEBHOOK_MESSAGES_PER_MINUTE_LIMIT = 10 skip_before_action :verify_authenticity_token, :redirect_to_login_if_required @@ -80,9 +79,9 @@ class Chat::IncomingChatWebhooksController < ApplicationController end def validate_message_length(message) - return if message.length <= WEBHOOK_MAX_MESSAGE_LENGTH + return if message.length <= SiteSetting.chat_maximum_message_length raise Discourse::InvalidParameters.new( - "Body cannot be over #{WEBHOOK_MAX_MESSAGE_LENGTH} characters", + "Body cannot be over #{SiteSetting.chat_maximum_message_length} characters", ) end diff --git a/plugins/chat/app/models/chat_message.rb b/plugins/chat/app/models/chat_message.rb index 2a73b0f1791..4c1f7083287 100644 --- a/plugins/chat/app/models/chat_message.rb +++ b/plugins/chat/app/models/chat_message.rb @@ -48,6 +48,16 @@ class ChatMessage < ActiveRecord::Base ), ) end + + if message_too_long? + self.errors.add( + :base, + I18n.t( + "chat.errors.message_too_long", + maximum: SiteSetting.chat_maximum_message_length, + ), + ) + end end def attach_uploads(uploads) @@ -205,6 +215,10 @@ class ChatMessage < ActiveRecord::Base message.length < SiteSetting.chat_minimum_message_length end + def message_too_long? + message.length > SiteSetting.chat_maximum_message_length + end + def ensure_last_editor_id self.last_editor_id ||= self.user_id end diff --git a/plugins/chat/config/locales/server.en.yml b/plugins/chat/config/locales/server.en.yml index 66971e7d5a3..0a9838ce72f 100644 --- a/plugins/chat/config/locales/server.en.yml +++ b/plugins/chat/config/locales/server.en.yml @@ -48,6 +48,7 @@ en: duplicate_message: "You posted an identical message too recently." delete_channel_failed: "Delete channel failed, please try again." minimum_length_not_met: "Message is too short, must have a minimum of %{minimum} characters." + message_too_long: "Message is too long, messages must be a maximum of %{maximum} characters." max_reactions_limit_reached: "New reactions are not allowed on this message." message_move_invalid_channel: "The source and destination channel must be public channels." message_move_no_messages_found: "No messages were found with the provided message IDs." diff --git a/plugins/chat/config/settings.yml b/plugins/chat/config/settings.yml index bcf2177bc2b..be2d86cf9f6 100644 --- a/plugins/chat/config/settings.yml +++ b/plugins/chat/config/settings.yml @@ -75,6 +75,13 @@ chat: min: 1 max: 50 client: true + chat_maximum_message_length: + type: integer + default: 6000 + client: true + hidden: true + min: 100 + max: 12000 chat_allow_uploads: default: true client: true diff --git a/plugins/chat/db/post_migrate/20221117052348_truncate_chat_messages_over_max_length.rb b/plugins/chat/db/post_migrate/20221117052348_truncate_chat_messages_over_max_length.rb new file mode 100644 index 00000000000..60a0d80961e --- /dev/null +++ b/plugins/chat/db/post_migrate/20221117052348_truncate_chat_messages_over_max_length.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class TruncateChatMessagesOverMaxLength < ActiveRecord::Migration[7.0] + def up + if table_exists?(:chat_messages) + # 6000 is the default of the chat_maximum_message_length + # site setting, its safe to do this because this will be + # run the first time the setting is introduced. + execute <<~SQL + UPDATE chat_messages + SET message = LEFT(message, 6000), cooked_version = NULL + WHERE LENGTH(message) > 6000 + SQL + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/plugins/chat/spec/components/chat_message_creator_spec.rb b/plugins/chat/spec/components/chat_message_creator_spec.rb index bebdd302966..0520ce31d8b 100644 --- a/plugins/chat/spec/components/chat_message_creator_spec.rb +++ b/plugins/chat/spec/components/chat_message_creator_spec.rb @@ -72,6 +72,23 @@ describe Chat::ChatMessageCreator do ) end + it "errors when length is greater than `chat_maximum_message_length`" do + SiteSetting.chat_maximum_message_length = 100 + creator = + Chat::ChatMessageCreator.create( + chat_channel: public_chat_channel, + user: user1, + content: "a really long and in depth message that is just too detailed" * 100, + ) + expect(creator.failed?).to eq(true) + expect(creator.error.message).to match( + I18n.t( + "chat.errors.message_too_long", + { maximum: SiteSetting.chat_maximum_message_length }, + ), + ) + end + it "allows message creation when length is less than `chat_minimum_message_length` when upload is present" do upload = Fabricate(:upload, user: user1) SiteSetting.chat_minimum_message_length = 10 diff --git a/plugins/chat/spec/components/chat_message_updater_spec.rb b/plugins/chat/spec/components/chat_message_updater_spec.rb index d31e4bae8e1..d53bf0d7142 100644 --- a/plugins/chat/spec/components/chat_message_updater_spec.rb +++ b/plugins/chat/spec/components/chat_message_updater_spec.rb @@ -68,15 +68,38 @@ describe Chat::ChatMessageUpdater do expect(chat_message.reload.message).to eq(og_message) end + it "errors when length is greater than `chat_maximum_message_length`" do + SiteSetting.chat_maximum_message_length = 100 + og_message = "This won't be changed!" + chat_message = create_chat_message(user1, og_message, public_chat_channel) + new_message = "2 long" * 100 + + updater = + Chat::ChatMessageUpdater.update( + guardian: guardian, + chat_message: chat_message, + new_content: new_message, + ) + expect(updater.failed?).to eq(true) + expect(updater.error.message).to match( + I18n.t( + "chat.errors.message_too_long", + { maximum: SiteSetting.chat_maximum_message_length }, + ), + ) + expect(chat_message.reload.message).to eq(og_message) + end + it "errors if a user other than the message user is trying to edit the message" do og_message = "This won't be changed!" chat_message = create_chat_message(user1, og_message, public_chat_channel) new_message = "2 short" - updater = Chat::ChatMessageUpdater.update( - guardian: Guardian.new(Fabricate(:user)), - chat_message: chat_message, - new_content: new_message, - ) + updater = + Chat::ChatMessageUpdater.update( + guardian: Guardian.new(Fabricate(:user)), + chat_message: chat_message, + new_content: new_message, + ) expect(updater.failed?).to eq(true) expect(updater.error).to match(Discourse::InvalidAccess) end @@ -95,13 +118,14 @@ describe Chat::ChatMessageUpdater do it "publishes a DiscourseEvent for updated messages" do chat_message = create_chat_message(user1, "This will be changed", public_chat_channel) - events = DiscourseEvent.track_events { - Chat::ChatMessageUpdater.update( - guardian: guardian, - chat_message: chat_message, - new_content: "Change to this!", - ) - } + events = + DiscourseEvent.track_events do + Chat::ChatMessageUpdater.update( + guardian: guardian, + chat_message: chat_message, + new_content: "Change to this!", + ) + end expect(events.map { _1[:event_name] }).to include(:chat_message_edited) end diff --git a/plugins/chat/spec/requests/incoming_chat_webhooks_controller_spec.rb b/plugins/chat/spec/requests/incoming_chat_webhooks_controller_spec.rb index 448230c2b83..a8a54334718 100644 --- a/plugins/chat/spec/requests/incoming_chat_webhooks_controller_spec.rb +++ b/plugins/chat/spec/requests/incoming_chat_webhooks_controller_spec.rb @@ -19,10 +19,10 @@ RSpec.describe Chat::IncomingChatWebhooksController do expect(response.status).to eq(400) end - it "errors when the body is over WEBHOOK_MAX_MESSAGE_LENGTH characters" do + it "errors when the body is over chat_maximum_message_length characters" do post "/chat/hooks/#{webhook.key}.json", params: { - text: "$" * (Chat::IncomingChatWebhooksController::WEBHOOK_MAX_MESSAGE_LENGTH + 1), + text: "$" * (SiteSetting.chat_maximum_message_length + 1), } expect(response.status).to eq(400) end