From 932a2fe41973b57519630f1be5a2ca8d5d08eea2 Mon Sep 17 00:00:00 2001 From: Andrei Prigorshnev Date: Wed, 2 Jun 2021 15:27:04 +0400 Subject: [PATCH] FIX: PG::StringDataRightTruncation when linking posts (#13134) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Users who use encoded slugs on their sites sometimes run into 500 error when pasting a link to another topic in a post. The problem happens when generating a backward "reflection" link that would appear in a linked topic. Link URL restricted on the database level to 500 chars in length. At first glance, it should work since we have a restriction on topic title length. But it doesn't work when a site uses encoded slugs, like here (take a look at the URL). The link to a topic, in this case, can be much longer than 500 characters. By the way, an error happens only when generating a "reflection" link and doesn't happen with a direct link, we truncate that link. It works because, in this case, the original long link is still present in the post body and can be used for navigation. But we can't do the same for backward "reflection" links (without rewriting their implementation), the whole link must be saved to the database. The simplest and cleanest solution will be just to remove the restriction on the database level. Abuse is impossible here since we are already protected by the restriction on topic title length. There aren’t performance benefits in using length-constrained columns in Postgres, in fact, length-constrained columns need a few extra CPU cycles to check the length when storing data. --- app/models/topic_link.rb | 2 +- ...ve_length_constrain_from_topic_link_url.rb | 11 ++++++ spec/models/topic_link_spec.rb | 36 +++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 db/post_migrate/20210525112226_remove_length_constrain_from_topic_link_url.rb diff --git a/app/models/topic_link.rb b/app/models/topic_link.rb index 1770ea7ed48..6758689c712 100644 --- a/app/models/topic_link.rb +++ b/app/models/topic_link.rb @@ -386,7 +386,7 @@ end # topic_id :integer not null # post_id :integer # user_id :integer not null -# url :string(500) not null +# url :string not null # domain :string(100) not null # internal :boolean default(FALSE), not null # link_topic_id :integer diff --git a/db/post_migrate/20210525112226_remove_length_constrain_from_topic_link_url.rb b/db/post_migrate/20210525112226_remove_length_constrain_from_topic_link_url.rb new file mode 100644 index 00000000000..f7d9c606f20 --- /dev/null +++ b/db/post_migrate/20210525112226_remove_length_constrain_from_topic_link_url.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class RemoveLengthConstrainFromTopicLinkUrl < ActiveRecord::Migration[6.1] + def up + change_column :topic_links, :url, :string, null: false + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/spec/models/topic_link_spec.rb b/spec/models/topic_link_spec.rb index b6f6f1c9790..6e52b74c231 100644 --- a/spec/models/topic_link_spec.rb +++ b/spec/models/topic_link_spec.rb @@ -201,6 +201,42 @@ describe TopicLink do expect(other_topic.reload.topic_links.where(link_post_id: linked_post.id)).to be_blank end + + it 'truncates long links' do + SiteSetting.slug_generation_method = 'encoded' + long_title = "Καλημερα σε ολους και ολες" * 9 # 234 chars, but the encoded slug will be 1224 chars in length + other_topic = Fabricate(:topic, user: user, title: long_title) + expect(other_topic.slug.length).to be > TopicLink.max_url_length + other_topic.posts.create(user: user, raw: 'initial post') + other_topic_url = "http://#{test_uri.host}/t/#{other_topic.slug}/#{other_topic.id}" + + post_with_link = topic.posts.create(user: user, raw: "Link to another topic: #{other_topic_url}") + TopicLink.extract_from(post_with_link) + topic.reload + link = topic.topic_links.first + + expect(link.url.length).to eq(TopicLink.max_url_length) + end + + it 'does not truncate reflection links' do + SiteSetting.slug_generation_method = 'encoded' + long_title = "Καλημερα σε ολους και ολες" * 9 # 234 chars, but the encoded slug will be 1224 chars in length + topic = Fabricate(:topic, user: user, title: long_title) + expect(topic.slug.length).to be > TopicLink.max_url_length + topic_url = "http://#{test_uri.host}/t/#{topic.slug}/#{topic.id}" + + other_topic = Fabricate(:topic, user: user) + other_topic.posts.create(user: user, raw: 'initial post') + other_topic_url = "http://#{test_uri.host}/t/#{other_topic.slug}/#{other_topic.id}" + + post_with_link = topic.posts.create(user: user, raw: "Link to another topic: #{other_topic_url}") + expect { TopicLink.extract_from(post_with_link) }.to_not raise_error + + other_topic.reload + reflection_link = other_topic.topic_links.first + expect(reflection_link.url.length).to be > (TopicLink.max_url_length) + expect(reflection_link.url).to eq(topic_url) + end end context "link to a user on discourse" do