FIX: PG::StringDataRightTruncation when linking posts (#13134)

FIX: PG::StringDataRightTruncation when linking posts (#13134)

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.

diff --git a/app/models/topic_link.rb b/app/models/topic_link.rb
index 1770ea7..6758689 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 0000000..f7d9c60
--- /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 b6f6f1c..6e52b74 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

GitHub sha: 932a2fe4

This commit appears in #13134 which was approved by SamSaffron. It was merged by AndrewPrigorshnev.