From f916d994077bd62155f54f505f0ed2a108ecba3e Mon Sep 17 00:00:00 2001
From: Philip DeFraties <65036872+PhilipDeFraties@users.noreply.github.com>
Date: Wed, 18 Oct 2023 17:31:58 -0600
Subject: [PATCH] refactor `#signed_resource` method (#722)

breaks into helper methods

adds error handling and logging

reduces length of time the `s3_signer` object is cached to be less than
---
 app/controllers/application_controller.rb | 36 +++++++++++++++--------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 6382e0ad0..46edb17f9 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -39,26 +39,38 @@ def signed_resource
     # In order to circumvent making a request to AWS for tests, we can return the Paperclip attachment's 'url'.
     # If there isn't one, the default value is set to an empty string.
     if Rails.env.test?
-      render plain: params[:url]
-    else
-      signer = Rails.cache.fetch('s3_signer', expires_in: 45.minutes) do
-        s3_bucket = Aws::S3::Bucket.new(ENV['S3_BUCKET_NAME'])
-        WT::S3Signer.for_s3_bucket(s3_bucket, expires_in: 45.minutes)
-      end
-
-      path = params[:path].sub('/', '')
-      # any special characters not escaped by paperclip also need to be escaped
-      parser = URI::Parser.new
-      parsed_path = parser.escape(path).gsub(/[\(\)\*]/) {|m| "%#{m.ord.to_s(16).upcase}" }
+      render plain: params[:url] || ''
+      return
+    end
 
-      render plain: parsed_path.blank? ? parsed_path : signer.presigned_get_url(object_key: parsed_path)
+    unless params[:path]
+      render plain: "Missing path parameter", status: :bad_request
+      return
     end
+
+    parsed_path = sanitize_path(params[:path])
+    render plain: parsed_path.blank? ? "Invalid path" : s3_signer.presigned_get_url(object_key: parsed_path)
+  rescue => e
+    Rails.logger.error "Error in signed_resource: #{e.message}. Backtrace: #{e.backtrace.join("\n")}"
+    render plain: "An error occurred: #{e.message}", status: :internal_server_error
   end
 
   protected
 
   private
 
+  def sanitize_path(path)
+    parser = URI::Parser.new
+    parser.escape(path.sub('/', '')).gsub(/[\(\)\*]/) {|m| "%#{m.ord.to_s(16).upcase}" }
+  end
+
+  def s3_signer
+    Rails.cache.fetch('s3_signer', expires_in: 40.minutes) do
+      s3_bucket = Aws::S3::Bucket.new(ENV['S3_BUCKET_NAME'])
+      WT::S3Signer.for_s3_bucket(s3_bucket, expires_in: 45.minutes)
+    end
+  end
+
   def origin_data_json
     JSON.parse(File.read("#{Rails.root}/lib/assets/practice_origin_lookup.json"))
   end