From 367fb1c524cff06a33c7a4144cd13a270a9f3489 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 26 Sep 2017 18:34:54 +0800 Subject: [PATCH] FIX: Onebox fails on encoded URL. https://meta.discourse.org/t/onebox-breaks-if-theres-chinese-text-in-url/67364 --- app/models/topic_embed.rb | 6 ++++-- lib/final_destination.rb | 5 ++++- spec/components/final_destination_spec.rb | 13 +++++++++++-- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/app/models/topic_embed.rb b/app/models/topic_embed.rb index 279ea3bfaf2..ad0e407d802 100644 --- a/app/models/topic_embed.rb +++ b/app/models/topic_embed.rb @@ -30,8 +30,10 @@ class TopicEmbed < ActiveRecord::Base # Prevents double URL encode # https://stackoverflow.com/a/37599235 - def self.escape_uri(uri) - URI.encode(uri).gsub(DOUBLE_ESCAPED_EXPR, '%\1') + def self.escape_uri(uri, pattern = URI::UNSAFE) + encoded = URI.encode(uri, pattern) + encoded.gsub!(DOUBLE_ESCAPED_EXPR, '%\1') + encoded end # Import an article from a source (RSS/Atom/Other) diff --git a/lib/final_destination.rb b/lib/final_destination.rb index 2e55f662e29..f74ff6cca0f 100644 --- a/lib/final_destination.rb +++ b/lib/final_destination.rb @@ -180,7 +180,10 @@ class FinalDestination end def escape_url - URI.escape(CGI.unescapeHTML(@url), Regexp.new("[^#{URI::PATTERN::UNRESERVED}#{URI::PATTERN::RESERVED}#]")) + TopicEmbed.escape_uri( + CGI.unescapeHTML(@url), + Regexp.new("[^#{URI::PATTERN::UNRESERVED}#{URI::PATTERN::RESERVED}#]") + ) end def private_ranges diff --git a/spec/components/final_destination_spec.rb b/spec/components/final_destination_spec.rb index 8c58a6a2a6a..e198b1e766b 100644 --- a/spec/components/final_destination_spec.rb +++ b/spec/components/final_destination_spec.rb @@ -301,13 +301,22 @@ describe FinalDestination do end end - describe ".escape_url" do + describe "#escape_url" do it "correctly escapes url" do fragment_url = "https://eviltrout.com/2016/02/25/fixing-android-performance.html#discourse-comments" expect(fd(fragment_url).escape_url.to_s).to eq(fragment_url) - expect(fd("https://eviltrout.com?s=180&d=mm&r=g").escape_url.to_s).to eq("https://eviltrout.com?s=180&d=mm&r=g") + + expect(fd("https://eviltrout.com?s=180&d=mm&r=g").escape_url.to_s) + .to eq("https://eviltrout.com?s=180&d=mm&r=g") + expect(fd("http://example.com/?a=\11\15").escape_url.to_s).to eq("http://example.com/?a=%09%0D") + + expect(fd("https://ru.wikipedia.org/wiki/%D0%A1%D0%B2%D0%BE%D0%B1%D0%BE").escape_url.to_s) + .to eq('https://ru.wikipedia.org/wiki/%D0%A1%D0%B2%D0%BE%D0%B1%D0%BE') + + expect(fd('https://ru.wikipedia.org/wiki/Свобо').escape_url.to_s) + .to eq('https://ru.wikipedia.org/wiki/%D0%A1%D0%B2%D0%BE%D0%B1%D0%BE') end end