From 8daf1fe033561c8fefa6bbb78324448bea48a0aa Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Wed, 11 Aug 2021 16:40:28 +0530 Subject: [PATCH] chore: Add down gem for Local file downloads (#2765) - Add down gem to handle downloading files to host machine - Remove the LocalResource class - Introduce max limit for contact avatars send via SDK --- Gemfile | 2 + Gemfile.lock | 3 ++ .../messages/facebook/message_builder.rb | 21 ++++++--- .../api/v1/accounts/callbacks_controller.rb | 32 ++------------ app/jobs/contact_avatar_job.rb | 9 ++-- app/jobs/contact_ip_lookup_job.rb | 6 ++- .../twilio/incoming_message_service.rb | 12 +++--- lib/chatwoot_hub.rb | 6 +-- lib/exception_list.rb | 4 +- lib/local_resource.rb | 43 ------------------- lib/webhooks/trigger.rb | 2 +- 11 files changed, 47 insertions(+), 93 deletions(-) delete mode 100644 lib/local_resource.rb diff --git a/Gemfile b/Gemfile index 0b5336889..c172b7d08 100644 --- a/Gemfile +++ b/Gemfile @@ -35,6 +35,8 @@ gem 'commonmarker' gem 'json_schemer' # Rack middleware for blocking & throttling abusive requests gem 'rack-attack' +# a utility tool for streaming, flexible and safe downloading of remote files +gem 'down', '~> 5.0' ##-- for active storage --## gem 'aws-sdk-s3', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 533bccb8b..853cba537 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -177,6 +177,8 @@ GEM dotenv-rails (2.7.6) dotenv (= 2.7.6) railties (>= 3.2) + down (5.2.3) + addressable (~> 2.8) dry-inflector (0.2.1) ecma-re-validator (0.3.0) regexp_parser (~> 2.0) @@ -651,6 +653,7 @@ DEPENDENCIES devise-secure_password (~> 2.0) devise_token_auth dotenv-rails + down (~> 5.0) facebook-messenger factory_bot_rails faker diff --git a/app/builders/messages/facebook/message_builder.rb b/app/builders/messages/facebook/message_builder.rb index bd9212ce0..23c9d92b4 100644 --- a/app/builders/messages/facebook/message_builder.rb +++ b/app/builders/messages/facebook/message_builder.rb @@ -24,6 +24,7 @@ class Messages::Facebook::MessageBuilder build_contact build_message end + ensure_contact_avatar rescue Koala::Facebook::AuthenticationError Rails.logger.info "Facebook Authorization expired for Inbox #{@inbox.id}" rescue StandardError => e @@ -41,7 +42,6 @@ class Messages::Facebook::MessageBuilder return if contact.present? @contact = Contact.create!(contact_params.except(:remote_avatar_url)) - ContactAvatarJob.perform_later(@contact, contact_params[:remote_avatar_url]) if contact_params[:remote_avatar_url] @contact_inbox = ContactInbox.create(contact: contact, inbox: @inbox, source_id: @sender_id) end @@ -61,10 +61,21 @@ class Messages::Facebook::MessageBuilder end def attach_file(attachment, file_url) - file_resource = LocalResource.new(file_url) - attachment.file.attach(io: file_resource.file, filename: file_resource.filename, content_type: file_resource.encoding) - rescue *ExceptionList::URI_EXCEPTIONS => e - Rails.logger.info "invalid url #{file_url} : #{e.message}" + attachment_file = Down.download( + file_url + ) + attachment.file.attach( + io: attachment_file, + filename: attachment_file.original_filename, + content_type: attachment_file.content_type + ) + end + + def ensure_contact_avatar + return if contact_params[:remote_avatar_url].blank? + return if @contact.avatar.attached? + + ContactAvatarJob.perform_later(@contact, contact_params[:remote_avatar_url]) end def conversation diff --git a/app/controllers/api/v1/accounts/callbacks_controller.rb b/app/controllers/api/v1/accounts/callbacks_controller.rb index 82c82f4bc..07a6b4d71 100644 --- a/app/controllers/api/v1/accounts/callbacks_controller.rb +++ b/app/controllers/api/v1/accounts/callbacks_controller.rb @@ -75,33 +75,9 @@ class Api::V1::Accounts::CallbacksController < Api::V1::Accounts::BaseController end def set_avatar(facebook_inbox, page_id) - uri = get_avatar_url(page_id) - - return unless uri - - avatar_resource = LocalResource.new(uri) - facebook_inbox.avatar.attach(io: avatar_resource.file, filename: avatar_resource.tmp_filename, content_type: avatar_resource.encoding) - rescue *ExceptionList::URI_EXCEPTIONS => e - Rails.logger.info "invalid url #{file_url} : #{e.message}" - end - - def get_avatar_url(page_id) - begin - url = 'http://graph.facebook.com/' << page_id << '/picture?type=large' - uri = URI.parse(url) - tries = 3 - begin - response = uri.open(redirect: false) - rescue OpenURI::HTTPRedirect => e - uri = e.uri # assigned from the "Location" response header - retry if (tries -= 1).positive? - raise - end - pic_url = response.base_uri.to_s - rescue StandardError => e - Rails.logger.debug { "Rescued: #{e.inspect}" } - pic_url = nil - end - pic_url + avatar_file = Down.download( + "http://graph.facebook.com/#{page_id}/picture?type=large" + ) + facebook_inbox.avatar.attach(io: avatar_file, filename: avatar_file.original_filename, content_type: avatar_file.content_type) end end diff --git a/app/jobs/contact_avatar_job.rb b/app/jobs/contact_avatar_job.rb index 9e0c7b813..4f0550632 100644 --- a/app/jobs/contact_avatar_job.rb +++ b/app/jobs/contact_avatar_job.rb @@ -2,9 +2,12 @@ class ContactAvatarJob < ApplicationJob queue_as :default def perform(contact, avatar_url) - avatar_resource = LocalResource.new(avatar_url) - contact.avatar.attach(io: avatar_resource.file, filename: avatar_resource.tmp_filename, content_type: avatar_resource.encoding) - rescue *ExceptionList::URI_EXCEPTIONS, NoMethodError => e + avatar_file = Down.download( + avatar_url, + max_size: 15 * 1024 * 1024 + ) + contact.avatar.attach(io: avatar_file, filename: avatar_file.original_filename, content_type: avatar_file.content_type) + rescue Down::Error => e Rails.logger.info "Exception: invalid avatar url #{avatar_url} : #{e.message}" end end diff --git a/app/jobs/contact_ip_lookup_job.rb b/app/jobs/contact_ip_lookup_job.rb index c8651a5c4..9b9d7f4b3 100644 --- a/app/jobs/contact_ip_lookup_job.rb +++ b/app/jobs/contact_ip_lookup_job.rb @@ -45,8 +45,10 @@ class ContactIpLookupJob < ApplicationJob def setup_vendor_db base_url = 'https://download.maxmind.com/app/geoip_download' - source = URI.parse("#{base_url}?edition_id=GeoLite2-City&suffix=tar.gz&license_key=#{ENV['IP_LOOKUP_API_KEY']}").open - tar_extract = Gem::Package::TarReader.new(Zlib::GzipReader.open(source)) + source_file = Down.download( + "#{base_url}?edition_id=GeoLite2-City&suffix=tar.gz&license_key=#{ENV['IP_LOOKUP_API_KEY']}" + ) + tar_extract = Gem::Package::TarReader.new(Zlib::GzipReader.open(source_file)) tar_extract.rewind tar_extract.each do |entry| diff --git a/app/services/twilio/incoming_message_service.rb b/app/services/twilio/incoming_message_service.rb index a52a988ae..c44f05324 100644 --- a/app/services/twilio/incoming_message_service.rb +++ b/app/services/twilio/incoming_message_service.rb @@ -93,7 +93,9 @@ class Twilio::IncomingMessageService def attach_files return if params[:MediaUrl0].blank? - file_resource = LocalResource.new(params[:MediaUrl0], params[:MediaContentType0]) + attachment_file = Down.download( + params[:MediaUrl0] + ) attachment = @message.attachments.new( account_id: @message.account_id, @@ -101,13 +103,11 @@ class Twilio::IncomingMessageService ) attachment.file.attach( - io: file_resource.file, - filename: file_resource.tmp_filename, - content_type: file_resource.encoding + io: attachment_file, + filename: attachment_file.original_filename, + content_type: attachment_file.content_type ) @message.save! - rescue *ExceptionList::URI_EXCEPTIONS => e - Rails.logger.info "invalid url #{file_url} : #{e.message}" end end diff --git a/lib/chatwoot_hub.rb b/lib/chatwoot_hub.rb index e98a6b86e..91b5150dc 100644 --- a/lib/chatwoot_hub.rb +++ b/lib/chatwoot_hub.rb @@ -36,7 +36,7 @@ class ChatwootHub info = info.merge(instance_metrics) unless ENV['DISABLE_TELEMETRY'] response = RestClient.post(PING_URL, info.to_json, { content_type: :json, accept: :json }) version = JSON.parse(response)['version'] - rescue *ExceptionList::REST_CLIENT_EXCEPTIONS, *ExceptionList::URI_EXCEPTIONS => e + rescue *ExceptionList::REST_CLIENT_EXCEPTIONS => e Rails.logger.info "Exception: #{e.message}" rescue StandardError => e Sentry.capture_exception(e) @@ -47,7 +47,7 @@ class ChatwootHub def self.register_instance(company_name, owner_name, owner_email) info = { company_name: company_name, owner_name: owner_name, owner_email: owner_email, subscribed_to_mailers: true } RestClient.post(REGISTRATION_URL, info.merge(instance_config).to_json, { content_type: :json, accept: :json }) - rescue *ExceptionList::REST_CLIENT_EXCEPTIONS, *ExceptionList::URI_EXCEPTIONS => e + rescue *ExceptionList::REST_CLIENT_EXCEPTIONS => e Rails.logger.info "Exception: #{e.message}" rescue StandardError => e Raven.capture_exception(e) @@ -58,7 +58,7 @@ class ChatwootHub info = { event_name: event_name, event_data: event_data } RestClient.post(EVENTS_URL, info.merge(instance_config).to_json, { content_type: :json, accept: :json }) - rescue *ExceptionList::REST_CLIENT_EXCEPTIONS, *ExceptionList::URI_EXCEPTIONS => e + rescue *ExceptionList::REST_CLIENT_EXCEPTIONS => e Rails.logger.info "Exception: #{e.message}" rescue StandardError => e Sentry.capture_exception(e) diff --git a/lib/exception_list.rb b/lib/exception_list.rb index 513dd5365..79436eb07 100644 --- a/lib/exception_list.rb +++ b/lib/exception_list.rb @@ -1,7 +1,7 @@ module ExceptionList - URI_EXCEPTIONS = [Errno::ETIMEDOUT, Errno::ECONNREFUSED, URI::InvalidURIError, Net::OpenTimeout, SocketError].freeze REST_CLIENT_EXCEPTIONS = [RestClient::NotFound, RestClient::GatewayTimeout, RestClient::BadRequest, - RestClient::MethodNotAllowed, RestClient::Forbidden, RestClient::InternalServerError, RestClient::PayloadTooLarge].freeze + RestClient::MethodNotAllowed, RestClient::Forbidden, RestClient::InternalServerError, + RestClient::PayloadTooLarge, SocketError].freeze SMTP_EXCEPTIONS = [ Net::SMTPSyntaxError ].freeze diff --git a/lib/local_resource.rb b/lib/local_resource.rb deleted file mode 100644 index e7fa28106..000000000 --- a/lib/local_resource.rb +++ /dev/null @@ -1,43 +0,0 @@ -class LocalResource - attr_reader :uri - - def initialize(uri, file_type = nil) - @uri = URI(uri) - @file_type = file_type - end - - def file - @file ||= Tempfile.new(tmp_filename, tmp_folder, encoding: encoding).tap do |f| - io.rewind - f.write(io.read) - f.close - end - @file.open - end - - def io - # TODO: should we use RestClient here too ? - @io ||= uri.open(read_timeout: 5) - end - - def encoding - io.rewind - io.read.encoding - end - - def find_file_type - @file_type ? @file_type.split('/').last : Pathname.new(uri.path).extname - end - - def tmp_filename - [Time.now.to_i.to_s, find_file_type].join('.') - end - - def tmp_folder - Rails.root.join('tmp') - end - - def filename - File.basename(uri.path) - end -end diff --git a/lib/webhooks/trigger.rb b/lib/webhooks/trigger.rb index fa90f3093..49ac6bb09 100644 --- a/lib/webhooks/trigger.rb +++ b/lib/webhooks/trigger.rb @@ -6,7 +6,7 @@ class Webhooks::Trigger headers: { content_type: :json, accept: :json }, timeout: 5 ) - rescue *ExceptionList::REST_CLIENT_EXCEPTIONS, *ExceptionList::URI_EXCEPTIONS => e + rescue *ExceptionList::REST_CLIENT_EXCEPTIONS => e Rails.logger.info "Exception: invalid webhook url #{url} : #{e.message}" rescue StandardError => e Sentry.capture_exception(e)