diff --git a/app/controllers/drafts_controller.rb b/app/controllers/drafts_controller.rb index 78d1ea4fad6..bf7e82881c6 100644 --- a/app/controllers/drafts_controller.rb +++ b/app/controllers/drafts_controller.rb @@ -24,6 +24,16 @@ class DraftsController < ApplicationController def create raise Discourse::NotFound.new if params[:draft_key].blank? + if params[:data].size > SiteSetting.max_draft_length + raise Discourse::InvalidParameters.new(:data) + end + + begin + data = JSON.parse(params[:data]) + rescue JSON::ParserError + raise Discourse::InvalidParameters.new(:data) + end + sequence = begin Draft.set( @@ -59,12 +69,6 @@ class DraftsController < ApplicationController json = success_json.merge(draft_sequence: sequence) - begin - data = JSON.parse(params[:data]) - rescue JSON::ParserError - raise Discourse::InvalidParameters.new(:data) - end - if data.present? # this is a bit of a kludge we need to remove (all the parsing) too many special cases here # we need to catch action edit and action editSharedDraft diff --git a/config/site_settings.yml b/config/site_settings.yml index a39482aec90..e27f702b113 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1090,6 +1090,9 @@ posting: skip_auto_delete_reply_likes: 5 review_every_post: default: false + max_draft_length: + default: 400_000 + hidden: true email: email_time_window_mins: diff --git a/db/post_migrate/20230117143451_drop_invalid_drafts.rb b/db/post_migrate/20230117143451_drop_invalid_drafts.rb new file mode 100644 index 00000000000..dd44c4c7ecc --- /dev/null +++ b/db/post_migrate/20230117143451_drop_invalid_drafts.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class DropInvalidDrafts < ActiveRecord::Migration[7.0] + def up + execute <<~SQL + DELETE FROM drafts + WHERE LENGTH(data) > 400000 + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/spec/requests/drafts_controller_spec.rb b/spec/requests/drafts_controller_spec.rb index 9cb50febee3..4af1fd795df 100644 --- a/spec/requests/drafts_controller_spec.rb +++ b/spec/requests/drafts_controller_spec.rb @@ -192,6 +192,40 @@ RSpec.describe DraftsController do expect(response.status).to eq(409) end + + context "when data is too big" do + let(:user) { Fabricate(:user) } + let(:data) { "a" * (SiteSetting.max_draft_length + 1) } + + before do + SiteSetting.max_draft_length = 500 + sign_in(user) + end + + it "returns an error" do + post "/drafts.json", + params: { + draft_key: "xyz", + data: { reply: data }.to_json, + sequence: 0, + } + expect(response).to have_http_status :bad_request + end + end + + context "when data is not too big" do + context "when data is not proper JSON" do + let(:user) { Fabricate(:user) } + let(:data) { "not-proper-json" } + + before { sign_in(user) } + + it "returns an error" do + post "/drafts.json", params: { draft_key: "xyz", data: data, sequence: 0 } + expect(response).to have_http_status :bad_request + end + end + end end describe "#destroy" do