Punycode URI normalization (#2370)

* Fix #2119 - Whenever about to send a HTTP request, normalize the URI

* Add test for IDN request in FetchLinkCardService

* Perform IDN normalization on domains before they are stored in the DB

15 files changed, 49 insertions, 26 deletions

diff --git a/app/controllers/api/push_controller.rb b/app/controllers/api/push_controller.rb
index f2ddfd969..bc547d653 100644
--- a/app/controllers/api/push_controller.rb
+++ b/app/controllers/api/push_controller.rb
@@ -26,7 +26,7 @@ class Api::PushController < ApiController
   def topic_to_account(topic_url)
     return if topic_url.blank?
 
-    uri    = Addressable::URI.parse(topic_url)
+    uri    = Addressable::URI.parse(topic_url).normalize
     params = Rails.application.routes.recognize_path(uri.path)
     domain = uri.host + (uri.port ? ":#{uri.port}" : '')
 
diff --git a/app/controllers/authorize_follow_controller.rb b/app/controllers/authorize_follow_controller.rb
index c98a5f45f..9b28a9455 100644
--- a/app/controllers/authorize_follow_controller.rb
+++ b/app/controllers/authorize_follow_controller.rb
@@ -6,7 +6,7 @@ class AuthorizeFollowController < ApplicationController
   before_action :authenticate_user!
 
   def new
-    uri = Addressable::URI.parse(acct_param)
+    uri = Addressable::URI.parse(acct_param).normalize
 
     if uri.path && %w(http https).include?(uri.scheme)
       set_account_from_url
diff --git a/app/lib/tag_manager.rb b/app/lib/tag_manager.rb
index f26c943d2..3bddfba7c 100644
--- a/app/lib/tag_manager.rb
+++ b/app/lib/tag_manager.rb
@@ -64,6 +64,12 @@ class TagManager
     domain.nil? || domain.gsub(/[\/]/, '').casecmp(Rails.configuration.x.local_domain).zero?
   end
 
+  def normalize_domain(domain)
+    uri = Addressable::URI.new
+    uri.host = domain
+    uri.normalize.host
+  end
+
   def same_acct?(canonical, needle)
     return true if canonical.casecmp(needle).zero?
     username, domain = needle.split('@')
@@ -71,7 +77,7 @@ class TagManager
   end
 
   def local_url?(url)
-    uri    = Addressable::URI.parse(url)
+    uri    = Addressable::URI.parse(url).normalize
     domain = uri.host + (uri.port ? ":#{uri.port}" : '')
     TagManager.instance.local_domain?(domain)
   end
diff --git a/app/models/account.rb b/app/models/account.rb
index 084b17f43..eebcf90b8 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -182,22 +182,22 @@ class Account < ApplicationRecord
   end
 
   def avatar_remote_url=(url)
-    parsed_url = URI.parse(url)
+    parsed_url = Addressable::URI.parse(url).normalize
 
     return if !%w(http https).include?(parsed_url.scheme) || parsed_url.host.empty? || self[:avatar_remote_url] == url
 
-    self.avatar              = parsed_url
+    self.avatar              = URI.parse(parsed_url.to_s)
     self[:avatar_remote_url] = url
   rescue OpenURI::HTTPError => e
     Rails.logger.debug "Error fetching remote avatar: #{e}"
   end
 
   def header_remote_url=(url)
-    parsed_url = URI.parse(url)
+    parsed_url = Addressable::URI.parse(url).normalize
 
     return if !%w(http https).include?(parsed_url.scheme) || parsed_url.host.empty? || self[:header_remote_url] == url
 
-    self.header              = parsed_url
+    self.header              = URI.parse(parsed_url.to_s)
     self[:header_remote_url] = url
   rescue OpenURI::HTTPError => e
     Rails.logger.debug "Error fetching remote header: #{e}"
@@ -331,16 +331,25 @@ class Account < ApplicationRecord
     end
   end
 
-  before_create do
-    if local?
-      keypair = OpenSSL::PKey::RSA.new(Rails.env.test? ? 1024 : 2048)
-      self.private_key = keypair.to_pem
-      self.public_key  = keypair.public_key.to_pem
-    end
-  end
+  before_create :generate_keys
+  before_validation :normalize_domain
 
   private
 
+  def generate_keys
+    return unless local?
+
+    keypair = OpenSSL::PKey::RSA.new(Rails.env.test? ? 1024 : 2048)
+    self.private_key = keypair.to_pem
+    self.public_key  = keypair.public_key.to_pem
+  end
+
+  def normalize_domain
+    return if local?
+
+    self.domain = TagManager.instance.normalize_domain(domain)
+  end
+
   def set_file_extensions
     unless avatar.blank?
       extension = Paperclip::Interpolations.content_type_extension(avatar, :original)
diff --git a/app/models/domain_block.rb b/app/models/domain_block.rb
index baf5c3973..6c2ba70f7 100644
--- a/app/models/domain_block.rb
+++ b/app/models/domain_block.rb
@@ -13,4 +13,12 @@ class DomainBlock < ApplicationRecord
   def self.blocked?(domain)
     where(domain: domain, severity: :suspend).exists?
   end
+
+  before_validation :normalize_domain
+
+  private
+
+  def normalize_domain
+    self.domain = TagManager.instance.normalize_domain(domain)
+  end
 end
diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb
index 181e01674..a43c76c77 100644
--- a/app/models/media_attachment.rb
+++ b/app/models/media_attachment.rb
@@ -42,7 +42,7 @@ class MediaAttachment < ApplicationRecord
   end
 
   def file_remote_url=(url)
-    self.file = URI.parse(url)
+    self.file = URI.parse(Addressable::URI.parse(url).normalize.to_s)
   end
 
   def to_param
diff --git a/app/services/fetch_link_card_service.rb b/app/services/fetch_link_card_service.rb
index 31f9be194..f74a0d34d 100644
--- a/app/services/fetch_link_card_service.rb
+++ b/app/services/fetch_link_card_service.rb
@@ -19,7 +19,7 @@ class FetchLinkCardService < BaseService
 
     card.title       = meta_property(page, 'og:title') || page.at_xpath('//title')&.content
     card.description = meta_property(page, 'og:description') || meta_property(page, 'description')
-    card.image       = URI.parse(meta_property(page, 'og:image')) if meta_property(page, 'og:image')
+    card.image       = URI.parse(Addressable::URI.parse(meta_property(page, 'og:image')).normalize.to_s) if meta_property(page, 'og:image')
 
     return if card.title.blank?
 
diff --git a/app/services/fetch_remote_account_service.rb b/app/services/fetch_remote_account_service.rb
index 50ffc47c6..7c2fb0ef1 100644
--- a/app/services/fetch_remote_account_service.rb
+++ b/app/services/fetch_remote_account_service.rb
@@ -21,7 +21,7 @@ class FetchRemoteAccountService < BaseService
 
     email = xml.at_xpath('//xmlns:author/xmlns:email').try(:content)
     if email.nil?
-      url_parts = Addressable::URI.parse(url)
+      url_parts = Addressable::URI.parse(url).normalize
       username  = xml.at_xpath('//xmlns:author/xmlns:name').try(:content)
       domain    = url_parts.host
     else
diff --git a/app/services/fetch_remote_status_service.rb b/app/services/fetch_remote_status_service.rb
index e2d185723..c666961ad 100644
--- a/app/services/fetch_remote_status_service.rb
+++ b/app/services/fetch_remote_status_service.rb
@@ -31,7 +31,7 @@ class FetchRemoteStatusService < BaseService
   end
 
   def extract_author(url, xml)
-    url_parts = Addressable::URI.parse(url)
+    url_parts = Addressable::URI.parse(url).normalize
     username  = xml.at_xpath('//xmlns:author/xmlns:name').try(:content)
     domain    = url_parts.host
 
diff --git a/app/services/follow_remote_account_service.rb b/app/services/follow_remote_account_service.rb
index 2d7414bc0..fbf983093 100644
--- a/app/services/follow_remote_account_service.rb
+++ b/app/services/follow_remote_account_service.rb
@@ -73,7 +73,7 @@ class FollowRemoteAccountService < BaseService
   end
 
   def get_feed(url)
-    response = http_client.get(Addressable::URI.parse(url))
+    response = http_client.get(Addressable::URI.parse(url).normalize)
     [response.to_s, Nokogiri::XML(response)]
   end
 
diff --git a/app/services/process_feed_service.rb b/app/services/process_feed_service.rb
index 98d92f630..d002b9130 100644
--- a/app/services/process_feed_service.rb
+++ b/app/services/process_feed_service.rb
@@ -174,7 +174,7 @@ class ProcessFeedService < BaseService
     end
 
     def account_from_href(href)
-      url = Addressable::URI.parse(href)
+      url = Addressable::URI.parse(href).normalize
 
       if TagManager.instance.web_domain?(url.host)
         Account.find_local(url.path.gsub('/users/', ''))
@@ -195,7 +195,7 @@ class ProcessFeedService < BaseService
         next unless link['href']
 
         media = MediaAttachment.where(status: parent, remote_url: link['href']).first_or_initialize(account: parent.account, status: parent, remote_url: link['href'])
-        parsed_url = URI.parse(link['href'])
+        parsed_url = Addressable::URI.parse(link['href']).normalize
 
         next if !%w[http https].include?(parsed_url.scheme) || parsed_url.host.empty?
 
@@ -271,7 +271,7 @@ class ProcessFeedService < BaseService
     def acct(xml = @xml)
       username = xml.at_xpath('./xmlns:author/xmlns:name', xmlns: TagManager::XMLNS).content
       url      = xml.at_xpath('./xmlns:author/xmlns:uri', xmlns: TagManager::XMLNS).content
-      domain   = Addressable::URI.parse(url).host
+      domain   = Addressable::URI.parse(url).normalize.host
 
       "#{username}@#{domain}"
     end
diff --git a/app/services/process_interaction_service.rb b/app/services/process_interaction_service.rb
index 805ca5a27..410e805d3 100644
--- a/app/services/process_interaction_service.rb
+++ b/app/services/process_interaction_service.rb
@@ -14,7 +14,7 @@ class ProcessInteractionService < BaseService
 
     username = xml.at_xpath('/xmlns:entry/xmlns:author/xmlns:name', xmlns: TagManager::XMLNS).content
     url      = xml.at_xpath('/xmlns:entry/xmlns:author/xmlns:uri', xmlns: TagManager::XMLNS).content
-    domain   = Addressable::URI.parse(url).host
+    domain   = Addressable::URI.parse(url).normalize.host
     account  = Account.find_by(username: username, domain: domain)
 
     if account.nil?
diff --git a/app/services/pubsubhubbub/subscribe_service.rb b/app/services/pubsubhubbub/subscribe_service.rb
index bf36e3fa6..3642b4eca 100644
--- a/app/services/pubsubhubbub/subscribe_service.rb
+++ b/app/services/pubsubhubbub/subscribe_service.rb
@@ -4,7 +4,7 @@ class Pubsubhubbub::SubscribeService < BaseService
   def call(account, callback, secret, lease_seconds)
     return ['Invalid topic URL',        422] if account.nil?
     return ['Invalid callback URL',     422] unless !callback.blank? && callback =~ /\A#{URI.regexp(%w(http https))}\z/
-    return ['Callback URL not allowed', 403] if DomainBlock.blocked?(Addressable::URI.parse(callback).host)
+    return ['Callback URL not allowed', 403] if DomainBlock.blocked?(Addressable::URI.parse(callback).normalize.host)
 
     subscription = Subscription.where(account: account, callback_url: callback).first_or_create!(account: account, callback_url: callback)
     Pubsubhubbub::ConfirmationWorker.perform_async(subscription.id, 'subscribe', secret, lease_seconds)
diff --git a/app/validators/url_validator.rb b/app/validators/url_validator.rb
index 4a5c4ef3f..f39560d90 100644
--- a/app/validators/url_validator.rb
+++ b/app/validators/url_validator.rb
@@ -8,7 +8,7 @@ class UrlValidator < ActiveModel::EachValidator
   private
 
   def compliant?(url)
-    parsed_url = Addressable::URI.parse(url)
+    parsed_url = Addressable::URI.parse(url).normalize
     !parsed_url.nil? && %w(http https).include?(parsed_url.scheme) && parsed_url.host
   end
 end
diff --git a/app/workers/pubsubhubbub/delivery_worker.rb b/app/workers/pubsubhubbub/delivery_worker.rb
index b440f7986..f645b1e24 100644
--- a/app/workers/pubsubhubbub/delivery_worker.rb
+++ b/app/workers/pubsubhubbub/delivery_worker.rb
@@ -13,7 +13,7 @@ class Pubsubhubbub::DeliveryWorker
   def perform(subscription_id, payload)
     subscription = Subscription.find(subscription_id)
     headers      = {}
-    host         = Addressable::URI.parse(subscription.callback_url).host
+    host         = Addressable::URI.parse(subscription.callback_url).normalize.host
 
     return if DomainBlock.blocked?(host)