github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-01-31 17:06:30 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

FIX: do not escape already escaped chars in URL

Browse Source
This commit is contained in:
Gerhard Schlager 2017-09-22 17:36:44 +02:00
parent 1a435414d5
commit 6f6b47f096
2 changed files with 56 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
app/models/topic_embed.rb
View File

@ -26,6 +26,14 @@ class TopicEmbed < ActiveRecord::Base
"\n<hr>\n<small>#{I18n.t('embed.imported_from', link: "<a href='#{url}'>#{url}</a>")}</small>\n" "\n<hr>\n<small>#{I18n.t('embed.imported_from', link: "<a href='#{url}'>#{url}</a>")}</small>\n"
end end
DOUBLE_ESCAPED_EXPR = /%25([0-9a-f]{2})/i
# Prevents double URL encode
# https://stackoverflow.com/a/37599235
def self.escape_uri(uri)
URI.encode(uri).gsub(DOUBLE_ESCAPED_EXPR, '%\1')
end
# Import an article from a source (RSS/Atom/Other) # Import an article from a source (RSS/Atom/Other)
def self.import(user, url, title, contents) def self.import(user, url, title, contents)
return unless url =~ /^https?\:\/\// return unless url =~ /^https?\:\/\//
@ -77,7 +85,8 @@ class TopicEmbed < ActiveRecord::Base
def self.find_remote(url) def self.find_remote(url)
require 'ruby-readability' require 'ruby-readability'
original_uri = URI.parse(URI.encode(url)) url = escape_uri(url)
original_uri = URI.parse(url)
opts = { opts = {
tags: %w[div p code pre h1 h2 h3 b em i strong a img ul li ol blockquote], tags: %w[div p code pre h1 h2 h3 b em i strong a img ul li ol blockquote],
attributes: %w[href src class], attributes: %w[href src class],
@ -90,7 +99,7 @@ class TopicEmbed < ActiveRecord::Base
response = FetchResponse.new response = FetchResponse.new
begin begin
html = open(URI.encode(url), allow_redirections: :safe).read html = open(url, allow_redirections: :safe).read
rescue OpenURI::HTTPError, Net::OpenTimeout rescue OpenURI::HTTPError, Net::OpenTimeout
return return
end end
@ -119,7 +128,7 @@ class TopicEmbed < ActiveRecord::Base
src = node[url_param] src = node[url_param]
unless (src.nil? || src.empty?) unless (src.nil? || src.empty?)
begin begin
uri = URI.parse(URI.encode(src)) uri = URI.parse(escape_uri(src))
unless uri.host unless uri.host
uri.scheme = original_uri.scheme uri.scheme = original_uri.scheme
uri.host = original_uri.host uri.host = original_uri.host
@ -148,6 +157,8 @@ class TopicEmbed < ActiveRecord::Base
def self.import_remote(import_user, url, opts = nil) def self.import_remote(import_user, url, opts = nil)
opts = opts || {} opts = opts || {}
response = find_remote(url) response = find_remote(url)
return if response.nil?
response.title = opts[:title] if opts[:title].present? response.title = opts[:title] if opts[:title].present?
import_user = response.author if response.author.present? import_user = response.author if response.author.present?
@ -157,7 +168,7 @@ class TopicEmbed < ActiveRecord::Base
# Convert any relative URLs to absolute. RSS is annoying for this. # Convert any relative URLs to absolute. RSS is annoying for this.
def self.absolutize_urls(url, contents) def self.absolutize_urls(url, contents)
url = normalize_url(url) url = normalize_url(url)
uri = URI(URI.encode(url)) uri = URI(escape_uri(url))
prefix = "#{uri.scheme}://#{uri.host}" prefix = "#{uri.scheme}://#{uri.host}"
prefix << ":#{uri.port}" if uri.port != 80 && uri.port != 443 prefix << ":#{uri.port}" if uri.port != 80 && uri.port != 443

42
spec/models/topic_embed_spec.rb
View File

@ -186,7 +186,8 @@ describe TopicEmbed do
before do before do
file.stubs(:read).returns contents file.stubs(:read).returns contents
TopicEmbed.stubs(:open).returns file TopicEmbed.stubs(:open)
.with('http://eviltrout.com/test/%D9%85%D8%A7%D9%87%DB%8C', allow_redirections: :safe).returns file
end end
it "doesn't throw an error" do it "doesn't throw an error" do
@ -195,6 +196,24 @@ describe TopicEmbed do
end end
end end
context "encoded URL" do
let(:url) { 'http://example.com/hello%20world' }
let(:contents) { "<title>Hello World!</title><body></body>" }
let!(:embeddable_host) { Fabricate(:embeddable_host) }
let!(:file) { StringIO.new }
before do
file.stubs(:read).returns contents
TopicEmbed.stubs(:open)
.with('http://example.com/hello%20world', allow_redirections: :safe).returns file
end
it "doesn't throw an error" do
response = TopicEmbed.find_remote(url)
expect(response.title).to eq("Hello World!")
end
end
context "emails" do context "emails" do
let(:url) { 'http://example.com/foo' } let(:url) { 'http://example.com/foo' }
let(:contents) { '<p><a href="mailto:foo%40example.com">URL encoded @ symbol</a></p><p><a href="mailto:bar@example.com">normal mailto link</a></p>' } let(:contents) { '<p><a href="mailto:foo%40example.com">URL encoded @ symbol</a></p><p><a href="mailto:bar@example.com">normal mailto link</a></p>' }
@ -214,4 +233,25 @@ describe TopicEmbed do
end end
end end
context ".escape_uri" do
it "doesn't escape simple URL" do
url = TopicEmbed.escape_uri('http://example.com/foo/bar')
expect(url).to eq('http://example.com/foo/bar')
end
it "escapes unsafe chars" do
url = TopicEmbed.escape_uri("http://example.com/?a=\11\15")
expect(url).to eq('http://example.com/?a=%09%0D')
end
it "escapes non-ascii chars" do
url = TopicEmbed.escape_uri('http://example.com/ماهی')
expect(url).to eq('http://example.com/%D9%85%D8%A7%D9%87%DB%8C')
end
it "doesn't escape already escaped chars" do
url = TopicEmbed.escape_uri('http://example.com/foo%20bar/foo bar/')
expect(url).to eq('http://example.com/foo%20bar/foo%20bar/')
end
end
end end